Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

304 responses changes expire time of resource #230

Closed
markwoon opened this issue May 14, 2019 · 6 comments
Closed

304 responses changes expire time of resource #230

markwoon opened this issue May 14, 2019 · 6 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@markwoon
Copy link
Contributor

Problem:

  1. Expiration time is currently determined by content-type.
  2. Content-type is not set for 304 responses.
  3. This means 304 responses gets the default expiration time, which could be very different from the original.
@LeoColomb LeoColomb added bug Something isn't working help wanted Extra attention is needed labels May 14, 2019
@LeoColomb
Copy link
Member

LeoColomb commented May 14, 2019

Indeed. Good catch @markwoon!

-> GET http://server.localhost/test.json

<- HTTP/1.1 200 OK
Server: nginx
Date: Tue, 14 May 2019 22:24:23 GMT
Content-Type: application/json; charset=utf-8
Last-Modified: Sat, 09 Mar 2019 11:21:52 GMT
Transfer-Encoding: chunked
Connection: keep-alive
Vary: Accept-Encoding
ETag: W/"5c83a1d0-117"
Expires: Tue, 14 May 2019 22:24:23 GMT
Cache-Control: max-age=0
X-Content-Type-Options: nosniff

-> GET http://server.localhost/test.json
If-None-Match: W/"5c83a1d0-117"

<- HTTP/1.1 304 Not Modified
Server: nginx
Date: Tue, 14 May 2019 22:25:06 GMT
Last-Modified: Sat, 09 Mar 2019 11:21:52 GMT
Connection: keep-alive
ETag: "5c83a1d0-117"
Expires: Thu, 13 Jun 2019 22:25:06 GMT
Cache-Control: max-age=2592000
X-Content-Type-Options: nosniff

Sending a Cache-Control is expected for a 304:

The server generating a 304 response MUST generate any of the
following header fields that would have been sent in a 200 (OK)
response to the same request: Cache-Control, Content-Location, Date,
ETag, Expires, and Vary.
https://tools.ietf.org/html/rfc7232#section-4.1

But the miss of Content-Type seems to set the expiration delay to the default one, indeed.

Looking for a solution, any suggestion is welcomed.

@LeoColomb
Copy link
Member

LeoColomb commented May 15, 2019

One solution is to add an empty condition to the $expires map:

map $sent_http_content_type $expires {
  default  1M;
  ""       off; # <--
  ...
}
-> GET http://server.localhost/test.json
If-None-Match: W/"5c83a1d0-117"

<- HTTP/1.1 304 Not Modified
Server: nginx
Date: Wed, 15 May 2019 00:16:42 GMT
Last-Modified: Sat, 09 Mar 2019 11:21:52 GMT
Connection: keep-alive
ETag: "5c83a1d0-117"
X-Content-Type-Options: nosniff

But we lose Cache-Control for empty Content-Type and for 304.

@LeoColomb
Copy link
Member

LeoColomb commented May 15, 2019

@markwoon
Copy link
Contributor Author

That works for me. I don't think there's much else we can do here.

I think my bigger issue is that there's no way to add "must-revalidate" when using expires, although that's a different issue.

@LeoColomb
Copy link
Member

@markwoon Thanks.

there's no way to add "must-revalidate"

Did you try to add must-revalidate the same way we did with no-transform?

@markwoon
Copy link
Contributor Author

Did you try to add must-revalidate the same way we did with no-transform?

Oops, my bad. I got bit by how add_header works when you use it at different scoping levels. I thought expires was somehow blocking the addition of must-revalidate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants