search

tokers / zstd-nginx-module

Nginx modules for the Zstandard compression
BSD 2-Clause "Simplified" License
196 stars 23 forks source link

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

Open bradsoto opened 5 years ago

bradsoto commented 5 years ago

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"

tokers commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

@bradsoto OK, will have a close look.

tokers commented 5 years ago

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

tokers commented 5 years ago

@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 commented 5 years ago

@bradsoto Hi, I have just pushed a commit to resolve this problem: https://github.com/tokers/zstd-nginx-module/commit/46f95bfbbed8cf611eb57a08fd37d1e05cb6ba82. Please try the latest master branch.

bradsoto commented 5 years ago

Fix verified! Everything works as expected.

kravietz commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

Ah, just as I expected - #5 :)

felixhandte commented 5 years ago

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

tokers commented 5 years ago

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 commented 2 years ago

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