Ensure cookies are still parsed after a malformed cookie (aio-libs#11724

)

CHANGES/11632.bugfix.rst

@@ -0,0 +1 @@
+Fixed cookie parser to continue parsing subsequent cookies when encountering a malformed cookie that fails regex validation, such as Google's ``g_state`` cookie with unescaped quotes -- by :user:`bdraco`.

aiohttp/_cookie_helpers.py

@@ -166,7 +166,10 @@ def parse_cookie_header(header: str) -> list[tuple[str, Morsel[str]]]:
     attribute names (like 'path' or 'secure') should be treated as cookies.
 
     This parser uses the same regex-based approach as parse_set_cookie_headers
-    to properly handle quoted values that may contain semicolons.
+    to properly handle quoted values that may contain semicolons. When the
+    regex fails to match a malformed cookie, it falls back to simple parsing
+    to ensure subsequent cookies are not lost
+    https://github.com/aio-libs/aiohttp/issues/11632
 
     Args:
         header: The Cookie header value to parse
@@ -177,6 +180,7 @@ def parse_cookie_header(header: str) -> list[tuple[str, Morsel[str]]]:
     if not header:
         return []
 
+    morsel: Morsel[str]
     cookies: list[tuple[str, Morsel[str]]] = []
     i = 0
     n = len(header)
@@ -185,7 +189,32 @@ def parse_cookie_header(header: str) -> list[tuple[str, Morsel[str]]]:
         # Use the same pattern as parse_set_cookie_headers to find cookies
         match = _COOKIE_PATTERN.match(header, i)
         if not match:
-            break
+            # Fallback for malformed cookies https://github.com/aio-libs/aiohttp/issues/11632
+            # Find next semicolon to skip or attempt simple key=value parsing
+            next_semi = header.find(";", i)
+            eq_pos = header.find("=", i)
+
+            # Try to extract key=value if '=' comes before ';'
+            if eq_pos != -1 and (next_semi == -1 or eq_pos < next_semi):
+                end_pos = next_semi if next_semi != -1 else n
+                key = header[i:eq_pos].strip()
+                value = header[eq_pos + 1 : end_pos].strip()
+
+                # Validate the name (same as regex path)
+                if not _COOKIE_NAME_RE.match(key):
+                    internal_logger.warning(
+                        "Can not load cookie: Illegal cookie name %r", key
+                    )
+                else:
+                    morsel = Morsel()
+                    morsel.__setstate__(  # type: ignore[attr-defined]
+                        {"key": key, "value": _unquote(value), "coded_value": value}
+                    )
+                    cookies.append((key, morsel))
+
+            # Move to next cookie or end
+            i = next_semi + 1 if next_semi != -1 else n
+            continue
 
         key = match.group("key")
         value = match.group("val") or ""
@@ -197,7 +226,7 @@ def parse_cookie_header(header: str) -> list[tuple[str, Morsel[str]]]:
             continue
 
         # Create new morsel
-        morsel: Morsel[str] = Morsel()
+        morsel = Morsel()
         # Preserve the original value as coded_value (with quotes if present)
         # We use __setstate__ instead of the public set() API because it allows us to
         # bypass validation and set already validated state. This is more stable than

docs/spelling_wordlist.txt

@@ -341,8 +341,9 @@ tuples
 UI
 un
 unawaited
-undercounting
 unclosed
+undercounting
+unescaped
 unhandled
 unicode
 unittest

tests/test_cookie_helpers.py

@@ -1137,7 +1137,6 @@ def test_parse_cookie_header_empty() -> None:
     assert parse_cookie_header("   ") == []
 
 
-@pytest.mark.xfail(reason="https://github.com/aio-libs/aiohttp/issues/11632")
 def test_parse_cookie_gstate_header() -> None:
     header = (
         "_ga=ga; "
@@ -1444,6 +1443,142 @@ def test_parse_cookie_header_illegal_names(caplog: pytest.LogCaptureFixture) ->
     assert "Can not load cookie: Illegal cookie name 'invalid,cookie'" in caplog.text
 
 
+def test_parse_cookie_header_large_value() -> None:
+    """Test that large cookie values don't cause DoS."""
+    large_value = "A" * 8192
+    header = f"normal=value; large={large_value}; after=cookie"
+
+    result = parse_cookie_header(header)
+    cookie_names = [name for name, _ in result]
+
+    assert len(result) == 3
+    assert "normal" in cookie_names
+    assert "large" in cookie_names
+    assert "after" in cookie_names
+
+    large_cookie = next(morsel for name, morsel in result if name == "large")
+    assert len(large_cookie.value) == 8192
+
+
+def test_parse_cookie_header_multiple_equals() -> None:
+    """Test handling of multiple equals signs in cookie values."""
+    header = "session=abc123; data=key1=val1&key2=val2; token=xyz"
+
+    result = parse_cookie_header(header)
+
+    assert len(result) == 3
+
+    name1, morsel1 = result[0]
+    assert name1 == "session"
+    assert morsel1.value == "abc123"
+
+    name2, morsel2 = result[1]
+    assert name2 == "data"
+    assert morsel2.value == "key1=val1&key2=val2"
+
+    name3, morsel3 = result[2]
+    assert name3 == "token"
+    assert morsel3.value == "xyz"
+
+
+def test_parse_cookie_header_fallback_preserves_subsequent_cookies() -> None:
+    """Test that fallback parser doesn't lose subsequent cookies."""
+    header = 'normal=value; malformed={"json":"value"}; after1=cookie1; after2=cookie2'
+
+    result = parse_cookie_header(header)
+    cookie_names = [name for name, _ in result]
+
+    assert len(result) == 4
+    assert cookie_names == ["normal", "malformed", "after1", "after2"]
+
+    name1, morsel1 = result[0]
+    assert morsel1.value == "value"
+
+    name2, morsel2 = result[1]
+    assert morsel2.value == '{"json":"value"}'
+
+    name3, morsel3 = result[2]
+    assert morsel3.value == "cookie1"
+
+    name4, morsel4 = result[3]
+    assert morsel4.value == "cookie2"
+
+
+def test_parse_cookie_header_whitespace_in_fallback() -> None:
+    """Test that fallback parser handles whitespace correctly."""
+    header = "a=1; b = 2 ; c= 3; d =4"
+
+    result = parse_cookie_header(header)
+
+    assert len(result) == 4
+    for name, morsel in result:
+        assert name in ("a", "b", "c", "d")
+        assert morsel.value in ("1", "2", "3", "4")
+
+
+def test_parse_cookie_header_empty_value_in_fallback() -> None:
+    """Test that fallback handles empty values correctly."""
+    header = "normal=value; empty=; another=test"
+
+    result = parse_cookie_header(header)
+
+    assert len(result) == 3
+
+    name1, morsel1 = result[0]
+    assert name1 == "normal"
+    assert morsel1.value == "value"
+
+    name2, morsel2 = result[1]
+    assert name2 == "empty"
+    assert morsel2.value == ""
+
+    name3, morsel3 = result[2]
+    assert name3 == "another"
+    assert morsel3.value == "test"
+
+
+def test_parse_cookie_header_invalid_name_in_fallback(
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """Test that fallback parser rejects cookies with invalid names."""
+    header = 'normal=value; invalid,name={"x":"y"}; another=test'
+
+    result = parse_cookie_header(header)
+
+    assert len(result) == 2
+
+    name1, morsel1 = result[0]
+    assert name1 == "normal"
+    assert morsel1.value == "value"
+
+    name2, morsel2 = result[1]
+    assert name2 == "another"
+    assert morsel2.value == "test"
+
+    assert "Can not load cookie: Illegal cookie name 'invalid,name'" in caplog.text
+
+
+def test_parse_cookie_header_empty_key_in_fallback(
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """Test that fallback parser logs warning for empty cookie names."""
+    header = 'normal=value; ={"malformed":"json"}; another=test'
+
+    result = parse_cookie_header(header)
+
+    assert len(result) == 2
+
+    name1, morsel1 = result[0]
+    assert name1 == "normal"
+    assert morsel1.value == "value"
+
+    name2, morsel2 = result[1]
+    assert name2 == "another"
+    assert morsel2.value == "test"
+
+    assert "Can not load cookie: Illegal cookie name ''" in caplog.text
+
+
 @pytest.mark.parametrize(
     ("input_str", "expected"),
     [