Opened 6 years ago

Closed 4 months ago

#964 closed defect (fixed)

Expires header incorrectly prioritised over Cache-Control: max-age

Reported by: cbranch@… Owned by:
Priority: minor Milestone:
Component: nginx-core Version: 1.9.x
Keywords: Cc:
uname -a: Linux cbranch-vm 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt25-2 (2016-04-08) x86_64 GNU/Linux
nginx -V: nginx version: nginx/1.9.7
built by gcc 4.9.2 (Debian 4.9.2-10)

Description

When using nginx as a caching reverse proxy, items may be cached for the wrong amount of time if the Expires header is inconsistent with max-age. Caching will be disabled if the Expires header value is in the past or malformed.

Per RFC 2616 section 14.9.3, max-age takes precedence over Expires. However, nginx prefers whichever header/directive occurs first in the response, which causes unexpected results when migrating to nginx from an RFC-compilant caching reverse proxy.

A minimally-reproducible config is attached. Observe that no file is cached when accessing http://127.0.0.2:8080/fail, but a file is cached when accessing http://127.0.0.2:8080/success.

Attachments (1)

nginx.conf (828 bytes ) - added by cbranch@… 6 years ago.

Download all attachments as: .zip

Change History (7)

by cbranch@…, 6 years ago

Attachment: nginx.conf added

comment:1 by Maxim Dounin, 6 years ago

Component: documentationnginx-core
Status: newaccepted

The description isn't really correct: nginx prefers Cache-Control over Expires. But if Expires is given first and disables caching, nginx won't cache the response. This is believed to be correct, though may be suboptimal in some cases.

See 6a3ab6fdd70f for some additional details on the current behaviour. Previous attempts to address things can be found in this thread.

comment:2 by Maxim Dounin, 9 months ago

See also #2309.

comment:3 by yugo-horie.jocdn.co.jp@…, 8 months ago

As refer to #2309,

Further, it doesn't seem to solve stale-while-revalidate and stale-if-error times, which also behave differently depending on the headers order

Which should we prefer if there are both of Cache-Control: stale-while-revalidate and X-Accel-Expires (or Expires)? We have intended simply that X-Accel-Expires should be prioritized from Cache-Control. Although is it not seems to be enough what you commented? Thus, in case of Cache-Control: stale-xxx are there, is it should prioritized from any headers such as XAE or Expires?

in reply to:  3 comment:4 by Maxim Dounin, 8 months ago

Further, it doesn't seem to solve stale-while-revalidate and stale-if-error times, which also behave differently depending on the headers order

Which should we prefer if there are both of Cache-Control: stale-while-revalidate and X-Accel-Expires (or Expires)? We have intended simply that X-Accel-Expires should be prioritized from Cache-Control. Although is it not seems to be enough what you commented? Thus, in case of Cache-Control: stale-xxx are there, is it should prioritized from any headers such as XAE or Expires?

I don't think there is a simple answer. But current behaviour is different for responses with different order of headers. For example, the following headers:

X-Accel-Expires: 10
Cache-Control: max-age=100, stale-while-revalidate=1000

will result in the response cached for 10 seconds (and Cache-Control ignored), while

Cache-Control: max-age=100, stale-while-revalidate=1000
X-Accel-Expires: 10

will result in the response cache for 10 seconds with stale-while-revalidate=1000. This is an inconsistency mostly identical to the one you are trying to address, with non-cacheable status being used from Cache-Control in a similar situation.

An obvious solution would be to ignore Cache-Control completely if X-Accel-Expires is present in the response, and change the second case to produce the result identical to the first one. Another approach might be to assume that X-Accel-Expires is only equivalent to Cache-Control: max-age and the non-cacheable status, yet stale-while-revalidate still applies if present (and change the first case to match the second one).

comment:5 by Maxim Dounin <mdounin@…>, 4 months ago

In 8041:0784ab86ad08/nginx:

Upstream: fixed X-Accel-Expires/Cache-Control/Expires handling.

Previously, if caching was disabled due to Expires in the past, nginx
failed to cache the response even if it was cacheable as per subsequently
parsed Cache-Control header (ticket #964).

Similarly, if caching was disabled due to Expires in the past,
"Cache-Control: no-cache" or "Cache-Control: max-age=0", caching was not
used if it was cacheable as per subsequently parsed X-Accel-Expires header.

Fix is to avoid disabling caching immediately after parsing Expires in
the past or Cache-Control, but rather set flags which are later checked by
ngx_http_upstream_process_headers() (and cleared by "Cache-Control: max-age"
and X-Accel-Expires).

Additionally, now X-Accel-Expires does not prevent parsing of cache control
extensions, notably stale-while-revalidate and stale-if-error. This
ensures that order of the X-Accel-Expires and Cache-Control headers is not
important.

Prodded by Vadim Fedorenko and Yugo Horie.

comment:6 by Maxim Dounin, 4 months ago

Resolution: fixed
Status: acceptedclosed

Fixed, thanks to all involved.

Note: See TracTickets for help on using tickets.