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

Improve Accept-Encoding parsing #352

Open
adamchainz opened this issue Feb 4, 2022 · 3 comments
Open

Improve Accept-Encoding parsing #352

adamchainz opened this issue Feb 4, 2022 · 3 comments

Comments

@adamchainz
Copy link
Collaborator

Currently invalid values like gzip brotli spam still pass, and valid values like gzip;q=0 ("don't send me gzip") are intepreted as "send me gzip". Also the "no encoding" case relies on matching r"" within the string (?).

The header should be parsed correctly. Logic can be borrowed from Django's request.accepts().

@jribbens
Copy link
Contributor

This is a very valid point. Also along the same lines, if multiple encodings are available (e.g. gzip and brotli), whitenoise does not choose sensibly between them. Chrome for example sends Accept-Encoding: gzip, deflate, br which means all of those encodings are equally acceptable, and yet whitenoise treats it as "gzip by preference" (because it's earlier in the list), so despite the good work put in to make it support brotli, in practice the brotli versions will never be used - which is a shame given brotli is up to one hundred times better than gzip! Given whitenoise has the file sizes immediately available to it, it would seem only sensible for it to choose the smallest file from amongst the equally-acceptable encodings.

@evansd
Copy link
Owner

evansd commented Oct 25, 2022

whitenoise treats it as "gzip by preference" (because it's earlier in the list)

I'm pretty sure that's not true. It prefers the smallest response size:
https://github.com/evansd/whitenoise/blob/main/src/whitenoise/responders.py#L194

@jribbens
Copy link
Contributor

Hmm, that's not the behaviour I was seeing yesterday! Perhaps nginx is interfering somehow...

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

No branches or pull requests

3 participants