[Django] #36688: GzipMiddleware adds 'Vary: Accept-Encoding' unnecessarily

[Django]

#36688: GzipMiddleware adds 'Vary: Accept-Encoding' unnecessarily
-----------------------------------------+------------------------------
Reporter: Adam Johnson | Owner: Adam Johnson
Type: Bug | Status: assigned
Component: HTTP handling | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------------
`GZipMiddleware` has an early-return path when the compressed content is
longer than the original, to save bandwidth. Unfortunately, in this case,
it still adds `Vary: Accept-Encoding`, slightly degrading HTTP cache
performance.

The fix is to move the `patch_vary_headers()` call later.
--
Ticket URL: <https://code.djangoproject.com/ticket/36688>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36688: GzipMiddleware adds 'Vary: Accept-Encoding' unnecessarily

-------------------------------+----------------------------------------

Reporter: Adam Johnson | Owner: Adam Johnson
Type: Bug | Status: assigned
Component: HTTP handling | Version: dev

Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------+----------------------------------------
Comment (by Adam Johnson):

Wait, on second thought, I might be wrong. Just because on a given run of
a view, we couldn't compress the content, doesn't mean we won't be able to
in another run.. not sure on HTTP semantics actually.
--
Ticket URL: <https://code.djangoproject.com/ticket/36688#comment:1>

[Django]

#36688: GzipMiddleware adds 'Vary: Accept-Encoding' unnecessarily
-------------------------------+----------------------------------------
Reporter: Adam Johnson | Owner: Adam Johnson

Type: Bug | Status: closed

Component: HTTP handling | Version: dev

Severity: Normal | Resolution: invalid

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+----------------------------------------

Changes (by Adam Johnson):

* resolution: => invalid
* status: assigned => closed

Comment:

Okay, I'm wrong. `Vary: Accept-Encoding` should be sent anyway, because
the `Accept-Encoding` header is inspected, regardless of compression
outcome. I checked this with Claude, by asking its opinion and getting it
to test the behaviour of nginx and caddy with incompressible random data.
[https://gist.github.com/adamchainz/17c22f9aa87ce821972186964ce3d310 log
in this GIST]. (Oddly nginx requires `gzip_vary on` to even set the `Vary`
header, which seems wrong.)

But I think something is wrong then that we don't add the `Vary` header
before the *first* early-return clause:

{{{#!python
if not response.streaming and len(response.content) < 200:
return response
}}}

Just because the length was too small doesn't mean we wouldn't compress
the same URL when it sends larger values, as size may be dynamic. But I
think this is minor and I will reserve reporting it as a bug for a future
larger proposal to refactor the whole middleware to support zstd too,
based on [https://github.com/adamchainz/django-http-compression django-
http-compression].
--
Ticket URL: <https://code.djangoproject.com/ticket/36688#comment:2>