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

feat: add support for encrypted query parameters #65

Merged
merged 9 commits into from
Mar 27, 2025

Conversation

Fijxu
Copy link
Contributor

@Fijxu Fijxu commented Mar 18, 2025

Closes #29

Requires support in Invidious if videoplayback is going to be proxied trough Invidious, but that doesn't make that much sense since companion already does that. Same thing applies to piped-proxy

@absidue
Copy link

absidue commented Mar 18, 2025

Why are you encrypting all query parameters, instead of only the ones you consider sensitive? This will break multiple audio track support as well as detecting when the streaming data has expired in 3rd party clients like FreeTube.

Then again I doubt most people are using Invidious with FreeTube anymore these days as public instances have the API disabled and it's a lot easier to use FreeTube's local API than self-hosting Invidious.

@Fijxu
Copy link
Contributor Author

Fijxu commented Mar 18, 2025

This will break multiple audio track support as well as detecting when the streaming data has expired in 3rd party clients like FreeTube.

You are right. I was only thinking on the Invidious side of things when I was doing it. I'll fix it

@unixfox
Copy link
Member

unixfox commented Mar 18, 2025

Why are you encrypting all query parameters, instead of only the ones you consider sensitive? This will break multiple audio track support as well as detecting when the streaming data has expired in 3rd party clients like FreeTube.

I agree too, "ip", "pot" parameters would have been enough. The rest is OK for anybody to know the value of these parameters.

And encrypting everything adds unnecessary load, on top of the fact that just dealing with two parameters would be easier to implement in piped-proxy.

@Fijxu Fijxu force-pushed the encrypted-videoplayback-query-params branch from 3c28b02 to 91c849e Compare March 18, 2025 22:46
@Fijxu
Copy link
Contributor Author

Fijxu commented Mar 18, 2025

I was also thinking to add a configuration option to insert a list of query parameters that need to be encrypted. Something like this: server.protected_query_parameters = ["pot", "ip", "someparam"] so we can add more query parameters without the need of modifying the source code each time youtube adds a query parameter that needs to be secret. May be overkill for now, but if no one is against, I could add it

Copy link
Collaborator

@alexmaras alexmaras left a comment

Choose a reason for hiding this comment

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

I'd look at using the fact that URLSearchParams objects are just arrays of arrays to avoid a bunch of string appending and handling you're having to do here. It's a lot more fallible than doing everything you need to do with a URLSearchparams object and then calling .toString() once at the end.

@Fijxu Fijxu force-pushed the encrypted-videoplayback-query-params branch 3 times, most recently from 7d60cd4 to 5d427dd Compare March 20, 2025 19:29
@Fijxu Fijxu force-pushed the encrypted-videoplayback-query-params branch from 5d427dd to 741184a Compare March 27, 2025 02:54
@unixfox
Copy link
Member

unixfox commented Mar 27, 2025

I was also thinking to add a configuration option to insert a list of query parameters that need to be encrypted. Something like this: server.protected_query_parameters = ["pot", "ip", "someparam"] so we can add more query parameters without the need of modifying the source code each time youtube adds a query parameter that needs to be secret. May be overkill for now, but if no one is against, I could add it

I think we can make it evolve in the code instead as more parameters comes out.

@Fijxu Fijxu force-pushed the encrypted-videoplayback-query-params branch from 529390c to 10de73b Compare March 27, 2025 19:49
@unixfox unixfox merged commit 7a56a46 into iv-org:master Mar 27, 2025
1 check passed
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.

Support for encrypted videoplayback requests
4 participants