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

http.cookies, Cookie.py: Improper handling of duplicate cookies #42664

Open
valankar mannequin opened this issue Dec 7, 2005 · 13 comments
Open

http.cookies, Cookie.py: Improper handling of duplicate cookies #42664

valankar mannequin opened this issue Dec 7, 2005 · 13 comments
Labels
3.11 only security fixes 3.12 bugs and security fixes 3.13 bugs and security fixes easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@valankar
Copy link
Mannequin

valankar mannequin commented Dec 7, 2005

BPO 1375011
Nosy @Cito, @devdanzin, @karlcow
Files
  • Cookie.py.patch: Patch for revision 41632 of Cookie.py
  • issue1375011-2.7.patch: Patch, test & docs for Cookie.py on 2.7
  • issue1375011-3.2.patch: Patch, test & docs for http/cookies.py on 3.2
  • 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 = None
    created_at = <Date 2005-12-07.03:50:53.000>
    labels = ['easy', 'type-bug', '3.8', '3.9', '3.10', 'library']
    title = 'http.cookies, Cookie.py: Improper handling of duplicate cookies'
    updated_at = <Date 2021-01-21.15:12:28.480>
    user = 'https://bugs.python.org/valankar'

    bugs.python.org fields:

    activity = <Date 2021-01-21.15:12:28.480>
    actor = 'cito'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2005-12-07.03:50:53.000>
    creator = 'valankar'
    dependencies = []
    files = ['6910', '29178', '29179']
    hgrepos = []
    issue_num = 1375011
    keywords = ['patch', 'easy']
    message_count = 8.0
    messages = ['49178', '49179', '86297', '114634', '182758', '182759', '353845', '385430']
    nosy_count = 7.0
    nosy_names = ['jjlee', 'cito', 'sonderblade', 'valankar', 'ajaksu2', 'karlcow', 'mmelin']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'test needed'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue1375011'
    versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

    Linked PRs

    @valankar
    Copy link
    Mannequin Author

    valankar mannequin commented Dec 7, 2005

    This patch implements part of bug 1372650.

    Sometimes a web client will send 2 instances of the same name:

    Cookie: mycookie=foo; mycookie=bar

    The specs listed here:

    http://wp.netscape.com/newsref/std/cookie_spec.html

    state that the first one is the one that should be used. The other
    cookies listed are the inherited ones from paths that a prefix of the
    current URL. When this is parsed by the Cookie module, mycookie gets
    set to bar when it should be foo.

    This patch changes Cookie.py to only use the first instance of duplicate
    cookies when parsing cookie strings.

    @valankar valankar mannequin added stdlib Python modules in the Lib dir labels Dec 7, 2005
    @sonderblade
    Copy link
    Mannequin

    sonderblade mannequin commented Mar 14, 2007

    That link is misleading and just confuses you. :) Instead read John J. Lee's great explanation at the referenced bug report. I have tested the patch and it works as expected. Without the patch:

    >>> c = SimpleCookie('foo=33;foo=34')
    >>> print c
    Set-Cookie: foo=34

    With the patch:

    >>> c = SimpleCookie('foo=33;foo=34')
    >>> print c
    Set-Cookie: foo=33

    There should be a unit test though and something in the documentation. The keys dict should be a set instead.

    @devdanzin devdanzin mannequin added type-bug An unexpected behavior, bug, or error labels Feb 13, 2009
    @devdanzin
    Copy link
    Mannequin

    devdanzin mannequin commented Apr 22, 2009

    See discussion in bpo-1372650.

    @devdanzin devdanzin mannequin added easy labels Apr 22, 2009
    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Aug 22, 2010

    Even if the patch is still valid there are still no doc or unit test changes.

    @mmelin
    Copy link
    Mannequin

    mmelin mannequin commented Feb 23, 2013

    Attached is a patch with Viraj's original fix except using a set instead of a dict as suggested by Björn. This patch also includes a test case and a note in the docs about this behavior.

    Since Cookie has been moved and the code has been cleaned up somewhat between 2.7 and 3.2 I'm attaching patches for both branches.

    Of course, a decision still needs to be made whether or not this should be applied; the behavior is more correct now, but I don't know if it is worth potentially breaking applications that have come to expect the old behavior. There doesn't seem to be a consensus in bpo-1372650 but I thought having a complete patch would be a good thing regardless.

    @mmelin
    Copy link
    Mannequin

    mmelin mannequin commented Feb 23, 2013

    Just adding the 3.2 patch

    @vadmium vadmium changed the title Improper handling of duplicate cookies http.cookies, Cookie.py: Improper handling of duplicate cookies Aug 22, 2016
    @vadmium vadmium changed the title Improper handling of duplicate cookies http.cookies, Cookie.py: Improper handling of duplicate cookies Aug 22, 2016
    @karlcow
    Copy link
    Mannequin

    karlcow mannequin commented Oct 3, 2019

    Relevant spec
    https://tools.ietf.org/html/rfc6265

    @iritkatriel iritkatriel added 3.8 (EOL) end of life 3.9 only security fixes 3.10 only security fixes labels Nov 6, 2020
    @Cito
    Copy link
    Mannequin

    Cito mannequin commented Jan 21, 2021

    This patch should really be included.

    As carl already mentioned, the relevant spec is RFC 6265, see section 5.4.2: "The user agent SHOULD sort the cookie-list in the following order: Cookies with longer paths are listed before cookies with shorter paths. Among cookies that have equal-length path fields, cookies with earlier creation-times are listed before cookies with later creation-times."

    Currently, if the cookies are loaded with cookies.load(env['HTTP_COOKIE']) as most web frameworks do, then the cookies will be populated with the least specific or oldest values if there are duplicates. This is really bad.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @Hyper-glitch
    Copy link

    Hi! Sorry, I am very new in open source...
    Could you help me how can I assigned this issue for me?

    @arhadthedev
    Copy link
    Member

    @Hyper-glitch It's an open project, no preliminary assignment is required. The only thing you need is to follow the contributor's manual at https://devguide.python.org/ where conventions and processes are described.

    @hadrizi
    Copy link

    hadrizi commented Sep 26, 2023

    @Hyper-glitch are you still working on this?

    @iritkatriel iritkatriel removed the 3.10 only security fixes label Oct 28, 2023
    @iritkatriel iritkatriel added 3.11 only security fixes 3.12 bugs and security fixes 3.13 bugs and security fixes and removed 3.9 only security fixes 3.8 (EOL) end of life labels Oct 28, 2023
    @tavallaie
    Copy link
    Contributor

    @iritkatriel @arhadthedev Based my last PR that waiting for core review I suggest to remove the easy tag from this issue also based on time the issue created and corresponding Core developer opinion on this issue maybe we can close it without accepting my PR.

    @muravev-vasilii
    Copy link

    muravev-vasilii commented Jul 13, 2024

    Hi everyone,

    I am not a core developer and am new to open source, but I want to participate in this issue and help resolve it (mostly out of curiosity and to start being involved in open source Python). The idea came up from a sprint at this year’s EuroPython (https://ep2024.europython.eu/sprints#cpython-core-work-on-python-314).

    If I understand correctly, we want to have an up-to-date http.cookies library that conforms to https://datatracker.ietf.org/doc/html/rfc6265, whereas the current cookies library conforms to a relaxed version of RFC 2109/RFC 2965 (as seen in the documentation https://docs.python.org/3/library/http.cookies.html#module-http.cookies and in the code https://github.com/python/cpython/blob/main/Lib/http/cookies.py).

    Simply changing the current behavior of http.cookies is dangerous because there are probably users who rely on the current behavior, and silently changing it can break things for them without their knowledge.

    At the same time, I found that there is a library that handles cookies according to RFC 6265: https://pypi.org/project/cookies/. However, it is not part of the standard Python library. This library also refers to other implementations.

    So, maybe instead of changing the current implementation, we can do one of the following things:

    1. Add a link to the https://pypi.org/project/cookies/ library in the http.cookies documentation and propose using it if you want to be compliant with RFC 6265.
    2. Contact the author of the https://pypi.org/project/cookies/ library and ask if we can include it under http.cookies as a new module in the standard library.

    If either option makes sense to the community, I can contribute by doing this. :)

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes 3.12 bugs and security fixes 3.13 bugs and security fixes easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    Status: No status
    Development

    No branches or pull requests

    6 participants