Opened 9 years ago
Closed 8 years ago
#826 closed enhancement (fixed)
Add config option for NGX_HTTP_CACHE_VARY_LEN
Reported by: | Neil Craig | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | |
Component: | nginx-core | Version: | 1.9.x |
Keywords: | vary header | Cc: | |
uname -a: | n/a | ||
nginx -V: | n/a |
Description
Hi
Currently NGX_HTTP_CACHE_VARY_LEN is hardcoded at 42 characters in length. This is undocumented and causes us major issues (as we vary on zero to many header values) in our reverse proxy setup.
I have a workaround currently which is a build-time patch (against 1.9.x):
https://gist.github.com/neilstuartcraig/62cb5236eeabab9a8a4d
It would be really nice if this could be a configurable option to make it simple to change without mandating users compile nginx from source.
I have had messages via comments and twitter since documenting this on my blog (https://thedotproduct.org/nginx-vary-header-handling/) from several people who hit the same issue.
Cheers
Neil
Change History (12)
comment:1 by , 9 years ago
comment:3 by , 8 years ago
Resolution: | wontfix |
---|---|
Status: | closed → reopened |
I agree that adding a configuration option seems overkill, but I would seriously considering bumping the number.
A reasonable Vary-header for many sites might be Accept,Accept-Encoding,Accept-Language
(38 chars). Tthat's really only using standard headers and almost hits the limit.
The longest Very-header we (an api) emit in production (for a request we expect to cache at least) is Accept,Accept-Encoding,X-TummyLab-API,X-TummyLab-Last-Response-Identifier
(73 chars) - but at some point we'll probably throw at least Accept-Language in there and probably something more.
A secondary issue is that no error logging at all is done when the request is marked as uncachable, which makes this "issue" really hard to detect. I just found it by dumb luck after introducing a new header to Vary.
comment:4 by , 8 years ago
As a reference point, varnish actually allows up to 65k characters in Vary headers.
That is probably just a "random big number" but obviously they see real world value in keeping it quite large.
comment:5 by , 8 years ago
FWIW, we make extensive use of the vary header and out NGINX builds have the max length patched to 8k - think this would be sufficient for most purposes.
comment:6 by , 8 years ago
Agreed, I see no reason to make it bigger than 8k (or probably even 2k), whatever number is selected it should probably log an error message if it's hit though.
comment:7 by , 8 years ago
Yes, I hit this too in production. We added one more header to the 'Vary' and it stopped caching silently. A warning would have helped perhaps. We ended up patching to 254 (255 and above caused a compiler warning on our build).
I nearly got it working with a single Vary header per request header, as is valid per the HTTP spec (any multi-valued header can be repeated instead of the a, b, c syntax), but Express and Node,js did not like that. It seems a lot of frameworks make the assumption they can use a dict/map/object structure to model headers and thus cannot handle a header appearing multiple times.
I would be happy just to see the number raised instead of being made configurable.
comment:8 by , 8 years ago
By way of adding weight to this, I just noted that the new www.ft.com is using the following vary header:
ft-flags, ft-edition, ft-anonymous-user, ft-force-opt-in-device, FT-Site, FT-Regional-News, Accept-Encoding, FT-Site, FT-Regional-News, Accept-Encoding
Which i make 151 bytes/chars.
comment:9 by , 8 years ago
All FT-*
headers seems to be useless, as they are never sent by normal browsers. And the remaining 'Accept-Encoding' header listed twice is a good indicator that there was no intention to think while constructing the list.
comment:10 by , 8 years ago
Normal browsers might not be the only clients accessing that url though, so maybe useless for some but not necessarily for all traffic. Double Accept-Encoding is definitely a mistake, but something that could easily happen with multiple layers of middleware or whatnot.
I think the main questions here is: Would allowing a larger Vary header size would cause a problem larger than the existing problem of silent breaking cache for standard compliant http content?
If so, would it be possible to add a high priority message to the error log telling people their cache is breaking?
If it's all a matter of getting the code written I could have a look at it, even tough it would be the first effort in this sort of thing so it would probably require some review by people who knows the nginx code base better..
comment:12 by , 8 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
There are no plans to add configurable option, this is an overkill and also will break cache compatibility between nginx binaries compiled with different limits. We may consider increasing the limit though. For us to do so, please demonstrate a real use case where a long Vary header make sense.