Opened 5 years ago

Closed 5 years ago

#30701 closed Cleanup/optimization (fixed)

Allowing patch_vary_headers() caching utility to handle '*' value.

Reported by: Alexander-TX Owned by: Adnan Umer
Component: Core (Cache system) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Function "patch_vary_headers", simply appends new headers to list. If view code sets Vary header to asterisk, the resulting header (after applying SessionMiddleware and LocaleMiddleware) looks like this:

Vary: *, Accept-Language, Cookie

This is unnecessary and possible violates HTTP spec:

The "Vary" header field in a response describes what parts of a
   request message, aside from the method, Host header field, and
   request target, might influence the origin server's process for
   selecting and representing this response.  The value consists of
   either a single asterisk ("*") or a list of header field names
   (case-insensitive).

     Vary = "*" / 1#field-name

(from ​https://tools.ietf.org/html/rfc7231#page-70)

I am using Django to implement REST API, so I'd like it to speak robust HTTP, that works with all present and future caching libraries, — even if widely used browsers and Nginx can correctly interpret current form of the header.

Change History (8)

comment:1 by Carlton Gibson, 5 years ago

Component: HTTP handling → Core (Cache system)
Summary: Django generates overly verbose and possibly invalid Vary headers → Allowing patch_vary_headers() caching utility to handle '*' value.
Triage Stage: Unreviewed → Accepted
Type: Bug → New feature

Hi Alexander. Thanks for the report.

Happy to accept this as an enhancement. (It's clear from the existing test cases that '*' was never considered.)

I was going to uncheck "Easy Pickings" but perhaps it is simple enough...

comment:2 by Adnan Umer, 5 years ago

Owner: changed from nobody to Adnan Umer
Status: new → assigned

comment:3 by Adnan Umer, 5 years ago

​PR

comment:4 by Mariusz Felisiak, 5 years ago

Needs documentation: set
Version: 2.2 → master

comment:5 by Adnan Umer, 5 years ago

Needs documentation: unset

comment:6 by Mariusz Felisiak, 5 years ago

Has patch: set
Type: New feature → Cleanup/optimization

comment:7 by Carlton Gibson, 5 years ago

Triage Stage: Accepted → Ready for checkin

PR Looks good.

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: → fixed
Status: assigned → closed

In 6805c0f9:

Fixed #30701 -- Updated patch_vary_headers() to handle an asterisk according to RFC 7231.

Note: See TracTickets for help on using tickets.
Back to Top