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

ConditionalGET expects both If-None-Match and If-Modified-Since headers #665

Open
robrwo opened this issue Mar 25, 2021 · 6 comments
Open

Comments

@robrwo
Copy link
Contributor

robrwo commented Mar 25, 2021

The comment

      # check both ETag and If-Modified-Since, and at least one should exist
      # and all present headers should match, not either.

is incorrect.

According to the RFC 7232

A recipient MUST ignore If-Modified-Since if the request contains an
If-None-Match header field; the condition in If-None-Match is
considered to be a more accurate replacement for the condition in
If-Modified-Since, and the two are only combined for the sake of
interoperating with older intermediaries that might not implement
If-None-Match.

If the resource response contains both ETag and Last-Modified headers, and the request has an If-None-Match header but no If-Modified-Since, then it will not return HTTP 304 when it should.

@robrwo
Copy link
Contributor Author

robrwo commented Mar 25, 2021

Additionally, it should compare the dates if the strings do not match.

@robrwo
Copy link
Contributor Author

robrwo commented Mar 25, 2021

It looks like there are several issues with ConditionalGET. I'll work on seperate PRs:

  • Should only apply to HTTP 200
  • Should handle weak comparison with ETag
  • Should handle multiple tags in If-None-Match

@miyagawa
Copy link
Member

Additionally, it should compare the dates if the strings do not match.

Not sure if this is exactly what you mean, but the RFC says it's expected to use an exact match:

When handling an If-Modified-Since header field, some servers will use an exact date comparison function, rather than a less-than function, for deciding whether to send a 304 (Not Modified) response. To get best results when sending an If-Modified-Since header field for cache validation, clients are advised to use the exact date string received in a previous Last-Modified header field whenever possible.

@robrwo
Copy link
Contributor Author

robrwo commented Mar 25, 2021

I'm unsure that it's worth parsing the timestamp when they don't match. However, it's recommended that UAs both If-None-Match and If-Modified-Since as a fallback. (It looks like Googlebot and Chrome do this, for example.)

By changing it to check If-None-Match when it exists, and not checking If-Modified-Since at all, we avoid the extra hit from mismatched timestamps. But if a UA does want to use it (for example, using the timestamp of when a file was retrieved and saved in a mirror, instead of a Last-Modified timestamp) then it will still work properly.

@robrwo
Copy link
Contributor Author

robrwo commented Mar 29, 2021

There's another complication, at least with Apache and HTTP2 with gzip encoding.

If I make a request with "Accept-Encoding: gzip" then Apache is adding a "-gzip" suffix th the etag. So the UA will send the modified ETag back, (This is apparently caused when fixing Plack::Middleware::ETag's malformed ETags.)

I'm not sure if there's anything practical that we can do for that case. See https://bz.apache.org/bugzilla/show_bug.cgi?id=39727

@miyagawa
Copy link
Member

Not much you can do, except maybe write an inline middleware to strip that part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants