New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
http.cookies: Handle malformed cookie #61542
Comments
One of my user told me that she couldn't login to my website yesterday. I logged her cookie, and found it began with ',BRIDGE_R=;' which was a malformed cookie. In that case, Tornado has to treat her as a new user since it believes she didn't provide any cookies. I checked bpo-2193 and found the patch provided by spookylukey could fix the bug, but it was rejected. I believe SimpleCookie is useless for handling malformed cookies right now. |
Code behaving as documented is not a bug for tracker purposes. Adding a parameter to allow new behavior is an enhancement for a future release. Who is responsible for the invalid cookie. Pardon my ignorance, but if tornado re-sets the cookie, why cannot it read it the next time? If the existing test suite tests for CookieError for invalid cookies, writing tests for strict=False (return instead of CookieError) would be trivial. |
Terry, say that a user's cookie is ",BRIDGE_R=; a=b;" right now. The next time he sends cookie to the server, Cookie.SimpleCookie.load() tries to parse the cookie, but raises a CookieError. I cannot clear all cookies because Cookie.SimpleCookie.load() even dosen't let me know the keys in his cookie. |
keakon, changing the headers after a developer sets them is insulting, annoying, a waste of my time to change them back again, and a distraction from the issue. |
Terry, I think that's the standard process of web applications.
After the 4 steps, the user agent should be considered as a logged-in user. I don't think there is anything wrong with the process except the strange behavior of Cookie.SimpleCookie.load(). |
Just a quick note that the new specification for HTTP State Mechanism (aka cookies) is http://tools.ietf.org/html/rfc6265 keakon, Do you know why her cookie was ',BRIDGE_R=;' |
karl, I don't know the exact reason. The Baidu Bridge is an external JavaScript resource. It can do anything like: document.cookie = ",BRIDGE_R=;"; |
Carl, do you know if the (2 year old) draft better reflect actual usage than 2965? Is there much change other than "deprecates the use of the Cookie2 and Set-Cookie2 header fields."? |
Yes the new RFC has been written by Adam Barth who wanted to describe things matching the reality of HTTP and servers/browsers issues. |
I believe our normal policy is to only follow accepted RFCs. But your comment suggests that in this case we should pay attention to the new draft. Do you have any idea why apparently nothing has happened in two years. Do some people oppose it? |
I'm a core developer on Django, and I've looked into cookies a lot, and also Python's SimpleCookie, and I've found that all accepted RFCs are completely irrelevant for this issue. No accepted RFC was ever widely implemented - instead browsers mainly did something like the original "Netscape cookies", with various interpretations. Opera attempted RFC 2965, at least at one point, but no-one else. RFC 6265, whatever its status, is probably the closest thing to a useful document of how cookies "should" work. But even then, I'm afraid that the main guiding principle has to be sheer pragmatism. Read the source code or bug trackers of any other project that has to handle cookies and you'll find they have all come to that conclusion, unfortunately. |
The current status of RFC6265 is PROPOSED STANDARD Adam Barth is part of the Google Chrome Team. I do not want to talk for Adam. So better ask him, I don't think he has the energy/will to push further through the IETF process. |
The current Python 3.5 and default branches actually seem to parse the test case given: >>> c = SimpleCookie()
>>> c.load(",BRIDGE_R=; a=b; user_id=1;")
>>> c
<SimpleCookie: ,BRIDGE_R='' a='b' user_id='1'> But that is just a side effect of bpo-26302. When that is fixed, parsing the cookie string will raise CookieError and fail to set the invalid cookie “morsel”, and the ones that come after it. There seems to be a disconnect between _LegalChars (causes the CookieError if a comma is in a cookie key name) and _LegalKeyChars (allows a comma, but causes cookie string parsing to silently abort for other illegal characters). There are other cases where the entire cookie string is rejected, specifically added by bpo-22796 (revision a065ab1c67a8). On the other hand, bpo-25228 has a which has a patch to skip over some invalid cookie “morsels” and continue on to valid ones. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: