CommonMiddleware handles If-None-Match incorrectly
|Reported by:||Aymeric Augustin||Owned by:||Denis Cornehl|
|Cc:||hirokiky@…, real.human@…, k@…||Triage Stage:||Accepted|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
Description (last modified by )
Two middleware check ETags for unmodified responses:
ConditionalGetMiddleware and they do it inconsistently.
If the response's ETag matches the request's If-None-Match:
ConditionalGetMiddlewarechanges the response code to 304, preserving all headers; the content gets removed later on
CommonMiddlewarecreates a new
HttpResponseNotModifiedwithout content and simply restores the cookies.
As a consequence,
CommonMiddleware returns a response without ETag, which is wrong. I detected this with RedBot on a Django site I run. Any site with
USE_ETAGS = True has this problem.
In general, wiping headers sounds like a bad idea. A 304 is supposed to have the same headers as the 200. (Well, the RFC is more complicated, but I think it's the general idea. Future versions of HTTP will likely require the Content-Length not to be 0.)
I believe that
CommonMiddleware should simply generate the ETag and not handle conditional content removal; that's the job of
For example, if one is using GzipMiddleware, the correct response chain is:
CommonMiddlewarecomputes the ETag,
GzipMiddlewarecompresses the content and modifies the ETag,
ConditionalGetMiddlewareuses the modified ETag to decide if the response was modified or not.
This is a good reason to keep "ETag generation" and "Etag checking" concerns separate. The same argument applies to any middleware that sets or modifies ETags.
CommonMiddleware is documented to "take care of sending Not Modified responses, if appropriate", so this would be a backwards incompatible change.
Change History (24)
comment:1 Changed 4 years ago by
|Summary:||CommonMiddleware handles If-Modified-Since incorrectly → CommonMiddleware handles If-None-Match incorrectly|
|Triage Stage:||Unreviewed → Accepted|
comment:3 Changed 4 years ago by
|Owner:||changed from nobody to Hiroki Kiyohara|
|Status:||new → assigned|