#19705: CommonMiddleware handles If-None-Match incorrectly
-------------------------------+------------------------------------
Reporter: aaugustin | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by aaugustin):
* stage: Unreviewed => Accepted
Old description:
> Two middleware check ETags for unmodified responses: `CommonMiddleware`
> and `ConditionalGetMiddleware` and they do it inconsistently.
>
> If the response's ETag matches the request's If-Modified-Since:
> - `ConditionalGetMiddleware` changes the response code to 304, preserving
> all headers; the content gets removed later on
> - `CommonMiddleware` creates a new `HttpResponseNotModified` without
> 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. [http://tools.ietf.org/html/draft-ietf-
> httpbis-p1-messaging-21#section-3.3.2 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
> `ConditionalGetMiddleware`.
>
> For example, if one is using GzipMiddleware, the correct response chain
> is:
> - `CommonMiddleware` computes the ETag,
> - `GzipMiddleware` compresses the content and modifies the ETag,
> - `ConditionalGetMiddleware` uses 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.
>
> Unfortunately, `CommonMiddleware` is documented to "take care of sending
> Not Modified responses, if appropriate", so this would be a backwards
> incompatible change.
New description:
Two middleware check ETags for unmodified responses: `CommonMiddleware`
and `ConditionalGetMiddleware` and they do it inconsistently.
If the response's ETag matches the request's If-None-Match:
- `ConditionalGetMiddleware` changes the response code to 304, preserving
all headers; the content gets removed later on
- `CommonMiddleware` creates a new `HttpResponseNotModified` without
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. [http://tools.ietf.org/html/draft-ietf-
httpbis-p1-messaging-21#section-3.3.2 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
`ConditionalGetMiddleware`.
For example, if one is using GzipMiddleware, the correct response chain
is:
- `CommonMiddleware` computes the ETag,
- `GzipMiddleware` compresses the content and modifies the ETag,
- `ConditionalGetMiddleware` uses 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.
Unfortunately, `CommonMiddleware` is documented to "take care of sending
Not Modified responses, if appropriate", so this would be a backwards
incompatible change.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/19705#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
