[issue17340] Handle malformed cookie
Luke Plant added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17340 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2193] Cookie Colon Name Bug
Luke Plant l.plant...@cantab.net added the comment: David, Thanks again for the time on this. Can I push to get the patches included, or is there work that still needs to be done on the patches now that the idea is accepted in principle? I did experiment with a few approaches to implement, and it seemed like the most sensible. Thanks, Luke -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2193 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2193] Cookie Colon Name Bug
Luke Plant l.plant...@cantab.net added the comment: First, I agree with others who say that RFCs are basically irrelevant for cookies. For Django we've discovered this in various ways e.g. issue 9824 - http://bugs.python.org/issue9824 - which has now been applied. We have also had to work around the stdlib behaviour here. Second, I have implemented a patch for this, with tests, against trunk - please review. After looking at the implementation, this seems like the best way to make Python conservative in what is produces and liberal in what it accepts, which seems to be what the thread converged on. BaseCookie will now silently discard cookie 'morsels' with a colon in their name (and all other irregularities) when loading from a string, rather than raise an exception. This allows cookie parsing to continue, so that other cookies in the HTTP header will be found. However, if in Python code you attempt to directly set a morsel with an illegal name, you will still get the error. There is a more lax strategy: Simply add ':' to the _LegalChars variable. This would allow morsels to be *read* that have a colon in their name. However, from the current implementation, it would be very hard to add that ability without also allowing the BaseCookie class to produce such cookies. This would also raise other issues about at what point an error should be raised for setting invalid cookies etc. Also, allowing these illegal cookies to be read is a corner case that is much less important - it isn't needed either for Trac or for our needs in Django. For these reasons, I decided against the more lax strategy. -- keywords: +patch nosy: +spookylukey Added file: http://bugs.python.org/file22513/issue2193_patch_trunk.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2193 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2193] Cookie Colon Name Bug
Luke Plant l.plant...@cantab.net added the comment: Same patch backported to python 2.7 branch -- Added file: http://bugs.python.org/file22514/issue2193_patch_python27.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2193 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2193] Cookie Colon Name Bug
Luke Plant l.plant...@cantab.net added the comment: Found a bug with patch - this supersedes old one. -- Added file: http://bugs.python.org/file22515/issue2193_patch_2_trunk.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2193 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2193] Cookie Colon Name Bug
Luke Plant l.plant...@cantab.net added the comment: Same against Python 2.7 -- Added file: http://bugs.python.org/file22516/issue2193_patch_2_python27.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2193 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2193] Cookie Colon Name Bug
Changes by Luke Plant l.plant...@cantab.net: Removed file: http://bugs.python.org/file22513/issue2193_patch_trunk.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2193 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2193] Cookie Colon Name Bug
Changes by Luke Plant l.plant...@cantab.net: Removed file: http://bugs.python.org/file22514/issue2193_patch_python27.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2193 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2193] Cookie Colon Name Bug
Luke Plant l.plant...@cantab.net added the comment: I had a quick look, and there are these relevant bits: There are two audiences for this specification: developers of cookie-generating servers and developers of cookie-consuming user agents. And: To maximize interoperability with user agents, servers should limit themselves to the well-behaved profile defined in Section 4 when generating cookies. So, the document doesn't tell servers how to parse cookies, only how to generate them. With regards to generation, there is basically no change - we still disallow programmers to set cookie names that are not a 'token', as defined by section 4 of that document, which is the same as RFC 2109 in terms of valid cookie names if you look at it. It is not obvious to me that Python's BaseCookie implementation obeys RFC 2109 (due to the way character lists are defined in the opposite way), but if you believe the comments in the module then it does. I haven't read the rest of RFC 6265 and checked BaseCookie against it - that would be a much bigger job. But with respect to the change in my patch, it looks like we are all OK. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2193 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2193] Cookie Colon Name Bug
Luke Plant l.plant...@cantab.net added the comment:
@ David Murray:
Thanks for taking the time to look at this - can I trouble you to keep going
and read my response?
Thanks.
You wrote:
IMO the thing that needs to be fixed here is that receiving an invalid cookie
makes it difficult to receive the valid cookies.
I agree absolutely, and my patch implements exactly that aim. So I don't
understand why the rest of your reply goes on to assume a very different goal -
handling RFC-invalid cookies such that we can read their values.
I'd love to accept your patch, but silently ignore sounds like a bad
idea and is something we try to avoid in Python where practical.
silently ignore is what the current BaseCookie implementation does for
**every other** type of invalid input, with the only exception (I can find)
being the case where everything is correct apart from the name:
from Cookie import SimpleCookie
c = SimpleCookie()
c.load('rubbish')
c
SimpleCookie:
c.output()
''
c.load('more:rubbish')
c
SimpleCookie:
c.load('name=value')
c
SimpleCookie: name='value'
c.load('name=value; invalidattribute;')
c.output()
'Set-Cookie: name=value'
c.load('xyz123sfs;;=-abc')
c
SimpleCookie: name='value'
c.load('namewith:colon=value')
Traceback (most recent call last):
File stdin, line 1, in module
File /usr/lib/python2.7/Cookie.py, line 632, in load
self.__ParseString(rawdata)
File /usr/lib/python2.7/Cookie.py, line 665, in __ParseString
self.__set(K, rval, cval)
File /usr/lib/python2.7/Cookie.py, line 585, in __set
M.set(key, real_value, coded_value)
File /usr/lib/python2.7/Cookie.py, line 460, in set
raise CookieError(Illegal key value: %s % key)
Cookie.CookieError: Illegal key value: namewith:colon
As you said, I think this ticket is about fixing that last one, which is the
gross exception to the rule, and the only thing that is causing major problems
for people.
I agree that handling cookies with invalid names so that we can read them would
be nice, but I think it is **very** difficult to find a rationale for saying
that this bug fix should have to wait for that change.
In addition, I do not think there is any sensible way to implement that
suggestion given the API of BaseCookie at the moment - it's not just the
implementation I baulked at, it is the API of BaseCookie that works against you
every step of the way.
The API of BaseCookie works like this in the typical case:
Consuming:
input - load()- store in BaseCookie - __getitem__()
Generating:
input - __setitem__() - store in BaseCookie - output()
(Of course you don't have to do it that way, but looking at the docs and
examples, http://docs.python.org/library/cookie.html, it's very clear that it's
meant to be used that way).
The fact that both modes involves storing in BaseCookie really messes up any
attempt to make these two work differently, and means you would have a really
surprising and strange API whichever way you do it.
So, option (1): you could allow BaseCookie to store invalid cookie names if
they come via load, but not via __setitem__(). This means that you've got an
easy work-around for BaseCookie refusing to store your value - just use
'load()' instead. It also means that simple code like this fails:
c['name:val'] = c['name:val']
which can be a problem if you are trying to do some generalised processing of
the contents of a BaseCookie.
This leads us to option (2): allow RFC-invalid cookies to be stored, but then
quietly sanitise (i.e. discard) in output().
But this becomes a much worse example of silently ignoring - the former is
tolerant handling of data that is outside the programmers control - Postel's
law. But this would be accepting incorrect data from a programmer, and then
silently discarding it, is what we should do our utmost to avoid.
Neither of these options is any good, and it is because the API of BaseCookie
was just not designed for it. The only sensible things to do, given our API, is
sanitise on input. We could have an 'RFC-invalid' mode, but it would have to be
turned on for the whole Cookie instance, and it would be a feature addition. An
alternative implementation would be a 'badmorsels' attribute which preserves
the rubbish where possible, in case you want it - but again, it's a feature
addition.
Finally, I think the behaviour you are aiming at is unreasonable to ask for,
especially in this ticket. You are essentially asking for a tolerant kind of
cookie parsing which does its best to do 'do-what-I-mean'. But:
1) You haven't specified further than that (and there are no RFCs of use)
2) in general that kind of thing is notoriously hard to get right
3) the job is never finished - there are always more cases of invalid cookies
that you *could* handle
4) and in fact it is impossible to provide an implementation that pleases
everyone - there will always be invalid cookies that were 'meant' to be one
thing, but the implementation
[issue11001] Various obvious errors in cookies documentation
New submission from Luke Plant l.plant...@cantab.net: Docs for SimpleCookie, BaseCookie.value_encode and BaseCookie.value_decode are obviously incorrect. Attempt at patch attached. The error has existed in every Python version I've seen, I've tagged the ones I believe can receive fixes, sorry if I've made a mistake. Thanks. -- assignee: docs@python components: Documentation files: http_cookies_doc_corrections.diff keywords: patch messages: 126962 nosy: docs@python, spookylukey priority: normal severity: normal status: open title: Various obvious errors in cookies documentation versions: Python 2.7, Python 3.1, Python 3.2 Added file: http://bugs.python.org/file20507/http_cookies_doc_corrections.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11001 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9824] SimpleCookie should escape commas and semi-colons
New submission from Luke Plant l.plant...@cantab.net: In developing Django, we found that some browsers don't treat commas and semi-colons in cookie values (i.e. the Set-Cookie header) the way that RFC 2109 says they should. (Safari splits the header on a comma followed by space, Internet Explorer splits on semi-colons - both irrespective of any 'quoting'). The result is that if you use SimpleCookie to create Set-Cookie headers, where the cookie value contains a comma or semi-colon, you can get all kinds of breakage. In the end, we realised that the RFCs are kind of irrelevant, and we have to look at what browsers actually do. So, it would be much more useful if semi-colons and commas were escaped the way that other characters are by SimpleCookie. Our discussion/findings are here: http://code.djangoproject.com/ticket/12470#comment:4 http://groups.google.com/group/django-developers/msg/2cb729938e8e67ca The patch to Cookie.py (Python 2.X) or http/cookies.py (Python 3.X) is simple and follows. I'm assuming that this applies to Python 3.2 and 3.3, but I haven't checked. -- components: Library (Lib) files: simplecookie_fix.diff keywords: patch messages: 116030 nosy: spookylukey priority: normal severity: normal status: open title: SimpleCookie should escape commas and semi-colons type: behavior versions: Python 2.5, Python 2.6, Python 2.7, Python 3.1, Python 3.2, Python 3.3 Added file: http://bugs.python.org/file18833/simplecookie_fix.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9824 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9824] SimpleCookie should escape commas and semi-colons
Luke Plant l.plant...@cantab.net added the comment: I forgot to mention backwards compatibility: In the context of Cookie being used in a web application, if developers were relying on literal commas and semi-colons being present in the client side cookie value (e.g. in javascript), the patch will introduce an incompatibility. A quick review of cookies on my computer shows that 22 out of 3079 have commas in them, and none have semi-colons in them. For those with commas, there would still only be a problem if they were reading them client side, or not using Python's Cookie library to decode the values server side. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9824 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
