Opened 8 years ago

Closed 10 months ago

#27225 closed Bug (fixed)

"age" header is not set for responses taken from cache.

Reported by: Rinat Khabibiev Owned by: Alexander Lazarević
Component: HTTP handling Version: 1.10
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When fetching response from the cache, 'Cache-Control' contains 'max-age' corresponding to the moment of original response generation time, thus for another time it usually doesn't match 'Expires' value.

Change History (15)

comment:1 by Rinat Khabibiev, 8 years ago

Status: new → assigned

comment:2 by Tim Graham, 8 years ago

I'm not sure if the proposed behavior is correct. From ​RC2616 sec14.9.3 (which I only skimmed, so please provide another reference if it's wrong):

If a response includes both an Expires header and a max-age directive, the max-age directive overrides the Expires header, even if the Expires header is more restrictive.

I think I understand the idea behind the ​PR, however, it seems to treat the Expires as authoritative rather than max-age.

comment:3 by Rinat Khabibiev, 8 years ago

Exactly, and that's why it is a bug.

It would be correct behavior if response include Date header corresponding to original response generation time. But I'm not sure

comment:4 by Rinat Khabibiev, 8 years ago

[RFC7234](​https://tools.ietf.org/html/rfc7234#section-5.1) says about Age header which indicates age of the cache, so instead of Cache-Control's max-age regeneration it is possible to set Age. Also there is good visual explanation how browser cache should work: ​https://developer.mozilla.org/en-US/docs/Web/HTTP/Caching#Freshness

Version 0, edited 8 years ago by Rinat Khabibiev (next)

comment:5 by Tim Graham, 8 years ago

My understanding is that the current pull request fix isn't correct then, right?

comment:6 by Rinat Khabibiev, 8 years ago

Yes, it isn't. I'm going to fix it soon.

comment:7 by Rinat Khabibiev, 8 years ago

Ready. Changed behavior: FetchFromCacheMiddleware sets Age header now.

comment:8 by Tim Graham, 8 years ago

Has patch: set
Triage Stage: Unreviewed → Accepted

​PR

comment:9 by Tim Graham, 8 years ago

Patch needs improvement: set

I left comments for improvement on the PR. Please uncheck "Patch needs improvement" after updating it.

comment:10 by Alexander Lazarević, 11 months ago

Owner: changed from Rinat Khabibiev to Alexander Lazarević

Created a new (draft) PR: ​https://github.com/django/django/pull/17726

This one contains not yet all recommended changes from the original PR: ​https://github.com/django/django/pull/7246

comment:11 by Alexander Lazarević, 10 months ago

Patch needs improvement: unset

I updated the PR

comment:12 by Alexander Lazarević, 10 months ago

It's not blocked by #35141 anymore

comment:13 by Mariusz Felisiak, 10 months ago

Summary: Cache-Control's max-age doesn't match Expires for responses taken from cache → "age" header is not set for responses taken from cache.

comment:14 by Mariusz Felisiak, 10 months ago

Triage Stage: Accepted → Ready for checkin

comment:15 by Mariusz Felisiak <felisiak.mariusz@…>, 10 months ago

Resolution: → fixed
Status: assigned → closed

In 3580b47e:

Fixed #27225 -- Added "Age" header when fetching cached responses.

Co-Authored-By: Author: Alexander Lazarević <laza@…>

Note: See TracTickets for help on using tickets.
Back to Top