Skip to content

SimpleCookie regex can make parsing extremely slow for adversarial payloads #149028

Closed

Description

@KowalskiThomas

Bug report

Bug description:

The current version of http.cookies has a potential performance/security regression (seemingly introduced by #113663) and detected through fuzzing, where a bad cookie payload could significantly slow down request processing on a server relying on SimpleCookie (such as bottle here or tornado there). Such a cookie could be easily be crafted by a bad actor, leading to potential denial of service.

This branch on my fork proposes an alternative regular expression that does not seem to have the problem (removed this to make it clear that my intention was not to get that merged 😅) as well as a reproducer. When run (without the regex change), it clearly shows the problem:

$ ./python reproducer_cookie_redos.py
Timing SimpleCookie().load() with increasing payload size
  units   bytes        time
      1      18      0.0001s
      2      36      0.0001s
      3      54      0.0003s
      4      72      0.0010s
      5      90      0.0042s
      6     108      0.0163s
      7     126      0.0648s
      8     144      0.2560s
      9     162      1.0294s
     10     180      4.1919s
     11     198     16.6055s

For reference, the reproducer is

import sys
import time
import http.cookies

_UNIT = 'x="qu\\"qu\\\\"quot<'   # raw: x="qu\"qu\\"quot<

def make_payload(units: int = 11) -> str:
    """Return a Cookie header value that causes ~4^units backtrack steps."""
    return (" " + _UNIT) * units

print("Timing SimpleCookie.load with increasing payload size")
print(f"  {'units':>5}  {'bytes':>6}  {'time':>10}")
for n in range(1, 14):
    payload = make_payload(n)
    start = time.perf_counter()
    http.cookies.SimpleCookie().load(payload)
    elapsed = time.perf_counter() - start
    print(f"  {n:>5}  {len(payload):>6}  {elapsed:>10.4f}s")
    if elapsed > 10:
        print("  (stopping)")
        break

Note I did not report this as a security issue because the original PR went out in August 2025; 3.14 was already in beta and so only the unreleased version of Python is affected. I downloaded and built 3.14 manually to make sure, and 3.14 is not affected by this regression.

The PSRT does not accept reports that only affect pre-release versions of software, as these features are considered "in-development", please open public issues. (source)

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

Activity

  1. Dreamsorcerer commented on Apr 27, 2026

    @Dreamsorcerer
    Contributor

    Strictly follow Section 4.1.1 and reject nested double quotes.

    That will almost certainly break real-world use cases. The spec says producers SHOULD follow the grammar, not that they MUST, nor that consumers should evaluate them based on that grammar. I think it'd be better to follow the instructions above and split on semicolon with little other processing.

    Our current implementation actually copies the implementation from SimpleCookie, but with several extra characters allowed to stop breaking things:
    https://github.com/aio-libs/aiohttp/blob/0f0c0e5f963ebd59cc9b604197dea41df7836656/aiohttp/_cookie_helpers.py#L50-L82

    We had an audit some time ago, so I don't believe the current version is vulnerable to ReDoS, although it's a rather heavy regex, so we were seeing some worst case inputs taking a little over 10ms. Still likely to be a better idea to drop the regex and split on semicolon though.

    Only allow JSON-encoded values with an additional flag (say loads(strict=False)) or something like that. We can't add a feature that allows adversaries to perform DoS.

    I don't think it's acceptable to have a vulnerable branch needed to support common real-world cookies. We need to have this supported by default for our users, and if it ends up being vulnerable to ReDoS, we'll likely continue rolling our own hacks.

  2. tim-one commented on Apr 28, 2026

    @tim-one
    Member

    Strictly follow Section 4.1.1 and reject nested double quotes.

    That will almost certainly break real-world use cases.

    We can't break what we never supported 😉. The PR to start accepting (some ill-defined patterns of) double quotes is in the main branch now, but never yet released.

    It's unthinkable (to me) too that we'll ever release this change - not so long as exponential-time behavior is possible.

    Python has always embraced "be liberal in what you accept", and I doubt too this will be an exception. So I'm happy to support any form of embedded double quotes that's (a) actually useful to people; (b) is well-defined; (c) can be implemented efficiently without heroic effort.

    I see that aiohttp does have a sensible regexp way to accept JSON-encoded string literals, and I'd be delighted to adopt that in the core distro. But:

    • That breaks tests that were checked in along with the double-quote change. For reasons that are clear as mud, that change also allows patterns of double quotes that I've never seen accepted by anyone else in any context. But I haven't seen all contexts, and for all I know that's important to someone. Although aiohttp apparently wouldn't accept them.

    • aiohttp also seems to accept a lone double quote never followed by a closing double quote.

    "[^";]*                            # Unmatched opening quote (differs from SimpleCookie - issue #7993)
    

    So see last comment: that's also a usage I've never seen accepted by anyone else in any context, yet it was apparently important to aiohttp. Should Python adopt that too? If so, do bizarre. undocumented exceptions ever end? 😉.

    I'd personally be happy to dial waaaaaaay back, and just "parse" on instances of top-level metacharacters, like "=" and ";". Otherwise WYSIWYG, and it's not Python's job to enforce anything else. It's not like SimpleCookie gives any useful feedback anyway on cases it rejects - offending key/value pairs are silently rejected, and just never show up in the output. As un-Pythonic as can be.

    Back to the real world, there is sufficient case to my eyes to support some form of cookie values containing double-quote characters, and JSON-encoded strings make sense to me, are well-defined, and have precedent in the highly Python-adjacent aiohttp.

    But whether "that's enough" I can't say, and I don't have enough relevant experience in the domain to imagine I'll ever be able to be confident about an answer.

  3. Dreamsorcerer commented on Apr 28, 2026

    @Dreamsorcerer
    Contributor

    We can't break what we never supported 😉

    It's certainly not a backwards compatibility concern, but it does break, it's just always been broken..

    I'd personally be happy to dial waaaaaaay back, and just "parse" on instances of top-level metacharacters, like "=" and ";". Otherwise WYSIWYG, and it's not Python's job to enforce anything else. It's not like SimpleCookie gives any useful feedback anyway on cases it rejects - offending key/value pairs are silently rejected, and just never show up in the output. As un-Pythonic as can be.

    I think this is what we've been suggesting. The aiohttp regex is simply because the code was based on SimpleCookie, evaluating it today, I don't think it's the best option. As per the RFC quoted above, it makes more sense to simply split on semicolons. Then we avoid all the annoying edge cases and the computational overhead of such a complex regex.

    Ultimately, I believe this is what browsers are already doing. They pretty much allow anything, loosely following one of the first 2 cookie specs. SimpleCookie was originally based on the third cookie spec, which was never adopted by any browser, hence why we've ended up with such broken behaviour. The RFC mentioned above is the fourth cookie spec, which basically tries be compatible with all the previous specs, meaning very restrictive grammar for producing cookies, and very liberal parsing algorithm for receiving them.

  4. orsenthil commented on Apr 28, 2026

    @orsenthil
    Member

    d7dbde8 wasn't backported to 3.14?

    Just to close the point, it wasn't. We have this report on the main branch (and yet to be released 3.15).

  5. tim-one commented on Apr 28, 2026

    @tim-one
    Member

    The plot thickens 😉..

    $ py
    Python 3.14.4 (tags/v3.14.4:23116f9, Apr  7 2026, 14:10:54) [MSC v.1944 64 bit (AMD64)] on win32
    ...
    >>> from http import cookies
    >>> C = cookies.SimpleCookie()
    >>> C.load(r'x="a\"\"b"')
    >>> C
    <SimpleCookie: x='a""b'>

    That is, 3.14 already accepted properly formed JSON-encoded strings as cookie values. And the internal _unquote() function strips off the backslashes. That's why I thought the problematic PR was in 3.14 too.

    So now I'm totally lost as to the intended purpose of the "problem PR". It was apparently to accept some (seemingly wholly undefined) superset of what JSON does.

  6. tim-one commented on Apr 28, 2026

    @tim-one
    Member

    And the "problem PR" in fact replaced the well-behaved JSON regexp (same as the one aiohttp uses) with the ill-behaved one on the main branch. So no doubt that is was intentional. "But why?" I can't guess. The issue report on the PR seemed to talk about accepting JSON string literals - but they were already accepted.

  7. tim-one commented on Apr 28, 2026

    @tim-one
    Member

    Recommendation: Revert #113663. It introduced the possibility for a potent (exponential time in the length of the input) reDOS attack. It's unclear what it was hoping to accomplish, as Python already accepted JSON-encoded string literals in cookie values.

    Since nobody seems to have a clear idea of what new kinds of string spellings it intended to cater to, there's no way I can see to "repair" the PR. The only "spec" we have is the regexp itself, and it's inherently ambiguous (the source of exponential-time behavior in some cases). The issue report the PR closed doesn't shed light on this either (it only talks explicitly about JSON strings - which were already supported).

  8. KowalskiThomas commented on Apr 28, 2026

    @KowalskiThomas
    ContributorAuthor

    Since nobody seems to have a clear idea of what new kinds of string spellings it intended to cater to, there's no way I can see to "repair" the PR.

    Actually, if you look at the PR that introduced the regex change, it references a follow-up PR -- #137566 -- which updates documentation and provides an example of what it allows.

    I took the example from these exacts docs:

    >>> import json
    >>> from http import cookies
    >>> C = cookies.SimpleCookie()
    >>> C.load(f'cookies=7; mixins="{json.dumps({"chips": "dark chocolate"})}"; state=gooey')
    >>> print(C)

    and ran it on Python 3.14:

    Set-Cookie: cookies=7
    

    ... then on Python 3.15 / dev:

    Set-Cookie: cookies=7
    Set-Cookie: mixins="{"chips": "dark chocolate"}"
    Set-Cookie: state=gooey
    

    It also anecdotally does seem to be something people actually encounter in real-life according to at least one comment on the original PR + the fact that many Python packages seem to have worked around the issue one way or another after issues were posted (see downstream issues on the original PR).

  9. picnixz commented on Apr 28, 2026

    @picnixz
    Member

    I'm marking this as a release blocker, because we cannot let something that allow ReDoS attacks. I suggest to revert this change because beta is around the corner. If we still have time until then, I would suggest using a non-regex approach and split on semicolonrs. An approach with a find() + count() to check for balanced non-quoted double quotes like I did in #136072 can be used.

  10. KowalskiThomas commented on Apr 28, 2026

    @KowalskiThomas
    ContributorAuthor

    I agree, that sounds like the safest move indeed, nothing prevents us/someone from re-implementing the change with a more sound solution in a follow-up PR.

    Should I go ahead and make a PR?

  11. picnixz commented on Apr 28, 2026

    @picnixz
    Member

    I don't really know whether we want to fix it now or revert it for 3.15. And I don't know if we can actually leave it in 3.15 and fix it during the beta.

    cc @sethmlarson @hugovk @serhiy-storchaka

  12. Dreamsorcerer commented on Apr 28, 2026

    @Dreamsorcerer
    Contributor

    An approach with a find() + count() to check for balanced non-quoted double quotes like I did in #136072 can be used.

    I still don't think even that should be checked. Just allow anything (except for the recent rejection of CTL characters). There is no such thing as a quoted cookie, that was a historical mistake, so I don't think we should check for them. The RFC doesn't tell us to check for them.

    Also, just highlighting the RFC here "A user agent MUST use an algorithm equivalent to the following". Using this regex or checking for number of quotes is directly violating the RFC.

  13. hugovk commented on Apr 28, 2026

    @hugovk
    Member

    We can still fix bugs during the beta, and also revert features.

    If we revert a feature now, we can't re-add it during the beta.

  14. orsenthil commented on Apr 28, 2026

    @orsenthil
    Member

    Hi @KowalskiThomas ,

    First, Thanks for this report.

    My problem with the proposed fix in the branch is

    "(?:[^"\](?:\.[^"\])"(?!\s(?:\s|;|$)))[^"\](?:\.[^"\])" # Any double-quoted string

    It is such a complex state machine that it will be hard for humans to grasp and understand for the benefit that it is bringing.

    re-implementing the change with a more sound solution in a follow-up PR.

    What will be a sound solution?


    Hi @Dreamsorcerer ,

    Using this regex or checking for number of quotes is directly violating the RFC.

    I tend to disagree here. RFC determines what we should, how it is done, is left to the implementor.

  15. KowalskiThomas commented on Apr 28, 2026

    @KowalskiThomas
    ContributorAuthor

    Hi @orsenthil, thanks for taking a look.

    Sorry if that was unclear from the description, my intention was never to get the alternative regex in for this exact reason -- I have no idea what it's even trying to do so I would never have the confidence to merge that.
    I completely removed that commit from my branch (and updated the description) so now there's only the reproducer, which I think is the one thing that matters for this issue.

    What will be a sound solution?

    To be honest I don't know at this point.
    I was just trying to say that, given this is a release blocker, if we don't see a solution to it right now, the best thing to do might be to revert it to unblock the release and in a second phase to seek a better solution -- which might event come before the beta actually (in response to @picnixz's comment).

    Based on the various issues I linked, it seems like CPython is not the only Python project that had to tackle cookie parsing, and it seems like others were able to figure it out one way or another (each with different caveats/shortcomings, I guess), so there probably is some way (but there probably is some work to do to figure out which trade-offs we want to choose).

  16. StanFromIreland commented on Apr 28, 2026

    @StanFromIreland
    Member

    We can defer the release blocking, as Hugo (the RM for 3.15) noted, we can revert this in beta anyway.

  17. Dreamsorcerer commented on Apr 28, 2026

    @Dreamsorcerer
    Contributor

    Using this regex or checking for number of quotes is directly violating the RFC.

    I tend to disagree here. RFC determines what we should, how it is done, is left to the implementor.

    I'm unclear what you disagree with here. The exact implementation is up to the implementor, as described at the start of the RFC:

    Conformance requirements phrased as algorithms or specific steps can be implemented in any manner, so long as the end result is equivalent.

    But, using the regex or checking for number of quotes does not produce the same end result. That is surely a violation of the RFC. You can choose not to follow the RFC, but then we should stop referring to it's grammar in that case..

  18. serhiy-storchaka commented on Apr 28, 2026

    @serhiy-storchaka
    Member

    RFC 2109 refers to RFC 2068 which does not allow a double-quote in quoted-string.

    RFC 2965 refers to RFC 2616 which allow a double-quote in quoted-string only if it was backslash-escaped.

    RFC 6265 does not allow semicolon in a value, even in the lenient parsing mode. Strict output mode does not allow whitespaces, double-quote, comma and backslash.

    So examples with non-escaped JSON strings are invalid in RFC 2109 and RFC 2965. All embedded double-quotes should be backslash-escaped, as well as the backslash itself, for RFC 2965. But examples containing semicolon are invalid in RFC 6265.

    What is worse, our code tries to parse both Set-Cookie: and Cookie: syntax simultaneously, and tries to support some non-standard quirks (separation of key-value pairs by a simple whitespace instead of semicolon). It begs for redesign.

  19. tim-one commented on Apr 28, 2026

    @tim-one
    Member

    Ah, the light dawns! The original didn't care about JSON string literals per se, but about arbitrary JSON encodings that could embed a (one or more) JSON string literal(s). We can't parse the result of

    json.dumps({"chips": "dark chocolate"})

    on its own because it contains "illegal" characters for a cookie value (double quote, and space). So they slapped a pair of double quotes around the result of that to make it "look like" a JSON string literal. But it isn't - JSON string literals never contain an unescaped double quote, and our JSON regexp snippet relies on that. So we couldn't robustly parse that either.

    i agree with marking this issue as a release blocker, but this is so far out of my areas of expertise that I don't have strong preferences about what to do instead. We're certainly (I hope) not going to embed a full blown validating JSON decoder.

    Possible quick hack: change the interior of our string regexp snippet to accept any number of:

    • backslash followed by dquote; or
    • dquote not followed by (negative lookahead assertion) any number of whitespace followed by a semicolon or end-of-input; or
    • single character not in {backslash,, dquote}

    The second part will accept unescaped dquotes, provided they're not the dquote ending the string. The three alternatives are mutually exclusive, so any backtracking can't follow a different path.

  20. tim-one commented on Apr 28, 2026

    @tim-one
    Member

    @serhiy-storchaka, alas, there are formal standards and de facto standards. The latter are more important, but fuzzy because they derive from what widespread software actually does. Which isn't always consistent, or even documented. But libraries that hope to be useful in "real life" need to cater to them.

  21. Dreamsorcerer commented on Apr 28, 2026

    @Dreamsorcerer
    Contributor

    RFC 2109 refers to RFC 2068 which does not allow a double-quote in quoted-string.

    RFC 2965 refers to RFC 2616 which allow a double-quote in quoted-string only if it was backslash-escaped.

    RFC 6265 does not allow semicolon in a value, even in the lenient parsing mode. Strict output mode does not allow whitespaces, double-quote, comma and backslash.

    Which is why the parsing algorithm in 6265 is so lenient, as it allows parsing cookies produced by any of these standards, including the original netscape standard that predated RFC 2109. As far as I understand, most browsers still implement something along the lines of the original netscape standard. Every RFC since has been largely ignored by browser vendors.

    RFC 6265 is the end result of that path. So it provides grammar for generating cookies that will be valid according to all 3 previous standards (i.e. it's the most restrictive), and a parsing algorithm that allows cookies from all 3 standards (i.e. it's the most lenient).

  22. Dreamsorcerer commented on Apr 28, 2026

    @Dreamsorcerer
    Contributor

    In other words, validating received cookies according to the RFC 6265 grammar is guaranteed to fail, which is why it says you MUST parse with the much simpler algorithm, ignoring the grammar it asks you to use when producing cookies.

  23. tim-one commented on Apr 28, 2026

    @tim-one
    Member

    FYI, this regexp fragment replacement passes all the main branch cookie tests, and has no cases worse than linear time (the OP's reproducer runs to completion in a tiny fraction of a second). It intends to support unescaped double quotes, and a superset of all JSON escape sequences. It makes no attempt to ensure that the number of embedded raw dquotes is even. To trigger, it still requires that a cookie value begin with a raw dquote, and end with a raw dquote (before any stretch of whitespace up to a semicolon or end-of-input).

        "(?: [^\\"] | \\. | "(?! \s*(;|$)))*"  # Any double-quoted string
    
    
  24. serhiy-storchaka commented on Apr 28, 2026

    @serhiy-storchaka
    Member

    Unfortunately, RFC 2965 and RFC 6265 are not compatible. The former allows embedded semicolons in quoted-strings, the latter parses such data incorrectly.

  25. Dreamsorcerer commented on Apr 28, 2026

    @Dreamsorcerer
    Contributor

    Unfortunately, RFC 2965 and RFC 6265 are not compatible. The former allows embedded semicolons in quoted-strings, the latter parses such data incorrectly.

    I'm not so clear that RFC 2965 intends to allow that, but RFC 6265 is certainly the closest possible attempt at being compatible with everything. Regardless, RFC 2965 is a spec that has never been adopted by a single browser from what I understand. I don't think there is any browser available that has any understanding of a quoted string. Python is one of the only things I'm aware of which has implemented this spec, which is why it has always been completely incompatible with the web and we've needed to hack around it.

  26. tim-one commented on Apr 29, 2026

    @tim-one
    Member

    Regardless of what standards say, Python has its own backward-compatibility to worry about. We apparently have accepted properly formed JSON-encoded string literals as cookie values "for a long time", and I'm -1 on giving that up without broad-based community buy-in well in advance.

    The minimal change now is to just revert the reDOS PR.

    Second-most minimal is to accept a variation of my one-line regexp-fragment replacement, which passes all the new tests added by the reDOS PR, but can't be provoked into worse than linear time behavior. That would preserve the apparent intent of the reDOS PR, but plugging the security hole with a better implementation.

    Beyond that, schemes to rewrite SimpleCookie in fundamental ways shouldn't be rushed to meet 3.15's imminent beta cutoff.

    Yes, I'm mostly a fan of doing minimal parsing, just enough to identify whatever was passed for "key" and "value", with no concern for what they may "mean", and doing no transformations on the text values the user passed in. But I can't sell (or buy) that our doing more than just that is "a bug". At worst, it may be an ill-considered design, but a last-minute rush to replace the old design isn't a sensible way to address that.

  27. tim-one commented on Apr 29, 2026

    @tim-one
    Member

    Yes, I'm mostly a fan of doing minimal parsing,

    Although that may be a doomed fantasy. Looking at the commit history, a number of changes were made to cookiies.py to make parsing stricter, to plug other putative "security holes". LIke

    commit b1e36073cdde71468efa27e88016aa6dd46f3ec7
    Author: Antoine Pitrou <solipsis@pitrou.net>
    Date:   Fri Nov 21 01:20:57 2014 +0100
    
        Issue #22796: HTTP cookie parsing is now stricter, in order to protect against potential injection attack
    

    And that in turn appeared to trigger a number of "my perfectly fine code no longer works!" complaints.

    It's hard to be all things to all people 😉 .

  28. orsenthil commented on Apr 29, 2026

    @orsenthil
    Member

    The minimal change now is to just revert the reDOS PR.

    Second-most minimal is to accept a variation of my one-line regexp-fragment replacement, which passes all the new tests added by the reDOS PR, but can't be provoked into worse than linear time behavior. That would preserve the apparent intent of the reDOS PR, but plugging the security hole with a better implementation.

    I am inclined to one of these too. I need to study / convince myself through state machines that the regex replacement wont be introducing yet parsing bug (or reDOS).

  29. tim-one commented on Apr 29, 2026

    @tim-one
    Member

    I've rewritten in a more efficient form, but that's just a constant-factor thing (still linear time). The key is that we have 3 mutually exclusive choices:

    • Backslash comes next. Then consume it and the following character too. That's always safe for backslash escapes JSON produces. For example, it does no harm to parsing to consume the leading \u in \u1234
    • Dquote comes next. Then consume it if and only it's not the last dquote in the input (the last has to be saved to close the string).
    • If neither backslash nor dquote, consume it.

    Backtracking can occur (although my rewrite uses possessive quantifiers to minimize that), but at each point it backtracks to there are no other choices that can be taken, so backtracking has to give up and go back yet another character.

    A more involved argument, but still straightforward, establishes that the \s* in the negative lookahead assertion also can't be provoked into bad behavior. If there is long string of whitespace following a dquote, in context it will be crawled over twice, but not more than that.

  30. serhiy-storchaka commented on Apr 29, 2026

    @serhiy-storchaka
    Member

    One thing bothers me. All "real world" examples in #92936 and #71861 use non-quoted JSON in cookie. So why the regex for quoted strings was changed? This does not help with that examples.

    It is possible to use RFC 2109/RFC 2965 for quoted strings and RFC 6265 for unquoted strings.

  31. Dreamsorcerer commented on Apr 29, 2026

    @Dreamsorcerer
    Contributor

    Regardless of what standards say, Python has its own backward-compatibility to worry about. We apparently have accepted properly formed JSON-encoded string literals as cookie values "for a long time", and I'm -1 on giving that up without broad-based community buy-in well in advance.

    The proposal is to effectively allow anything (except security concerns like CTL chars), so it wouldn't break backwards compatibility. But, yes, this particular path doesn't need to be rushed in for 3.15, it can also be done later. That wasn't my intention.

  32. tim-one commented on Apr 29, 2026

    @tim-one
    Member

    @serhiy-storchaka, yes, the original issue was hard to follow, sloppy about details. Nevertheless, the actual patch obviously targeted only double-quoted strings as a whole. Which the OP acknowledged in a comment. But I'm fine with just reverting the PR, period. It wasn't sufficiently thought out to begin with, and on more than one count.

    @Dreamsorcerer

    so it wouldn't break backwards compatibility.

    Moving to just splitting on semicolons would break stuff. I've never before used this code, so that even newbie-me realizes that should be suggestive 😉.

    For example, this "works" now (3.14):

    >>> C = cookies.SimpleCookie()
    >>> C.load('x=";;;"')
    >>> C
    <SimpleCookie: x=';;;'>

    That is, semicolon loses its privileged status inside a dquted string value now. And we also evaluate some escape sequences in dquoted string values now, like

    >>> C = cookies.SimpleCookie()
    >>> C.load(r'x="\141\142\143"') # r-string so that load() sees the octal escapes
    >>> C
    <SimpleCookie: x='abc'>

    That one isn't about parsing, but about active transformations of the values. I personally don't think we "should be" in that business either.

    this particular path doesn't need to be rushed in for 3.15, it can also be done later

    I think that best. My take for 3.15: revert the reDOS PR, period.

  33. serhiy-storchaka commented on Apr 29, 2026

    @serhiy-storchaka
    Member

    See the draft #149145. It uses the hybrid approach.

    But there are other issues. If we make parsing too lenient, this can cause security issues. This will break some tests. If we make parsing more strict, what rules should we use? They are different between RFCs. This will break other existing tests. What is worse, handling invalid characters is inconsistent -- some cause an exception, others causes silent ignoring of the cookie.

    There are other issues. For example, I suspect that existing non-standard encoding scheme was designed for 8-bit strings in Python 2. It does not work as expected for non-ASCII strings in Python 3.

  34. collinanderson commented on Apr 29, 2026

    @collinanderson

    What is worse, handling invalid characters is inconsistent -- some cause an exception, others causes silent ignoring of the cookie.

    Too late for 3.15, but long term I suggest either 1. lenient: keeping it simple and following the spec (RFC 6265) and allowing everything (except nameless cookies), and then users can then filter/exclude/reject further, or 2. strict: matching the official web-platform-tests (including allowing nameless cookies). (The spec should probably get updated to match web-platform-tests anyway)
    https://github.com/web-platform-tests/wpt/blob/master/cookies/name/name-ctl.html
    https://github.com/web-platform-tests/wpt/blob/master/cookies/value/value-ctl.html
    https://github.com/web-platform-tests/wpt/blob/master/cookies/name/name.html
    https://github.com/web-platform-tests/wpt/blob/master/cookies/value/value.html
    https://github.com/web-platform-tests/wpt/blob/master/cookies/size/name-and-value.html
    etc

    There are other issues. For example, I suspect that existing non-standard encoding scheme was designed for 8-bit strings in Python 2. It does not work as expected for non-ASCII strings in Python 3.

    I think browsers encode the cookie header as cp1252, which generally limits the possible incoming characters.

  35. tim-one commented on Apr 29, 2026

    @tim-one
    Member

    See the draft #149145. It uses the hybrid approach.

    Unclear to me what "hybrid approach" means to you, exactly. This line in the PR is troublesome:

    "(?:\\.|[^"])*+"    # Any double-quoted string (RFC 2109/2965)
    

    The first alternative can consume a backslash. But so can the second alternative. The match engine can't guess which to use in advance, so potentially piles up an unbounded sequence of 2-way branches to explore. I understand that the possessive quantifier prevents actually backtracking, but a robust regexp really shouldn't rely on that. Best practice is to ensure all "choice points" are mutually exclusive at the time they're first seen.

    That's easily achieved here by changing the second alternative to [^"\\] You never intended for the second alternative to match a backslash anyway. Be explicit about that?

    I assume you realize this also gives up on what the reDOS PR intended to accomplish (allow unescaped dquotes in a string value too).

    I suspect that existing non-standard encoding scheme was designed for 8-bit strings in Python 2. It does not work as expected for non-ASCII strings in Python 3.

    I believe that's right. There's every sign that cookies.py implicitly assumes the character set is confined to 7-bit ASCII.

    But this particular issue is really unsuited to discussing such fundamental changes.

  36. tim-one commented on Apr 29, 2026

    @tim-one
    Member

    Another caution about haste 😉. "My" regexp tweak is solid so far as it goes, but it's also flawed.

    C.load(r'x=";""')

    yields the hoped-for

    <SimpleCookie: x=';"'>

    but the variant

    C.load(r'x=";";"')

    yields the unintended

    <SimpleCookie: x=';'>

    That the raw dquote is followed by a (optional string of whitespace followed by a) semicolon prevents the "very careful" regexp from accepting the raw dquote, assuming \s*; (or \s*$) following it must be the end of the cookie key=value pair input. So that dquote is not consumed in the guts, but is taken instead to be the closing dquote of the string, and the semicolon following it taken to mean "end of input field".

    This is typical of mysteries that pop up when trying to use regexps for "real parsing". They're often inadequate to the full task without heroic effort.

    Compounded by that we just let failed parses pass silently,. In the variation, it leaves the lone raw dquote at the end untouched, which isn't matched by anything, but we don't tell the user that we couldn't make any sense of that. But when using regexps for parsing, it's hard too to create a useful error message. "CooikeError: internal do-everything-all-at-once regexp failed to match" leaves a bit to be desired 😉.

  37. tim-one commented on Apr 30, 2026

    @tim-one
    Member

    To drive home why "quoted value string can contain anything" is intractable, here's an input:

    x=""1";y="2""

    There are then two different ways to parse it. Either it's setting x to

    ""1";y="2""

    or it's setting two keys

    x=""1"
    y="2""

    We can't know whether a semicolon does or doesn't terminate an input field unless we know whether it appears inside or outside a quoted string. But we can't know whether an unescaped dquote terminates a string unless we know whether it's the last significant character in an input field. Combined, those leave the "; substring of the input hopelessly ambiguous.

    Which is why every other gimmick on Earth establishes rigid rules for how to spell "here this meta-character has no special meaning - it's just another character".

    Why this doesn't bite us now (3.14): inside a quoted string, semicolon has no meta-meaning, period. But an unescaped dquote always has its meta-meaning. The string ends when and only when we encounter an unsscaped dqupte. So ";, unless preceded by backslash (escaping the dquote) means "the string ended, and so did the input field". The semicolon regains its meta-meaning.

    "My regexp" aims to preserve that resolution, tying the interpretation of a raw dquote to whether it is or isn't followed by \s*(;|$). Ugly, though - it's not the characters on their own that drive it, but rather how they combine in context.

  38. serhiy-storchaka commented on Apr 30, 2026

    @serhiy-storchaka
    Member

    Since my PR completely overwrites the previous changes, I think that it will be clearer if just revert them first.

    It does not match any specification and does not solve the original issue. It is also inherently ambiguous.

  39. orsenthil commented on Apr 30, 2026

    @orsenthil
    Member

    I think, this is good to revert the change right now and discuss separately if it is worth fixing, given the disadvantages.

  40. added a commit that references this issue on Apr 30, 2026
  41. tim-one commented on Apr 30, 2026

    @tim-one
    Member

    Since reverting the reDOS PRs is complete:

    #149182

    I'm closing this issue report. Other ideas for reworking cookies.py are most welcome, but should be pursued elsewhere (e.g., in the "Ideas" category of discuss.python.org).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    3.15bugs and security fixesrelease-blockerstdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions