search

google / ngx_brotli

NGINX module for Brotli compression
BSD 2-Clause "Simplified" License
2.1k stars 216 forks source link

"brotli_static on;" causes "Vary: Accept-Encoding" to be added to every file (including images) #97

Open Jas0n99 opened 4 years ago

Jas0n99 commented 4 years ago

Sorry, bumped my enter key and github submitted this empty... argh... Anyhow, I just noticed this bug.

I have brotli_static enabled at the 'server' level. I just noticed that the "Vary: Accept-Encoding" header is added to things it shouldn't be... Biggest example is images! I noticed all my .png, .jpg, and .gif had that header. I started going nuts trying to figure out what was causing it... Finally when I disabled brotli_static things went back to normal.

Having nginx's gzip_static enabled in the same 'server' context works as expected. Not sure how similar the code is, but maybe there's some inspiration there that could help fix this issue.

This is happening for files directly served by nginx, running 1.17.9

Jas0n99 commented 4 years ago

I guess I should add as a clarification that what was happening was brotli_static was adding the "Vary: Accept-Encoding" to file types NOT listed in the brotli_types directive. If that makes more sense.

dilyanpalauzov commented 4 years ago

Do gzip_static and brotli_static behave differently (taking into account brotli_types and gzip_types)?

Is the Vary only too much for static (pre-compressed) data, or also for run-time (live) compressed data? I mean do • do gzip_static and brotli_static behave the same in respect to Vary (when gzip_types and brotly_types are the same) • do broltli on and gzip on behava the same in respect to Vary (when gzip_types and brotli_types are the same)

and thus, is this only related to statically compressed files, or also for dynamically compressed files?

Jas0n99 commented 4 years ago

This patch is only relevant when you are using brotli_static on;. With that directive on, all files (even types that aren't specified in the brotli_types) get a Vary: Accept-Encoding header.

If you aren't using brotli_static on;, then everything works fine.

If you only use gzip static on; then it also works fine, and the Vary header is only added for types that are listed in the gzip_types.

Whether the statically compressed file exists or not is irrelevant. It's a logic-error in the static code that was triggering the vary header too soon before doing other checks.

ktecho commented 4 years ago

Is there any advance on this issue? I was going to put it into my server, but this scares me a lot.

Janaka-Steph commented 4 years ago

Regarding brotli_types configuration option, the readme says that it applies to on-the-fly compression, not static files. https://github.com/google/ngx_brotli#brotli_types