Opened 8 years ago

Closed 3 weeks 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: dwightgunning
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)

See source:/django/trunk/django/utils/ :

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.

>>> 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 8 years ago by paulegan

  • Cc paulegan@… removed
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 8 years ago by jacob

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 7 years ago by ramiro

  • Description modified (diff)

comment:4 Changed 5 years ago by julien

  • Severity set to Normal
  • Type set to Cleanup/optimization

comment:5 Changed 4 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:6 Changed 4 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:7 Changed 3 weeks ago by dwightgunning

  • Owner changed from nobody to dwightgunning
  • Status changed from new to assigned

comment:9 Changed 3 weeks ago by dwightgunning

Additional test created. Test suite passes.

Raised pull request:

comment:10 Changed 3 weeks ago by charettes

  • Has patch set
  • Triage Stage changed from Accepted to Ready for checkin

Patch LGTM

comment:11 Changed 3 weeks ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

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