plack / Plack

PSGI toolkit and server adapters
http://plackperl.org/
Other
486 stars 214 forks source link

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

Open robrwo opened 3 years ago

robrwo commented 3 years ago

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 commented 3 years ago

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

robrwo commented 3 years ago

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

miyagawa commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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