Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

settings.py: add 'proxy_images' #24

Merged
merged 6 commits into from
Oct 12, 2020

Conversation

zrose584
Copy link
Contributor

@zrose584 zrose584 commented Oct 7, 2020

No description provided.

Copy link
Owner

@user234683 user234683 left a comment

Choose a reason for hiding this comment

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

May I ask, what's the intent of the setting (i.e., is the extra complexity really worth it)?

I left some feedback on the code for reference, but this sort of feature would be cleaner if it acted in server.py at the routing level. It should still route through the server itself but not through Tor, so that the browser doesn't send any identifying info to the image servers that it doesn't have to.

proxy_site could be replaced with an intermediate handler which checks the setting for lh3.googleusercontent.com, yt3.ggpht.com, and ytimg.com in server.py.

youtube/local_playlist.py Outdated Show resolved Hide resolved
youtube/playlist.py Outdated Show resolved Hide resolved
youtube/subscriptions.py Outdated Show resolved Hide resolved
youtube/templates/base.html Outdated Show resolved Hide resolved
youtube/util.py Outdated Show resolved Hide resolved
@zrose584
Copy link
Contributor Author

zrose584 commented Oct 9, 2020

The motivation stems from an addon I use, imagus.
It has handmade rules for ggpht.com and ytimg.com.
Porting those to youtube_local would risk inconsistency with the original/upstream rules.

It should still route through the server itself but not through Tor, so that the browser doesn't send any identifying info to the image servers that it doesn't have to.

No, that would defeat the purpose of this patch. Also I think it's unlikely they use (image) data requests for user identification.
And even if, I could use some header-manipulation addon to achieve what youtube_local does (it's just about the HTTP-Headers, right?).

@zrose584
Copy link
Contributor Author

The addon uses element's src attribute, which is unchanged by a 302.
Can this be merged now?

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.

2 participants