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

Treat "Accept-Encoding: *" as not accepting any encoding #323

Merged
merged 4 commits into from
Feb 4, 2022

Conversation

rtibbles
Copy link
Contributor

  • Adds support for "*" Accept-Encoding value
  • Makes Accept-Encoding default to this value when missing as per spec
  • Adds tests for both values

Fixes #303

@adamchainz
Copy link
Collaborator

adamchainz commented Jan 31, 2022

I feel a bit uneasy about sending compressed data to clients that don't necessarily support it. It would probably be better to return the non-compressed version.

For reference, I checked what nginx does. I found here in its gzip plugin that it seems to bail out if no Accept-Encoding header is provided, or if gzip isn't explicitly mentioned: https://github.com/nginx/nginx/blob/1f01183b9e6658749934313fd72f7f16c1918b54/src/http/ngx_http_core_module.c#L2037-L2040

Copying this approach seems more sensible than returning the smallest compressed response.

Would you be willing to update the PR? Afaict it requires a bit more rework since alternatives is only sorted by size, but you explicitly want the not-compressed version.

@rtibbles
Copy link
Contributor Author

rtibbles commented Feb 2, 2022

Can update - I don't think it will be difficult, can just revert the change that updates the default to "*" when the value is missing and put it back to "".

@rtibbles
Copy link
Contributor Author

rtibbles commented Feb 2, 2022

Updated the PR.

tests/test_whitenoise.py Outdated Show resolved Hide resolved
tests/test_whitenoise.py Outdated Show resolved Hide resolved
tests/test_whitenoise.py Outdated Show resolved Hide resolved
@adamchainz adamchainz changed the title Add support for "*" Accept-Encoding. Default to "*" if missing. Treat "Accept-Encoding: *" as not accepting any encoding Feb 4, 2022
@adamchainz adamchainz merged commit 34aa2af into evansd:master Feb 4, 2022
@rtibbles rtibbles deleted the accept_encoding_star branch May 14, 2022 22:00
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.

Missing or "*" Accept-Encoding unhandled
2 participants