Skip to content

gh-154546: Make http.cookies.BaseCookie.load() atomic on error#154674

Open
gianghungtien wants to merge 1 commit into
python:mainfrom
gianghungtien:gh-154546-cookie-load-atomic
Open

gh-154546: Make http.cookies.BaseCookie.load() atomic on error#154674
gianghungtien wants to merge 1 commit into
python:mainfrom
gianghungtien:gh-154546-cookie-load-atomic

Conversation

@gianghungtien

@gianghungtien gianghungtien commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

BaseCookie.load() is documented as parse-then-apply — the code even comments that it "first parse[s] the whole cookie string and reject[s] it if it's syntactically invalid". However, the key and attribute legality checks actually run during the apply phase (inside Morsel.set() and Morsel.__setitem__()), not the parse phase. So when an illegal key or an unknown $-prefixed attribute appears part-way through the string, CookieError is raised only after the earlier cookies have already been written into the jar, leaving it in a partially-updated state.

from http.cookies import SimpleCookie, CookieError

C = SimpleCookie()
try:
    C.load("a=1; b,c=2; d=3")
except CookieError:
    pass
print(len(C))  # 1 on main, expected 0

This builds the morsels in a temporary mapping and only commits them to the jar once the whole string has validated, so load() is now all-or-nothing. Existing morsels are copied before being updated, so a failing load() no longer mutates cookies that were already present in the jar. This also covers the sibling case of an unknown $-prefixed attribute (e.g. "a=1; $foo=2"), which had the same partial-application problem.

Regression tests and a Misc/NEWS.d entry are included.

Fixes #154546

BaseCookie.load() parses the whole cookie string before applying it, but
the key and attribute legality checks run during the apply phase (inside
Morsel.set() and Morsel.__setitem__()). An illegal key or unknown
$-prefixed attribute part-way through the string therefore raised
CookieError only after the earlier cookies had already been written to the
jar, leaving it in a partially-updated state.

Build the morsels in a temporary mapping and only commit them to the
cookie jar once the entire string has been validated, so load() is now
all-or-nothing. Existing morsels are copied before being updated, so a
failing load() no longer mutates cookies that were already present.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

http.cookies.BaseCookie.load() applies partial state before raising CookieError

1 participant