Skip to content

Commit d171cbd

Browse files
Address PR review: improve tests, Sphinx roles, remove pyupgrade pin
- 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
1 parent cece051 commit d171cbd

4 files changed

Lines changed: 42 additions & 52 deletions

File tree

.pre-commit-config.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,6 @@ repos:
103103
hooks:
104104
- id: pyupgrade
105105
args: ['--py37-plus']
106-
language_version: python3.13
107106
- repo: https://github.com/PyCQA/flake8
108107
rev: '7.3.0'
109108
hooks:

CHANGES/12395.bugfix.rst

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
1-
Fixed a crash (``CookieError``) in the cookie parser when receiving cookies
2-
containing ASCII control characters on CPython builds with the CVE-2026-3644
3-
patch. The parser now gracefully falls back to storing the raw, still-escaped
4-
``coded_value`` when the decoded value contains control characters, and skips
5-
cookies whose raw header contains literal control characters that cannot be
6-
safely stored -- by :user:`rodrigobnogueira`.
1+
Fixed a crash (:external+python:exc:`~http.cookies.CookieError`) in the cookie parser when receiving cookies
2+
containing ASCII control characters on CPython builds with the :cve:`2026-3644`
3+
patch. The parser now gracefully skips cookies whose value contains control
4+
characters instead of letting the exception propagate -- by :user:`rodrigobnogueira`.

aiohttp/_cookie_helpers.py

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -93,15 +93,11 @@ def _safe_set_morsel_state(
9393
CPython builds that include the CVE-2026-3644 patch added validation in
9494
``Morsel.__setstate__`` that rejects values containing ASCII control
9595
characters. When ``_unquote`` decodes octal escape sequences
96-
(e.g. ``\012`` → ``\n``) the resulting value may contain such characters.
96+
(e.g. ``\012`` → ``\n``) the resulting value may contain such characters,
97+
causing ``CookieError`` to be raised.
9798
98-
When that happens we fall back to storing the *raw* (still-escaped)
99-
``coded_value`` as both ``value`` and ``coded_value`` so the cookie
100-
is preserved without crashing.
101-
102-
If the ``coded_value`` itself contains literal control characters
103-
(e.g. a raw ``\x07`` in the header), the cookie is unsalvageable and
104-
the function returns ``False`` so the caller can skip it.
99+
In that case the cookie is skipped entirely — the function returns ``False``
100+
so the caller can move on to the next cookie without crashing.
105101
106102
Returns:
107103
True if the morsel state was set successfully, False if the
@@ -112,15 +108,7 @@ def _safe_set_morsel_state(
112108
{"key": key, "value": value, "coded_value": coded_value}
113109
)
114110
except CookieError:
115-
# The decoded value contains control characters rejected after
116-
# CVE-2026-3644. Fall back to keeping the raw coded_value.
117-
try:
118-
morsel.__setstate__( # type: ignore[attr-defined]
119-
{"key": key, "value": coded_value, "coded_value": coded_value}
120-
)
121-
except CookieError:
122-
# coded_value itself has literal control chars — unsalvageable.
123-
return False
111+
return False
124112
return True
125113

126114

tests/test_cookie_helpers.py

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,12 @@
33
import logging
44
import sys
55
import time
6-
import typing
76
from http.cookies import (
87
CookieError,
98
Morsel,
109
SimpleCookie,
1110
_unquote as simplecookie_unquote,
1211
)
13-
from unittest.mock import patch
1412

1513
import pytest
1614

@@ -1139,8 +1137,18 @@ def test_parse_set_cookie_headers_uses_unquote_with_octal(
11391137
@pytest.mark.parametrize(
11401138
("header", "expected_name", "expected_coded"),
11411139
[
1142-
(r'name="\012newline\012"', "name", r'"\012newline\012"'),
1143-
(r'tab="\011separated\011values"', "tab", r'"\011separated\011values"'),
1140+
pytest.param(
1141+
r'name="\012newline\012"',
1142+
"name",
1143+
r'"\012newline\012"',
1144+
id="newline-octal-012",
1145+
),
1146+
pytest.param(
1147+
r'tab="\011separated\011values"',
1148+
"tab",
1149+
r'"\011separated\011values"',
1150+
id="tab-octal-011",
1151+
),
11441152
],
11451153
)
11461154
def test_parse_set_cookie_headers_ctl_chars_from_octal(
@@ -1150,18 +1158,17 @@ def test_parse_set_cookie_headers_ctl_chars_from_octal(
11501158
11511159
CPython builds with the CVE-2026-3644 patch reject control characters in
11521160
cookies. When octal unquoting produces a control character, the parser
1153-
should fall back to the raw coded_value instead of raising CookieError.
1161+
skips the cookie entirely instead of raising CookieError.
11541162
"""
11551163
result = parse_set_cookie_headers([header])
11561164

1157-
assert len(result) == 1
1158-
name, morsel = result[0]
1159-
1160-
assert name == expected_name
1161-
assert morsel.coded_value == expected_coded
1162-
# Depending on CPython build, morsel.value will either be the decoded string
1163-
# (pre CVE-2026-3644 patch) or the raw coded_value (post patch).
1164-
# We just ensure it doesn't crash and the coded_value is preserved.
1165+
# On CPython with CVE-2026-3644 patch the cookie is rejected (result is empty);
1166+
# on older builds it may be accepted with the decoded value.
1167+
# Either way, no crash.
1168+
if result:
1169+
name, morsel = result[0]
1170+
assert name == expected_name
1171+
assert morsel.coded_value == expected_coded
11651172

11661173

11671174
def test_parse_set_cookie_headers_literal_ctl_chars() -> None:
@@ -1183,8 +1190,7 @@ def test_parse_set_cookie_headers_literal_ctl_chars_preserves_others() -> None:
11831190
result = parse_set_cookie_headers(['bad="a\x07b"; good=value', "another=cookie"])
11841191
# "good" is an attribute of "bad" (same header), so it's not a separate cookie.
11851192
# "another" is in a separate header and must always be preserved.
1186-
names = [name for name, _ in result]
1187-
assert "another" in names
1193+
assert any(name == "another" for name, _ in result)
11881194

11891195

11901196
# Tests for parse_cookie_header (RFC 6265 compliant Cookie header parser)
@@ -1660,8 +1666,7 @@ def test_parse_cookie_header_literal_ctl_chars() -> None:
16601666
result = parse_cookie_header('name="a\x07b"; good=cookie')
16611667
# On CPython with CVE-2026-3644 patch the bad cookie is skipped;
16621668
# on older builds it may be accepted. Either way, no crash.
1663-
names = [name for name, _ in result]
1664-
assert "good" in names
1669+
assert any(name == "good" for name, _ in result)
16651670

16661671

16671672
@pytest.mark.parametrize(
@@ -1855,27 +1860,26 @@ def test_unquote_compatibility_with_simplecookie(test_value: str) -> None:
18551860

18561861

18571862
@pytest.fixture
1858-
def mock_strict_morsel() -> typing.Iterator[None]:
1863+
def mock_strict_morsel(
1864+
monkeypatch: pytest.MonkeyPatch,
1865+
) -> None:
18591866
original_setstate = Morsel.__setstate__ # type: ignore[attr-defined]
18601867

18611868
def _mock_setstate(self: Morsel[str], state: dict[str, str]) -> None:
18621869
if any(ord(c) < 32 for c in state.get("value", "")):
18631870
raise CookieError()
18641871
original_setstate(self, state)
18651872

1866-
with patch(
1873+
monkeypatch.setattr(
18671874
"aiohttp._cookie_helpers.Morsel.__setstate__",
1868-
autospec=True,
1869-
side_effect=_mock_setstate,
1870-
):
1871-
yield
1872-
1875+
_mock_setstate,
1876+
)
18731877

1874-
def test_cookie_helpers_cve_fallback(mock_strict_morsel: None) -> None:
1875-
m: Morsel[str] = Morsel()
1876-
assert helpers._safe_set_morsel_state(m, "k", "v\n", "v\\012") is True
1877-
assert m.value == "v\\012"
18781878

1879+
@pytest.mark.usefixtures("mock_strict_morsel")
1880+
def test_cookie_helpers_cve_fallback() -> None:
1881+
# With strict morsel: any CTL char in value → CookieError → rejected
1882+
assert helpers._safe_set_morsel_state(Morsel(), "k", "v\n", "v\\012") is False
18791883
assert helpers._safe_set_morsel_state(Morsel(), "k", "v\n", "v\n") is False
18801884

18811885
cookie: Morsel[str] = Morsel()
@@ -1886,3 +1890,4 @@ def test_cookie_helpers_cve_fallback(mock_strict_morsel: None) -> None:
18861890
assert parse_cookie_header("f=b\x07r") == []
18871891
assert parse_cookie_header('f="b\x07r";') == []
18881892
assert parse_set_cookie_headers(['f="b\x07r";']) == []
1893+
assert parse_set_cookie_headers([r'name="\012newline\012"']) == []

0 commit comments

Comments
 (0)