#19705: CommonMiddleware handles If-None-Match incorrectly
-------------------------------+------------------------------------
Reporter: aaugustin | Owner: syphar
Type: Bug | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by marfire):
* cc: k@… (added)
Comment:
At the moment there are several interrelated problems with the way Django
handles ETags. Briefly:
* As stated in the original report above, we're not returning the correct
headers with our `304s`.
* Gzipped responses are not getting the benefits of `304s` since their
ETags are not compared properly. See tickets #16035 and #26771.
* Our processing of conditional requests does not follow the
specification's rewritten
[[https://tools.ietf.org/html/rfc7232#section-6|precedence rules]] from
2014.
* The `condition()`, `vary*()`, `cache_control()`, and `gzip_page()`
decorators work properly in some undocumented orders but not in others.
Here are the changes I would suggest to address these problems:
* Move ETag handling out of `CommonMiddleware` and into
`ConditionalGetMiddleware`. This change has already been accepted (#26447)
and partially implemented ([[https://github.com/django/django/pull/6393|PR
6393]]). (Though I'm less confident about the deprecation of `USE_ETAGS`
in that ticket.)
* Change `GZipMiddleware` to remove the `;gzip` token from the incoming
ETags in `process_request()`. That will allow comparisons to work
properly. Document this.
* Change the `304` headers to match the specification.
* Change our conditional view logic to match the (simpler) specified
precedence rules.
* Change our use of `GzipFile` to specify a modification time of 0. That
will make our gzip output dependent only on the response body, which means
that we can usefully compare ETags on gzipped content, which means that we
don't have to care about the order of the `gzip_page()` decorator.
* Document that the `condition()` decorator should be below `vary()` and
`cache_control()` so that those headers can be set properly.
In more detail:
[[https://code.djangoproject.com/ticket/19705#comment:12|mrmachine's
comment]] raises a fair point. The fundamental issue here is that Django's
architecture strives to be layered and decoupled, but ETags are by
definition highly coupled to the response, so it's difficult to isolate
them to one layer (whether that be middleware or a decorator).
That said, [[https://tools.ietf.org/html/rfc7232#section-2.1|the
specification]] only requires that the ETag change when the response body
changes, and I don't think that's a very common middleware behavior. Only
`GZipMiddleware` meets that definition among the middleware in core, for
example. And if the body ''is'' changed, the ETag can be changed in a
manner similar to what I'm suggesting for `GZipMiddleware`. I do think
this needs to be documented, though.
Regarding which headers to send back with `304s`,
[[https://tools.ietf.org/html/rfc7232#section-4.1|the specification]] says
this:
> 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`. Since the goal of a 304 response is to minimize information
transfer when the recipient already has one or more cached
representations, a sender SHOULD NOT generate representation metadata
other than the above listed fields unless said metadata exists for the
purpose of guiding cache updates (e.g., `Last-Modified` might be useful if
the response does not have an `ETag` field).
So I think we should return those headers from the response, along with
the cookies (since that is what Django already does, and since it's
[[https://bz.apache.org/bugzilla/show_bug.cgi?id=18388|apparently very
common]] despite not being mandated by the standard).
I'm able to work on this, but the next step depends on the fate of
[[https://github.com/django/django/pull/6393|PR 6393]] , since that
accomplishes some of the reorganization mentioned above.
--
Ticket URL: <https://code.djangoproject.com/ticket/19705#comment:14>
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 [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/067.224b38f84dac91d967d222c9a58b124e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
