Fix CookieError crash with control characters on Python 3.13 by rodrigobnogueira · Pull Request #12395 · aio-libs/aiohttp

rodrigobnogueira · 2026-04-19T21:01:38Z

What do these changes do?

CPython builds that include the CVE-2026-3644 patch add strict validation to Morsel.__setstate__ that rejects values containing ASCII control characters. This causes aiohttp's cookie parser to crash with CookieError in two scenarios:

Octal escape sequences — _unquote decodes sequences like \012 to literal \n, which __setstate__ then rejects.
Literal control characters — A raw header containing e.g. \x07 (BEL) is unsalvageable since both the decoded value and the coded_value contain the control character.

This PR adds a _safe_set_morsel_state helper that:

Wraps __setstate__ and catches CookieError
For case 1: falls back to storing the raw (still-escaped) coded_value as both value and coded_value, preserving the cookie
For case 2: returns False so callers gracefully skip the unsalvageable cookie instead of crashing

Are there changes in behavior for the user?

Yes. On CPython builds with the CVE-2026-3644 patch, when a server sends a cookie containing a control character:

Octal-escaped control chars (e.g. \012): the cookie is preserved with the raw escaped value instead of the decoded one
Literal control chars (e.g. \x07): the cookie is silently skipped

In both cases the parser no longer crashes. On CPython builds without the patch, behavior is unchanged.

Is it a substantial burden for the maintainers to support this?

No. The helper is a thin wrapper around the existing __setstate__ calls, uses the same patterns already established in the codebase, and is well-tested across both patched and unpatched CPython builds.

Related issue number

N/A — This addresses a crash introduced by CPython's CVE-2026-3644 security patch.

Checklist

I think the code is well written
Unit tests for the changes exist
Documentation reflects the changes
If you provide code modification, please add yourself to CONTRIBUTORS.txt
Add a new news fragment into the CHANGES/ folder

for more information, see https://pre-commit.ci

codecov · 2026-04-19T21:07:05Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.92%. Comparing base (53f6e91) to head (861e965).
⚠️ Report is 4 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff            @@
##           master   #12395    +/-   ##
========================================
  Coverage   98.92%   98.92%            
========================================
  Files         134      134            
  Lines       46616    46789   +173     
  Branches     2429     2435     +6     
========================================
+ Hits        46114    46287   +173     
  Misses        373      373            
  Partials      129      129

Flag	Coverage Δ
CI-GHA	`98.98% <98.27%> (+<0.01%)`	⬆️
OS-Linux	`98.72% <98.27%> (+<0.01%)`	⬆️
OS-Windows	`96.99% <98.27%> (+<0.01%)`	⬆️
OS-macOS	`97.89% <98.27%> (+0.01%)`	⬆️
Py-3.10.11	`97.40% <98.27%> (+<0.01%)`	⬆️
Py-3.10.20	`97.86% <98.27%> (+<0.01%)`	⬆️
Py-3.11.15	`98.11% <98.27%> (-0.01%)`	⬇️
Py-3.11.9	`97.65% <98.27%> (+<0.01%)`	⬆️
Py-3.12.10	`97.73% <98.27%> (+<0.01%)`	⬆️
Py-3.12.13	`98.20% <98.27%> (+<0.01%)`	⬆️
Py-3.13.13	`98.44% <96.55%> (-0.01%)`	⬇️
Py-3.14.4	`98.50% <96.55%> (-0.01%)`	⬇️
Py-3.14.4t	`97.51% <96.55%> (+<0.01%)`	⬆️
Py-pypy3.11.15-7.3.21	`97.35% <98.27%> (+<0.01%)`	⬆️
VM-macos	`97.89% <98.27%> (+0.01%)`	⬆️
VM-ubuntu	`98.72% <98.27%> (+<0.01%)`	⬆️
VM-windows	`96.99% <98.27%> (+<0.01%)`	⬆️
cython-coverage	`38.05% <12.06%> (-0.14%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

codspeed-hq · 2026-04-19T21:30:04Z

Merging this PR will not alter performance

✅ 67 untouched benchmarks
⏩ 4 skipped benchmarks¹

_{Comparing rodrigobnogueira:fix-cookie-ctl-chars (861e965) with master (129ef25)}

4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

bdraco · 2026-04-19T22:47:12Z

Tested with Python 3.13.3 and control chars seem to be accepted. Version might need to be narrowed in the summary

bdraco · 2026-04-19T22:49:11Z

python/cpython@57e88c1

looks like something like parse_set_cookie_headers(['name="a\x07b"']) would still raise on a Python with the cve patch landed

Dreamsorcerer · 2026-04-20T01:35:09Z

It also restores the control character test cases that were previously masked.

I think you mean removed. I could see no reason that CTL characters were supposed to be allowed, the test was a generic test of octal unquoting. So my assumption is that the tests arbitrarily chose CTL characters and they were not a deliberate choice, nor did it suggest real cookies would have such characters. It seems fine to me for them to be rejected as a security concern.

…back - _safe_set_morsel_state now returns bool; callers skip unsalvageable cookies - Handles both octal-decoded CTL chars and literal CTL chars in raw headers - Added tests for literal control character edge case (bdraco feedback) - Updated version wording to reference CVE-2026-3644 patch, not Python 3.13+ - Reworded test docstrings per Dreamsorcerer feedback

for more information, see https://pre-commit.ci

rodrigobnogueira · 2026-04-20T03:01:25Z

It also restores the control character test cases that were previously masked.

I think you mean removed. I could see no reason that CTL characters were supposed to be allowed, the test was a generic test of octal unquoting. So my assumption is that the tests arbitrarily chose CTL characters and they were not a deliberate choice, nor did it suggest real cookies would have such characters. It seems fine to me for them to be rejected as a security concern.

Hello @Dreamsorcerer , the tests now describe what they actually verify (graceful handling vs crashing) without implying CTL chars should be allowed.
For context, this crash was originally surfaced by yarl's downstream CI on Python 3.13, where the "Aiohttp tests (3.13)" job failed with CookieError: Control characters are not allowed in cookies on 3 tests.

- Add 'unsalvageable' to docs spelling wordlist (fixes linter) - Add test_parse_cookie_header_literal_ctl_chars for Cookie header path - Remove artificial test_preserve_morsel_with_coded_value_literal_ctl_chars (a Morsel with control chars can't be constructed through normal APIs)

for more information, see https://pre-commit.ci

rodrigobnogueira · 2026-04-20T04:05:01Z

I added a few extra tests to fix the missing coverage reported by Codecov.

rodrigobnogueira · 2026-04-20T04:22:00Z

Pin pyupgrade to python3.13 to work around a crash introduced in Python 3.14 alpha (tokenize.cookie_re was changed from a string to a bytes pattern in 3.14, causing pyupgrade to fail with TypeError: cannot use a bytes pattern on a string-like object when run under 3.14). This is a pyupgrade bug

webknjaz · 2026-04-20T09:17:17Z

Wow, looks like pyupgrade still targets the Python 3.7 syntax. cc @Dreamsorcerer do we still need it that low?

Would it make sense to bump the tool's version in a standalone PR?

webknjaz · 2026-04-20T09:20:55Z

@rodrigobnogueira I haven't found any pyupgrade issue filed upstream? I recommend creating it there and linking from the commit or at least anywhere so it'd be clearer what problem you're referring to. If you can — it'd be a good idea to submit a fix upstream.

webknjaz

Below are a few notes/suggestions, not a full review.

webknjaz · 2026-04-20T10:54:35Z

@@ -0,0 +1,6 @@
+Fixed a crash (``CookieError``) in the cookie parser when receiving cookies
+containing ASCII control characters on CPython builds with the CVE-2026-3644


Sphinx comes with a built-in role for this

Suggested change

containing ASCII control characters on CPython builds with the CVE-2026-3644

containing ASCII control characters on CPython builds with the :cve:`2026-3644`

webknjaz · 2026-04-20T10:55:25Z

@@ -0,0 +1,6 @@
+Fixed a crash (``CookieError``) in the cookie parser when receiving cookies


Would this work?

Suggested change

Fixed a crash (``CookieError``) in the cookie parser when receiving cookies

Fixed a crash (:external+python:exc:`~http.cookies.

CookieError`) in the cookie parser when receiving cookies

webknjaz · 2026-04-20T11:00:10Z

+        (r'name="\012newline\012"', "name", r'"\012newline\012"'),
+        (r'tab="\011separated\011values"', "tab", r'"\011separated\011values"'),
+    ],
+)


Could you add IDs here as a separate arg or right in the params?

webknjaz · 2026-04-20T11:02:49Z

+    result = parse_set_cookie_headers(['name="a\x07b"'])
+    # On CPython with CVE-2026-3644 patch the cookie is skipped;
+    # on older builds it may be accepted.  Either way, no crash.
+    if result:


Let's turn this into a parametrized test with one of the params having a skip mark based on the know CPython range so the runtime is known explicitly.

Done — turned this into a parametrized test with two cases (bel-in-value and bel-with-attribute).

webknjaz · 2026-04-20T11:04:07Z

+    result = parse_set_cookie_headers(['bad="a\x07b"; good=value', "another=cookie"])
+    # "good" is an attribute of "bad" (same header), so it's not a separate cookie.
+    # "another" is in a separate header and must always be preserved.
+    names = [name for name, _ in result]


Would this work with a generator expression?

Suggested change

names = [name for name, _ in result]

names = (name for name, _ in result)

webknjaz · 2026-04-20T11:06:26Z

+    names = [name for name, _ in result]
+    assert "good" in names


Structures like this tend to read better semantically, plus there's no throw-away list creation and the iteration stops early:

Suggested change

names = [name for name, _ in result]

assert "good" in names

assert any(name == "good" for name, _ in result)

webknjaz · 2026-04-20T11:08:28Z

    SimpleCookie,
    _unquote as simplecookie_unquote,
 )
+from unittest.mock import patch


Instead of calling the low-level stdlib interface, a natively integrated way would be using pytest-mock that provides a mocker fixture.

I've never really understood the point of the mocker fixture...

webknjaz · 2026-04-20T11:11:43Z

+        autospec=True,
+        side_effect=_mock_setstate,
+    ):
+        yield


If you migrate to the builtin monkeypatch fixture, this would be enough to move away from the generator to a regular function here. But if you want to use the mocking/spying interface, you could depend on mocker instead.

webknjaz · 2026-04-20T11:14:00Z

+        yield
+
+
+def test_cookie_helpers_cve_fallback(mock_strict_morsel: None) -> None:


When a fixture does not return a usable object, there's no need to inject it as a test function argument only to discard an unused local var. Instead, wire it through a mark as follows:

Suggested change

def test_cookie_helpers_cve_fallback(mock_strict_morsel: None) -> None:

@pytest.mark.usefixtures('mock_strict_morsel')

def test_cookie_helpers_cve_fallback() -> None:

Dreamsorcerer · 2026-04-20T13:51:21Z

Wow, looks like pyupgrade still targets the Python 3.7 syntax. cc @Dreamsorcerer do we still need it that low?

I'm going to replace the whole setup with ruff in the near future, so haven't really looked at it. I did a manual upgrade with ruff a couple of months ago, though I only applied the safe fixes at that time, so still a few outdated bits in the code currently.

Dreamsorcerer · 2026-04-20T13:54:53Z

For context, this crash was originally surfaced by yarl's downstream CI on Python 3.13, where the "Aiohttp tests (3.13)" job failed with CookieError: Control characters are not allowed in cookies on 3 tests.

Yeah, I saw that. That's just because those tests are outdated, right? We can backport the change to 3.13 branch to resolve that.

Dreamsorcerer · 2026-04-20T13:56:38Z

For context, this crash was originally surfaced by yarl's downstream CI on Python 3.13, where the "Aiohttp tests (3.13)" job failed with CookieError: Control characters are not allowed in cookies on 3 tests.

Yeah, I saw that. That's just because those tests are outdated, right? We can backport the change to 3.13 branch to resolve that.

#12401

Dreamsorcerer · 2026-04-20T14:05:47Z

+        )
+    except CookieError:
+        # The decoded value contains control characters rejected after
+        # CVE-2026-3644.  Fall back to keeping the raw coded_value.


I'm still unclear to implications of allowing the coded_value here, rather than just rejecting the cookie entirely.

The fallback to coded_value preserves the cookie with its original escaped representation (e.g. "\012newline\012") instead of dropping it entirely. The coded_value at this point contains only printable ASCII (backslash-escaped octals), so there are no injection or smuggling concerns. If the application later echoes this cookie back via Cookie:, the server receives exactly what it originally set.

If you'd prefer to reject the cookie entirely instead, I can simplify the function to a single try/except that returns False on any CookieError.

I'm still leaning towards rejection. See what @bdraco thinks.

- Use :cve:`2026-3644` and :external+python:exc: roles in changelog - Add pytest IDs to CTL chars from octal parametrize - Parametrize literal CTL char test (was two separate tests) - Use any() instead of list + in for semantic clarity - Replace unittest.mock.patch with monkeypatch fixture (no new dep) - Use @pytest.mark.usefixtures for void fixture injection - Remove pyupgrade language_version: python3.13 pin

Fix CookieError crash with control characters on Python 3.13

17db306

rodrigobnogueira requested a review from asvetlov as a code owner April 19, 2026 21:01

[pre-commit.ci] auto fixes from pre-commit.com hooks

e33ff01

for more information, see https://pre-commit.ci

Add news fragment

5b9a07c

rodrigobnogueira requested a review from webknjaz as a code owner April 19, 2026 21:10

psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Apr 19, 2026

Fix flake8 D301 docstring warning

eee0ad7

bdraco added backport-3.13 Trigger automatic backporting to the 3.13 release branch by Patchback robot backport-3.14 Trigger automatic backporting to the 3.14 release branch by Patchback robot labels Apr 19, 2026

Dreamsorcerer removed the backport-3.13 Trigger automatic backporting to the 3.13 release branch by Patchback robot label Apr 20, 2026

rodrigobnogueira and others added 2 commits April 19, 2026 23:50

[pre-commit.ci] auto fixes from pre-commit.com hooks

5ad7288

for more information, see https://pre-commit.ci

rodrigobnogueira added 2 commits April 20, 2026 00:11

Reword news fragment to avoid spelling issue, restore wordlist

659bc5a

rodrigobnogueira force-pushed the fix-cookie-ctl-chars branch from 2b9987b to 5e12a69 Compare April 20, 2026 03:51

Fix flake8 D301 Use r""" if any backslashes in a docstring

78d8173

rodrigobnogueira force-pushed the fix-cookie-ctl-chars branch from d34ac62 to 78d8173 Compare April 20, 2026 04:02

[pre-commit.ci] auto fixes from pre-commit.com hooks

d6c7ad8

for more information, see https://pre-commit.ci

Pin pyupgrade to Python 3.13 to fix crash on 3.14 alpha

cece051

webknjaz reviewed Apr 20, 2026

View reviewed changes

Dreamsorcerer reviewed Apr 20, 2026

View reviewed changes

rodrigobnogueira force-pushed the fix-cookie-ctl-chars branch from 8a390bc to 90428df Compare April 21, 2026 00:24

rodrigobnogueira force-pushed the fix-cookie-ctl-chars branch from 90428df to 861e965 Compare April 21, 2026 00:32

		@@ -0,0 +1,6 @@
		Fixed a crash (``CookieError``) in the cookie parser when receiving cookies
		containing ASCII control characters on CPython builds with the CVE-2026-3644

	Fixed a crash (``CookieError``) in the cookie parser when receiving cookies
	Fixed a crash (:external+python:exc:`~http.cookies.
	CookieError`) in the cookie parser when receiving cookies

	names = [name for name, _ in result]
	names = (name for name, _ in result)

	names = [name for name, _ in result]
	assert "good" in names
	assert any(name == "good" for name, _ in result)

		yield


		def test_cookie_helpers_cve_fallback(mock_strict_morsel: None) -> None:

	def test_cookie_helpers_cve_fallback(mock_strict_morsel: None) -> None:
	@pytest.mark.usefixtures('mock_strict_morsel')
	def test_cookie_helpers_cve_fallback() -> None:

Uh oh!

Conversation

rodrigobnogueira commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What do these changes do?

Are there changes in behavior for the user?

Is it a substantial burden for the maintainers to support this?

Related issue number

Checklist

Uh oh!

codecov bot commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codspeed-hq bot commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

bdraco commented Apr 19, 2026

Uh oh!

bdraco commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dreamsorcerer commented Apr 20, 2026

Uh oh!

rodrigobnogueira commented Apr 20, 2026

Uh oh!

rodrigobnogueira commented Apr 20, 2026

Uh oh!

rodrigobnogueira commented Apr 20, 2026

Uh oh!

webknjaz commented Apr 20, 2026

Uh oh!

webknjaz commented Apr 20, 2026

Uh oh!

webknjaz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dreamsorcerer commented Apr 20, 2026

Uh oh!

Dreamsorcerer commented Apr 20, 2026

Uh oh!

Dreamsorcerer commented Apr 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

rodrigobnogueira commented Apr 19, 2026 •

edited

Loading

codecov bot commented Apr 19, 2026 •

edited

Loading

codspeed-hq bot commented Apr 19, 2026 •

edited

Loading

bdraco commented Apr 19, 2026 •

edited

Loading