Opened 4 years ago

Closed 3 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 Maxim Dounin, 4 years ago

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.

comment:2 by Maxim Dounin, 4 years ago

Resolution: wontfix
Status: newclosed

Feedback timeout.

comment:3 by Anders Carling, 4 years ago

Resolution: wontfix
Status: closedreopened

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.

Last edited 4 years ago by Anders Carling (previous) (diff)

comment:4 by Anders Carling, 4 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.

Last edited 4 years ago by Anders Carling (previous) (diff)

comment:5 by Neil Craig, 4 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 Anders Carling, 4 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 avengerpenguin@…, 3 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 Neil Craig, 3 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 Maxim Dounin, 3 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 Anders Carling, 3 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:11 by Maxim Dounin <mdounin@…>, 3 years ago

In 6908:609daeb3b48b/nginx:

Cache: increased cache header Vary and ETag lengths to 128.

This allows to store larger ETag values for proxy_cache_revalidate,
including ones generated as SHA256, and cache responses with longer
Vary (ticket #826).

In particular, this fixes caching of Amazon S3 responses with CORS
enabled, which now use "Vary: Origin, Access-Control-Request-Headers,
Access-Control-Request-Method".

Cache version bumped accordingly.

comment:12 by Maxim Dounin, 3 years ago

Resolution: fixed
Status: reopenedclosed
Note: See TracTickets for help on using tickets.