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

text/html is not included by default when specifying zstd_types #3

Open
bradsoto opened this issue Mar 6, 2019 · 14 comments
Open

text/html is not included by default when specifying zstd_types #3

bradsoto opened this issue Mar 6, 2019 · 14 comments
Assignees
Labels
bug Something isn't working

Comments

@bradsoto
Copy link

bradsoto commented Mar 6, 2019

When serving content-type text/html: this module does not encode zstd by default. It must be added to zstd_types manually. This goes against the expected behaviors like the gzip and brotli modules (when specifying zstd_types. The module serves zstd for text/html when zstd_types is not specified). Expected behavior is mentioned in the documentation, "types in addition to text/html"

@bradsoto bradsoto changed the title text/html is not the default type text/html is not the default type when specifying zstd_types Mar 6, 2019
@bradsoto bradsoto changed the title text/html is not the default type when specifying zstd_types text/html is not included by default when specifying zstd_types Mar 6, 2019
@tokers
Copy link
Owner

tokers commented Mar 7, 2019

@bradsoto
If you didn't specify the zstd_types directive, the default value would be used, or you must specify all the types that you want. This is also true when you use the gzip module. So I believe the behavior is expected.

BTW, just configure your zstd_types like this, it should cover the most scenes.

zstd_types text/plain text/css application/json application/x-javascript text/xml application/xml application/xml+rss text/javascript;

@bradsoto
Copy link
Author

bradsoto commented Mar 7, 2019

This is a snippet of my current config. When requesting gzip or brotli encoding for text/html it works. When requesting zstd encoding it also works, but (unlike gzip or brotli) requires text/html added in at the end.

gzip on;
gzip_min_length 256;
gzip_comp_level 9;
gzip_proxied any;
gzip_types application/atom+xml application/geo+json application/javascript application/json application/ld+json application/manifest+json application/rdf+xml application/rss+xml application/vnd.ms-fontobject application/wasm application/x-perl application/x-web-app-manifest+json application/xhtml+xml application/xml application/xspf+xml audio/midi font/otf image/bmp image/svg+xml text/cache-manifest text/calendar text/css text/javascript text/markdown text/plain text/vcard text/vnd.rim.location.xloc text/vtt text/x-component text/x-cross-domain-policy text/xml;
brotli on;
brotli_comp_level 8;
brotli_window 16m;
brotli_min_length 256;
brotli_types application/atom+xml application/geo+json application/javascript application/json application/ld+json application/manifest+json application/rdf+xml application/rss+xml application/vnd.ms-fontobject application/wasm application/x-perl application/x-web-app-manifest+json application/xhtml+xml application/xml application/xspf+xml audio/midi font/otf image/bmp image/svg+xml text/cache-manifest text/calendar text/css text/javascript text/markdown text/plain text/vcard text/vnd.rim.location.xloc text/vtt text/x-component text/x-cross-domain-policy text/xml;
zstd on;
zstd_comp_level 14;
zstd_min_length 256;
zstd_types application/atom+xml application/geo+json application/javascript application/json application/ld+json application/manifest+json application/rdf+xml application/rss+xml application/vnd.ms-fontobject application/wasm application/x-perl application/x-web-app-manifest+json application/xhtml+xml application/xml application/xspf+xml audio/midi font/otf image/bmp image/svg+xml text/cache-manifest text/calendar text/css text/javascript text/markdown text/plain text/vcard text/vnd.rim.location.xloc text/vtt text/x-component text/x-cross-domain-policy text/xml text/html;

@tokers
Copy link
Owner

tokers commented Mar 7, 2019

@bradsoto
OK, will have a close look.

@tokers
Copy link
Owner

tokers commented Mar 7, 2019

@bradsoto
By the way, what's your Nginx version?

@tokers tokers self-assigned this Mar 7, 2019
@tokers tokers added the help wanted Extra attention is needed label Mar 7, 2019
@tokers
Copy link
Owner

tokers commented Mar 7, 2019

@bradsoto
All right. It's my fault, I glanced the source code about the gzip module and I found that I forget to pass the default parameter to the command parser.

I will push a fix as soon as possible. Thanks for your report!

@tokers tokers added bug Something isn't working and removed help wanted Extra attention is needed labels Mar 7, 2019
@tokers
Copy link
Owner

tokers commented Mar 7, 2019

@bradsoto
Hi, I have just pushed a commit to resolve this problem: 46f95bf. Please try the latest master branch.

@bradsoto
Copy link
Author

bradsoto commented Mar 7, 2019

Fix verified! Everything works as expected.

@bradsoto bradsoto closed this as completed Mar 7, 2019
@kravietz
Copy link

Plese note this creates a potential vector for BREACH attack as documented in similar discussion on Brotli here https://answers.launchpad.net/ubuntu/+source/nginx/+question/678209

@felixhandte
Copy link

@kravietz, that's true, in the sense that all LZ-style compression of HTTP responses are vulnerable to BREACH. I don't believe Zstd has any features that make it any more or less vulnerable than Brotli or Gzip, the latter of which is nonetheless ubiquitously deployed on the web. Mitigating BREACH/CRIME/HEIST/etc. style vulnerabilities is therefore orthogonal to selecting a compressor.

@kravietz
Copy link

@felixhandte I think the point the Ubuntu Security Team had in the Brotli discussion linked above was that text/html is compressed by default and you cannot disable it. I'm just trying to get Zstd module into the mainline Ubuntu nginx-extras distribution and they will certainly raise this point :)

@kravietz
Copy link

Ah, just as I expected - #5 :)

@felixhandte
Copy link

@kravietz, aha, I missed that detail. Makes sense.

@tokers tokers reopened this May 17, 2019
@tokers
Copy link
Owner

tokers commented May 17, 2019

Actually I don't catch the point why nginx gzip module would always compress the text/html (maybe for the sake of convenience? I don't know). Just like I said at #5, I think we'd better to resist for this.

@felixhandte @kravietz
@bradsoto

@rthamrin
Copy link

How can I configure or edit my nginx.conf through configmap to activate the zstd?

@jason-x-xu jason-x-xu mentioned this issue Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants