Skip to content
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

Cookie.py does not correctly quote Morsels #40569

Closed
zenzen mannequin opened this issue Jul 15, 2004 · 15 comments
Closed

Cookie.py does not correctly quote Morsels #40569

zenzen mannequin opened this issue Jul 15, 2004 · 15 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@zenzen
Copy link
Mannequin

zenzen mannequin commented Jul 15, 2004

BPO 991266
Nosy @devdanzin, @alex, @berkerpeksag, @markrwilliams
PRs
  • bpo-991266: Fix quoting of the Comment attribute of SimpleCookie #6555
  • [3.7] bpo-991266: Fix quoting of Comment attribute of SimpleCookie (GH-6555) #6570
  • [3.6] bpo-991266: Fix quoting of Comment attribute of SimpleCookie (GH-6555) #6571
  • Files
  • 991266test.patch: Patch to test_cookie.py
  • 991266fix.patch: Fix - properly quote cookie's comment
  • issue991266.diff
  • 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:

    assignee = None
    closed_at = <Date 2018-05-16.08:16:42.420>
    created_at = <Date 2004-07-15.00:17:04.000>
    labels = ['3.7', '3.8', 'type-bug', 'library']
    title = 'Cookie.py does not correctly quote Morsels'
    updated_at = <Date 2018-05-16.08:16:42.418>
    user = 'https://bugs.python.org/zenzen'

    bugs.python.org fields:

    activity = <Date 2018-05-16.08:16:42.418>
    actor = 'berker.peksag'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-05-16.08:16:42.420>
    closer = 'berker.peksag'
    components = ['Library (Lib)']
    creation = <Date 2004-07-15.00:17:04.000>
    creator = 'zenzen'
    dependencies = []
    files = ['13085', '13130', '42589']
    hgrepos = []
    issue_num = 991266
    keywords = ['patch']
    message_count = 15.0
    messages = ['60528', '82094', '82418', '82420', '110392', '114367', '264172', '315496', '315498', '315499', '315500', '315634', '315636', '315637', '316782']
    nosy_count = 6.0
    nosy_names = ['zenzen', 'ajaksu2', 'alex', 'zdobersek', 'berker.peksag', 'Mark.Williams']
    pr_nums = ['6555', '6570', '6571']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue991266'
    versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

    @zenzen
    Copy link
    Mannequin Author

    zenzen mannequin commented Jul 15, 2004

    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;'
    >>>

    @zenzen zenzen mannequin added stdlib Python modules in the Lib dir labels Jul 15, 2004
    @devdanzin devdanzin mannequin added type-bug An unexpected behavior, bug, or error labels Feb 13, 2009
    @zdobersek
    Copy link
    Mannequin

    zdobersek mannequin commented Feb 14, 2009

    This patch adds an unicode character, converted to UTF8 as a cookie's
    comment and then checks if it is correctly quoted.

    @zdobersek
    Copy link
    Mannequin

    zdobersek mannequin commented Feb 18, 2009

    This patch properly quotes cookie's comment and successfully passes
    test_cookie.py with applied patch.

    @devdanzin
    Copy link
    Mannequin

    devdanzin mannequin commented Feb 18, 2009

    Thanks, Zan!

    All tests pass with both patches applied. Test and fix look correct to me.

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Jul 15, 2010

    Can someone please take a look at this Cookie.py two line patch.

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Aug 19, 2010

    Can we have this committed please, msg82420 says the patches are ok.

    @berkerpeksag
    Copy link
    Member

    Here is a patch for Python 3.

    @alex
    Copy link
    Member

    alex commented Apr 20, 2018

    Berker your patch looks good to me.

    Convert it to a PR and then merge?

    @markrwilliams
    Copy link
    Mannequin

    markrwilliams mannequin commented Apr 20, 2018

    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:

    append("%s=%s" % (self._reserved[key], value))

    Why not quote all attribute values?

    @berkerpeksag
    Copy link
    Member

    >>> 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?

    @berkerpeksag berkerpeksag added 3.7 (EOL) end of life 3.8 only security fixes labels Apr 20, 2018
    @alex
    Copy link
    Member

    alex commented Apr 20, 2018

    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.

    @berkerpeksag
    Copy link
    Member

    New changeset d5a2377 by Berker Peksag in branch 'master':
    bpo-991266: Fix quoting of Comment attribute of SimpleCookie (GH-6555)
    d5a2377

    @berkerpeksag
    Copy link
    Member

    New changeset 9fc998d by Berker Peksag (Miss Islington (bot)) in branch '3.7':
    bpo-991266: Fix quoting of Comment attribute of SimpleCookie (GH-6555)
    9fc998d

    @berkerpeksag
    Copy link
    Member

    New changeset 8a6f4b4 by Berker Peksag (Miss Islington (bot)) in branch '3.6':
    bpo-991266: Fix quoting of Comment attribute of SimpleCookie (GH-6555)
    8a6f4b4

    @berkerpeksag
    Copy link
    Member

    I've opened bpo-33535 to discuss Mark Williams' suggestion.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants