classification
Title: Cookie.py does not correctly quote Morsels
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Mark.Williams, ajaksu2, alex, berker.peksag, zdobersek, zenzen
Priority: normal Keywords: patch

Created on 2004-07-15 00:17 by zenzen, last changed 2018-05-16 08:16 by berker.peksag. This issue is now closed.

Files
File name Uploaded Description Edit
991266test.patch zdobersek, 2009-02-14 17:14 Patch to test_cookie.py review
991266fix.patch zdobersek, 2009-02-18 14:40 Fix - properly quote cookie's comment review
issue991266.diff berker.peksag, 2016-04-25 12:04 review
Pull Requests
URL Status Linked Edit
PR 6555 merged berker.peksag, 2018-04-20 21:29
PR 6570 merged miss-islington, 2018-04-22 23:48
PR 6571 merged miss-islington, 2018-04-22 23:49
Messages (15)
msg60528 - (view) Author: Stuart Bishop (zenzen) Date: 2004-07-15 00:17
The quoting works fine for cookie values, but doesn't kick in for 
attributes like Comment. 

>>> c = SimpleCookie()
>>> c['foo'] = u'\N{COPYRIGHT SIGN}'.encode('UTF8')
>>> print str(c)
Set-Cookie: foo="\302\251";
>>> c['foo']['comment'] = u'\N{BIOHAZARD SIGN}'.encode('UTF8')
>>> print str(c)
Set-Cookie: foo="\302\251"; Comment=?;
>>> str(c)
'Set-Cookie: foo="\\302\\251"; Comment=\xe2\x98\xa3;'
>>> 
msg82094 - (view) Author: Zan Dobersek (zdobersek) Date: 2009-02-14 17:14
This patch adds an unicode character, converted to UTF8 as a cookie's
comment and then checks if it is correctly quoted.
msg82418 - (view) Author: Zan Dobersek (zdobersek) Date: 2009-02-18 14:40
This patch properly quotes cookie's comment and successfully passes
test_cookie.py with applied patch.
msg82420 - (view) Author: Daniel Diniz (ajaksu2) (Python triager) Date: 2009-02-18 15:07
Thanks, Zan!

All tests pass with both patches applied. Test and fix look correct to me.
msg110392 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-07-15 22:17
Can someone please take a look at this Cookie.py two line patch.
msg114367 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-08-19 15:12
Can we have this committed please, msg82420 says the patches are ok.
msg264172 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-04-25 12:04
Here is a patch for Python 3.
msg315496 - (view) Author: Alex Gaynor (alex) * (Python committer) Date: 2018-04-20 00:16
Berker your patch looks good to me.

Convert it to a PR and then merge?
msg315498 - (view) Author: Mark Williams (Mark.Williams) * Date: 2018-04-20 02:04
This patch only quotes the Comment attribute, and the rest of the code only quotes attributes if they're of the expected type.  Consider Expires:

>>> from http.cookies import SimpleCookie
>>> c = SimpleCookie()
>>> c['name'] = 'value'
>>> c['name']['comment'] = '\n'
>>> c['name']['expires'] = 123
>>> c.output()
'Set-Cookie: name=value; Comment="\\012"; expires=Fri, 20 Apr 2018 02:03:13 GMT'
>>> c['name']['expires'] = '123; path=.example.invalid'
'Set-Cookie: name=value; Comment="\\012"; expires=123; path=.example.invalid'

Here's the offending line:

https://github.com/python/cpython/blob/b87c1c92fc93c5733cd3d8606ab2301ca6ba208f/Lib/http/cookies.py#L415

Why not quote all attribute values?
msg315499 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2018-04-20 03:04
>>> from http.cookies import SimpleCookie
>>> c = SimpleCookie()
>>> c['name'] = 'value'
>>> c['name']['comment'] = '\n'
>>> c['name']['expires'] = '123; path=.example.invalid'
'Set-Cookie: name=value; Comment="\\012"; expires=123; path=.example.invalid'

What do you think that the snippet above should return?

    'Set-Cookie: name=value; Comment="\\012"; expires=Fri, 20 Apr 2018 02:03:13 GMT; path=.example.invalid'

or

    'Set-Cookie: name=value; Comment="\\012"; expires=Fri, 20 Apr 2018 02:03:13 GMT; path=".example.invalid"'

or

    'Set-Cookie: name=value; Comment="\\012"; expires=123; path=".example.invalid"'

?

I don't think the path attribute (or all of them) needs to be quoted unconditionally. Looking at https://tools.ietf.org/html/rfc6265#section-4.1.1, it looks like quoting for cookie-value is optional.

Is there a use case or examples from other programming languages you can share with us?
msg315500 - (view) Author: Alex Gaynor (alex) * (Python committer) Date: 2018-04-20 03:07
None of the above :-) I'd expect the last one, but with quoting.

You should not be able to set fields in a cookie by injection.
msg315634 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2018-04-22 23:48
New changeset d5a2377c3d70e4143bcbee4a765b3434e21f683a by Berker Peksag in branch 'master':
bpo-991266: Fix quoting of Comment attribute of SimpleCookie (GH-6555)
https://github.com/python/cpython/commit/d5a2377c3d70e4143bcbee4a765b3434e21f683a
msg315636 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2018-04-23 00:58
New changeset 9fc998d761591f2741d8e94f5b3009c56ae83882 by Berker Peksag (Miss Islington (bot)) in branch '3.7':
bpo-991266: Fix quoting of Comment attribute of SimpleCookie (GH-6555)
https://github.com/python/cpython/commit/9fc998d761591f2741d8e94f5b3009c56ae83882
msg315637 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2018-04-23 00:58
New changeset 8a6f4b4bba950fb8eead1b176c58202d773f2f70 by Berker Peksag (Miss Islington (bot)) in branch '3.6':
bpo-991266: Fix quoting of Comment attribute of SimpleCookie (GH-6555)
https://github.com/python/cpython/commit/8a6f4b4bba950fb8eead1b176c58202d773f2f70
msg316782 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2018-05-16 08:16
I've opened bpo-33535 to discuss Mark Williams' suggestion.
History
Date User Action Args
2018-05-16 08:16:42berker.peksagsetstatus: open -> closed
versions: - Python 2.7
messages: + msg316782

resolution: fixed
stage: patch review -> resolved
2018-04-23 00:58:53berker.peksagsetmessages: + msg315637
2018-04-23 00:58:33berker.peksagsetmessages: + msg315636
2018-04-22 23:49:21miss-islingtonsetpull_requests: + pull_request6268
2018-04-22 23:48:27miss-islingtonsetpull_requests: + pull_request6267
2018-04-22 23:48:14berker.peksagsetmessages: + msg315634
2018-04-20 21:29:51berker.peksagsetpull_requests: + pull_request6251
2018-04-20 03:07:18alexsetmessages: + msg315500
2018-04-20 03:04:19berker.peksagsetmessages: + msg315499
versions: + Python 3.7, Python 3.8, - Python 3.4, Python 3.5
2018-04-20 02:04:19Mark.Williamssetnosy: + Mark.Williams

messages: + msg315498
versions: + Python 3.4
2018-04-20 00:16:17alexsetnosy: + alex
messages: + msg315496
2016-04-25 12:04:56berker.peksagsetfiles: + issue991266.diff
versions: + Python 3.5, Python 3.6, - Python 3.1, Python 3.2
nosy: + berker.peksag

messages: + msg264172
2014-02-03 19:49:29BreamoreBoysetnosy: - BreamoreBoy
2010-08-19 15:12:27BreamoreBoysetmessages: + msg114367
2010-07-15 22:17:56BreamoreBoysetversions: + Python 3.1, Python 2.7, Python 3.2, - Python 2.6
2010-07-15 22:17:00BreamoreBoysetnosy: + BreamoreBoy
messages: + msg110392
2009-02-18 15:07:02ajaksu2setnosy: + ajaksu2
messages: + msg82420
stage: test needed -> patch review
2009-02-18 14:40:15zdoberseksetfiles: + 991266fix.patch
messages: + msg82418
2009-02-14 17:14:14zdoberseksetfiles: + 991266test.patch
keywords: + patch
messages: + msg82094
nosy: + zdobersek
2009-02-13 21:13:09jjleesetnosy: - jjlee
2009-02-13 01:18:53ajaksu2setnosy: + jjlee
stage: test needed
type: behavior
versions: + Python 2.6, - Python 2.3
2004-07-15 00:17:04zenzencreate