Python 3.3 fails unit test for duplicate bad cookies
|Reported by:||Ian Clelland||Owned by:||nobody|
|Severity:||Release blocker||Keywords:||cookie, python3.3|
|Has patch:||no||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
In #15852, a user was encountering problems with a cookie that contained multiple copies of a morsel with a ":" character in it, and the Django-patched SimpleCookie class was throwing an error.
The solution to that problem was to further patch SimpleCookie to keep track of the bad morsel properly and not throw an exception. A unit test was put in place, which used "," as a bad-morsel-triggering character.
Unfortunately, two things have happened with Python 3.3 (Both of these should be good things, but they end up with an awkward regression):
First, the Python standard library version of http.cookie.SimpleCookie passes our tests, so Django doesn't try to patch it. This means that the patch put in place does not run, and the unit test fails.
Second, ':' is no longer considered an invalid character in cookies (see http://bugs.python.org/issue2193 for the four-year-long discussion). While this is good for the original problem -- those cookies can now be handled properly -- the patch actually changes the behaviour for other invalid characters, and people may be relying on that behaviour. (Is this covered by the backwards-incompatibility guarantee?)
I see a couple ways out of this -- we could:
- Check in django.http whether SimpleCookie skips bad morsels, and patch SimpleCookie if it doesn't. Practically, I think this means that we'll be patching SimpleCookie forever, unless we can convince the Python developers that it's a bug.
- Change the unit test to only make sure that it handles duplicate keys with ":" properly, like the original user needed, possibly leaving other the handling of other invalid cookies in an undetermined state (https://code.djangoproject.com/ticket/15852#comment:1 seems to claim that the Python handling of invalid cookies is acceptable)
or 3. Delete the unit test -- it advertises that we handle bad cookie morsels in a graceful way, which we definitely do not do if we use the built-in SimpleCookie. We call this a bug in Python, and just declare that "it's just how Python SimpleCookie works"
Change History (6)
comment:1 Changed 4 years ago by
|Patch needs improvement:||unset|
comment:2 Changed 4 years ago by
|Severity:||Normal → Release blocker|
|Triage Stage:||Unreviewed → Accepted|