Skip to content

fixed favicon issue#1064

Merged
abidlabs merged 4 commits into
mainfrom
favicon-fix
Apr 25, 2022
Merged

fixed favicon issue#1064
abidlabs merged 4 commits into
mainfrom
favicon-fix

Conversation

@abidlabs
Copy link
Copy Markdown
Member

We had dropped support for favicons at some point. This implements it again. Closes: #1036

Sample code to test:

import gradio as gr

gr.Interface(
    fn=lambda x:x,
    inputs="text",
    outputs="text",
).launch(
    favicon_path="lion.jpg",
)

Copy link
Copy Markdown
Contributor

@omerXfaruq omerXfaruq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good job👍

Comment thread gradio/routes.py Outdated

@app.get("/favicon.ico")
async def favicon():
return FileResponse(app.blocks.favicon_path)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about this but weren't we doing some checks to file paths to not access to different paths within user's computer?
like os.some_function

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I thought about this as well. In this case, we don't have to worry about that because the favicon route only returns a fixed filepath that is specified by the demo creator. Where this can become a security concern is if a demo user can request a file at an arbitrary path (like in the /file/ route)

@abidlabs abidlabs marked this pull request as draft April 22, 2022 18:29
@abidlabs
Copy link
Copy Markdown
Member Author

Going to add a default favicon, as suggested by @pngwn, thanks!

@abidlabs abidlabs marked this pull request as ready for review April 25, 2022 19:34
@abidlabs abidlabs merged commit 1ec62d9 into main Apr 25, 2022
@abidlabs abidlabs deleted the favicon-fix branch April 25, 2022 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

favicon_path doesn't work

2 participants