Opened 9 years ago

Closed 13 months ago

#6727 closed Cleanup/optimization (fixed)

django.utils.cache.patch_cache_control enhancement when response has empty cache control header

Reported by: pascal@… Owned by: Dwight Gunning
Component: HTTP handling Version: master
Severity: Normal Keywords:
Cc: doug@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Ramiro Morales)

See source:/django/trunk/django/utils/cache.py :

60: 	    if response.has_header('Cache-Control'):
61: 	        cc = cc_delim_re.split(response['Cache-Control'])
62: 	        cc = dict([dictitem(el) for el in cc])

If a Cache-Control header exists but is empty, this will add a useless comma in the result.
See:

>>> cache_control_header=''
>>> import re
>>> cc_delim_re = re.compile(r'\s*,\s*')
>>> cc = cc_delim_re.split(cache_control_header)
>>> def dictitem(s):
...         t = s.split('=',1)
...         if len(t) > 1:
...             return (t[0].lower(), t[1])
...         else:
...             return (t[0].lower(), True)
... 
>>> cc = dict([dictitem(el) for el in cc])
>>> cc
{'': True}
>>> def dictvalue(t):
...     if t[1] is True:
...         return t[0]
...     else:
...         return t[0] + '=' + smart_str(t[1])
... 
>>> from django.utils.encoding import smart_str
>>> cc['max-age']='300'
>>> cc = ', '.join([dictvalue(el) for el in cc.items()])
>>> cc
', max-age=300'

Note that this is still a valid cache control header according to:

But It would be nicer to test if existing cache control response header is empty, so something like:

60: 	    if response.has_header('Cache-Control') and response['Cache-Control']:

Change History (11)

comment:1 Changed 9 years ago by Paul Egan

Cc: paulegan@… removed

comment:2 Changed 9 years ago by Jacob

Triage Stage: UnreviewedAccepted

comment:3 Changed 8 years ago by Ramiro Morales

Description: modified (diff)

comment:4 Changed 6 years ago by Julien Phalip

Severity: Normal
Type: Cleanup/optimization

comment:5 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:6 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:7 Changed 13 months ago by Dwight Gunning

Owner: changed from nobody to Dwight Gunning
Status: newassigned

comment:9 Changed 13 months ago by Dwight Gunning

Additional test created. Test suite passes.

Raised pull request: https://github.com/django/django/pull/5597

comment:10 Changed 13 months ago by Simon Charette

Has patch: set
Triage Stage: AcceptedReady for checkin

Patch LGTM

comment:11 Changed 13 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 1f29164:

Fixed #6727 -- Made patch_cache_control() patch an empty Cache-Control header.

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