Django fails to log in when a cookie is set on the same domain containing a colon

[Warlax]

From the release notes for the development version of django 1.2 I found this.

"""
Cookie encoding
To fix bugs with cookies in Internet Explorer, Safari, and possibly other browsers, our encoding of cookie values was changed so that the characters comma and semi-colon are treated as non-safe characters, and are therefore encoded as \054 and \073 respectively. This could produce backwards incompatibilities, especially if you are storing comma or semi-colon in cookies and have javascript code that parses and manipulates cookie values client-side.
"""

One bug we have found through using version 1.0 and 1.1 is that django also fails to understand cookies with a ":" in their name. This makes the login system completely fall over, so it mite be worth encoding those too. We discovered this bug when running django under the same domain as another tool, and it was settings cookies such as blah:wiki, and once those cookies were set upon that domain, we simply could not log back into the django section of the site without first clearing the offending cookies.

Hopefully I have provided enough information to help you fix this bug.

Ash

[Russell Keith-Magee]

A colon *should* be a valid value in a cookie; you'll need to provide a specific failing test case.

[Warlax]

Create a standard django app with the default auth system

create a view that is @login_required

MANUALLY create a cookie in ur browser under the url django is servered at, ie 127.0.0.1:8000

create this cookie
"org.ditchnet.jsp.tabs:task-details" with a value of anything, eg "231432423"

try and log into the django app. you will not be able to view the page behind the login required decorator

[Russell Keith-Magee]

I can't reproduce with the provided instructions. Like I said, a colon should be a valid value for a cookie. A programatic test case might help.

[Ubercore]

Test illustrating the cookie issue

[Ubercore]

I've added a test case to illustrate what I think is going on here. The culprit in my case was Glassfish running on the same server. Its admin console adds this cookie:

form:tree-hi=;

This breaks cookie parsing, and no cookies appear in the request. Including the csrf token. I think the ideal case here is to lose only the non-standard cookies, instead of returning a blank dict when a CookieError is raised. This is, I think, what Trac has done.

​http://bugs.python.org/issue2193

​http://trac.edgewall.org/ticket/2256

[Ubercore]

Patch based on Trac's solution to the problem

[Ubercore]

Without rolling some kind of cookie pre-processing, Trac's solution seems like a decent option until Python provides better hooks for error handling on individual keys. I've attached a patch based on ​http://trac.edgewall.org/browser/trunk/trac/web/api.py?rev=3734#L97.

[Thomas Ashelford]

Spent a bit of time reviewing this ticket during the sprint.

A few points:

• While "A colon *should* be a valid value in a cookie", this ticket is actually about cookies with a colon in the name. While there is some uncertainty about what characters are legal in cookie names, most folks seem to thing colons aren't kosher.
• In any case, I followed the instructions, and with some fiddling was able to reproduce the problem. I failed to reproduce it in Safari (it looked like maybe Safari 'censored' the cookie with the colon), but managed to definitively reproduce it in Firefox.
• The patch takes a reasonable, prudent approach, and solves the issue cited
• The unittest isn't exactly conclusive but I can't think of a better way to test this - it's not exactly an easy situation to reproduce

I say let check it in!

[Russell Keith-Magee]

This patch doesn't apply clean to trunk, and it's not obvious how to adapt the patch -- there have been other modifications to the cookie code in the interim.

[Ramiro Morales]

Patch attached to trunk as of r15399

[Ramiro Morales]

I've attached the patch to apply after r15298. Please review, particularly:

• Replaced a explicit SimpleCookie.load(self, rawdata) in the added load() method with a super(SimpleCookie, self).load(rawdata) call
• Added a runtime _cookie_allows_colon_in_names flag in the same spirit of the _morsel_supports_httponly and _cookie_encodes_correctly already there.

[Aymeric Augustin]

#7183 was a duplicate.

[Luke Plant]

Just a small note - 13007.1.diff is absolutely the right way to do it in trunk, but it will be a pain to backport to 1.2.X, due to [15298] and the fact that [14707] is a feature addition which is not backported to 1.2.X. So it looks like 13007.1.diff should be applied to trunk, 13007.diff to 1.2.X.

[Ramiro Morales]

In [15523]:

Fixed #13007 -- Made cookie parsing resilent to the presence of cookies with invalid characters in their names. Thanks Warlax for the report, Ubercore for his work on a fix and Jannis and Luke for review and guidance.

[Ramiro Morales]

In [15524]:

[1.2.X] Fixed #13007 -- Made cookie parsing resilent to the presence of cookies with invalid characters in their names. Thanks Warlax for the report, Ubercore for his work on a fix and Jannis and Luke for review and guidance.

Backport of [15523] from trunk.

[Jacob]

Milestone 1.3 deleted