Initialize bad_cookies for redefined SimpleCookie #1605

wants to merge 5 commits into


None yet

2 participants

Django member

Could you add a regression test for thtis bug? It looks like there are some in tests/httpwrappers/


OK, I'll do it.

Django member

It looks to me like the test passes before your fix is applied.

Django member

The test doesn't pass on Python 3.3:

ERROR: test_bad_cookie (httpwrappers.tests.CookieTests)
Traceback (most recent call last):
  File "/django/tests/httpwrappers/", line 620, in test_bad_cookie
    r.set_cookie("a:.b/", 1)
  File "/django/django/http/", line 220, in set_cookie
    self.cookies[key] = value
  File "/usr/lib/python3.3/http/", line 487, in __setitem__
    self.__set(key, rval, cval)
  File "/usr/lib/python3.3/http/", line 481, in __set
    M.set(key, real_value, coded_value)
  File "/usr/lib/python3.3/http/", line 364, in set
    raise CookieError("Illegal key value: %s" % key)
http.cookies.CookieError: Illegal key value: a:.b/

We don't replace SimpleCookie for Python 3.3 and SimpleCookie in new Python doesn't have 'bad_cookies' set. Need to investigate more and update patch


I updated patch, but I'm not sure that it is good solution

@timgraham timgraham commented on an outdated diff Sep 9, 2013
@@ -611,3 +611,11 @@ def test_load_dict(self):
c = SimpleCookie()
c.load({'name': 'val'})
self.assertEqual(c['name'].value, 'val')
+ def test_bad_cookie(self):
+ r = HttpResponse()
+ r.set_cookie("a:.b/", 1)
+ if (hasattr(r.cookies, 'bad_cookies')):
timgraham Sep 9, 2013 Django member

I'd probably use if six.PY2 here, so we remember to remove this branch when we drop Python 2 compatability

Django member

I'm not sure if catching the exception on Python 3 is the right thing to do either. I've left a comment on the ticket in hopes someone else will offer an opinion. Thanks for your work on this thus far!

Django member

After thinking about it, I don't think we should silence Python 3 exceptions while fixing this issue. We'll only run the new test on Python 2.

merged in f2a4452 - thanks!

@timgraham timgraham closed this Sep 10, 2013

Thanks for feedback and help in fixing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment