Opened 3 weeks ago
Closed 3 weeks ago
#36688 closed Bug (invalid)
GzipMiddleware adds 'Vary: Accept-Encoding' unnecessarily
| Reported by: | Adam Johnson | Owned by: | Adam Johnson |
|---|---|---|---|
| Component: | HTTP handling | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
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.
Change History (2)
comment:1 by , 3 weeks ago
comment:2 by , 3 weeks ago
| Resolution: | → invalid |
|---|---|
| Status: | assigned → closed |
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. 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:
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 django-http-compression.
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.