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

Throw upon cleartext HTTP queries #1181

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Conversation

fynngodau
Copy link
Member

  • I carefully read the contribution guidelines and agree to them.
  • I have tested the API against NewPipe.
  • I agree to create a pull request for NewPipe as soon as possible to make it compatible with the changed API.

NewPipe presently does not have a network policy (see manifest), hence from Android 9 onwards, it is not ever possible for it to make cleartext HTTP queries. Thus as of now, to avoid errors, the extractor must make sure that all queries it sends are encrypted HTTPS queries.

I propose explicitly forcing the extractor itself to refuse HTTP queries. This extends the guarantee that no HTTPS queries are made towards all other downstreams as well, and allows testing for bugs related to missing HTTPS queries from within the extractor's tests. TeamNewPipe/NewPipe#11074 is such a bug that could have been caught through tests.

The alternative would be to transparently upgrade queries to HTTPS from within the Downloader class. However, I would prefer if extractors ensured that they never send HTTPS queries in the first place.

@fynngodau
Copy link
Member Author

Nicely, the tests fail because #1177 is not merged. 🙂

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.

1 participant