From 758738ed4d2a3c3d2c74b9d8ba217c7141af1a16 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 2 Jun 2025 03:13:39 -0500 Subject: [PATCH 1/2] Downgrade connector close error to debug (#11114) --- CHANGES/11114.misc.rst | 1 + aiohttp/connector.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 CHANGES/11114.misc.rst diff --git a/CHANGES/11114.misc.rst b/CHANGES/11114.misc.rst new file mode 100644 index 00000000000..2fcb1468c67 --- /dev/null +++ b/CHANGES/11114.misc.rst @@ -0,0 +1 @@ +Downgraded the logging level for connector close errors from ERROR to DEBUG, as these are expected behavior with TLS 1.3 connections -- by :user:`bdraco`. diff --git a/aiohttp/connector.py b/aiohttp/connector.py index 55cfb185f7a..27926d3de88 100644 --- a/aiohttp/connector.py +++ b/aiohttp/connector.py @@ -1,6 +1,5 @@ import asyncio import functools -import logging import random import socket import sys @@ -60,6 +59,7 @@ set_exception, set_result, ) +from .log import client_logger from .resolver import DefaultResolver if sys.version_info >= (3, 12): @@ -439,7 +439,7 @@ async def close(self) -> None: for res in results: if isinstance(res, Exception): err_msg = "Error while closing connector: " + repr(res) - logging.error(err_msg) + client_logger.debug(err_msg) def _close_immediately(self) -> List[Awaitable[object]]: waiters: List[Awaitable[object]] = [] From 8edec635b65035ad819cd98abc7bfeb192f788a3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 2 Jun 2025 03:18:57 -0500 Subject: [PATCH 2/2] Fix cookie parsing issues (#11112) --- CHANGES/11112.bugfix.rst | 8 + CHANGES/2683.bugfix.rst | 1 + CHANGES/5397.bugfix.rst | 1 + CHANGES/7993.bugfix.rst | 1 + aiohttp/_cookie_helpers.py | 221 +++++++ aiohttp/abc.py | 26 +- aiohttp/client_reqrep.py | 21 +- aiohttp/cookiejar.py | 45 +- aiohttp/web_request.py | 9 +- tests/test_client_functional.py | 36 +- tests/test_client_request.py | 45 ++ tests/test_cookie_helpers.py | 1031 +++++++++++++++++++++++++++++++ tests/test_cookiejar.py | 225 ++++++- tests/test_web_request.py | 151 +++++ 14 files changed, 1758 insertions(+), 63 deletions(-) create mode 100644 CHANGES/11112.bugfix.rst create mode 120000 CHANGES/2683.bugfix.rst create mode 120000 CHANGES/5397.bugfix.rst create mode 120000 CHANGES/7993.bugfix.rst create mode 100644 aiohttp/_cookie_helpers.py create mode 100644 tests/test_cookie_helpers.py diff --git a/CHANGES/11112.bugfix.rst b/CHANGES/11112.bugfix.rst new file mode 100644 index 00000000000..6edea1c9b23 --- /dev/null +++ b/CHANGES/11112.bugfix.rst @@ -0,0 +1,8 @@ +Fixed cookie parsing to be more lenient when handling cookies with special characters +in names or values. Cookies with characters like ``{``, ``}``, and ``/`` in names are now +accepted instead of causing a :exc:`~http.cookies.CookieError` and 500 errors. Additionally, +cookies with mismatched quotes in values are now parsed correctly, and quoted cookie +values are now handled consistently whether or not they include special attributes +like ``Domain``. Also fixed :class:`~aiohttp.CookieJar` to ensure shared cookies (domain="", path="") +respect the ``quote_cookie`` parameter, making cookie quoting behavior consistent for +all cookies -- by :user:`bdraco`. diff --git a/CHANGES/2683.bugfix.rst b/CHANGES/2683.bugfix.rst new file mode 120000 index 00000000000..fac3861027d --- /dev/null +++ b/CHANGES/2683.bugfix.rst @@ -0,0 +1 @@ +11112.bugfix.rst \ No newline at end of file diff --git a/CHANGES/5397.bugfix.rst b/CHANGES/5397.bugfix.rst new file mode 120000 index 00000000000..fac3861027d --- /dev/null +++ b/CHANGES/5397.bugfix.rst @@ -0,0 +1 @@ +11112.bugfix.rst \ No newline at end of file diff --git a/CHANGES/7993.bugfix.rst b/CHANGES/7993.bugfix.rst new file mode 120000 index 00000000000..fac3861027d --- /dev/null +++ b/CHANGES/7993.bugfix.rst @@ -0,0 +1 @@ +11112.bugfix.rst \ No newline at end of file diff --git a/aiohttp/_cookie_helpers.py b/aiohttp/_cookie_helpers.py new file mode 100644 index 00000000000..8184cc9bdc1 --- /dev/null +++ b/aiohttp/_cookie_helpers.py @@ -0,0 +1,221 @@ +""" +Internal cookie handling helpers. + +This module contains internal utilities for cookie parsing and manipulation. +These are not part of the public API and may change without notice. +""" + +import re +import sys +from http.cookies import Morsel +from typing import List, Optional, Sequence, Tuple, cast + +from .log import internal_logger + +__all__ = ("parse_cookie_headers", "preserve_morsel_with_coded_value") + +# Cookie parsing constants +# Allow more characters in cookie names to handle real-world cookies +# that don't strictly follow RFC standards (fixes #2683) +# RFC 6265 defines cookie-name token as per RFC 2616 Section 2.2, +# but many servers send cookies with characters like {} [] () etc. +# This makes the cookie parser more tolerant of real-world cookies +# while still providing some validation to catch obviously malformed names. +_COOKIE_NAME_RE = re.compile(r"^[!#$%&\'()*+\-./0-9:<=>?@A-Z\[\]^_`a-z{|}~]+$") +_COOKIE_KNOWN_ATTRS = frozenset( # AKA Morsel._reserved + ( + "path", + "domain", + "max-age", + "expires", + "secure", + "httponly", + "samesite", + "partitioned", + "version", + "comment", + ) +) +_COOKIE_BOOL_ATTRS = frozenset( # AKA Morsel._flags + ("secure", "httponly", "partitioned") +) + +# SimpleCookie's pattern for parsing cookies with relaxed validation +# Based on http.cookies pattern but extended to allow more characters in cookie names +# to handle real-world cookies (fixes #2683) +_COOKIE_PATTERN = re.compile( + r""" + \s* # Optional whitespace at start of cookie + (?P # Start of group 'key' + # aiohttp has extended to include [] for compatibility with real-world cookies + [\w\d!#%&'~_`><@,:/\$\*\+\-\.\^\|\)\(\?\}\{\=\[\]]+? # Any word of at least one letter + ) # End of group 'key' + ( # Optional group: there may not be a value. + \s*=\s* # Equal Sign + (?P # Start of group 'val' + "(?:[^\\"]|\\.)*" # Any double-quoted string (properly closed) + | # or + "[^";]* # Unmatched opening quote (differs from SimpleCookie - issue #7993) + | # or + # Special case for "expires" attr - RFC 822, RFC 850, RFC 1036, RFC 1123 + (\w{3,6}day|\w{3}),\s # Day of the week or abbreviated day (with comma) + [\w\d\s-]{9,11}\s[\d:]{8}\s # Date and time in specific format + (GMT|[+-]\d{4}) # Timezone: GMT or RFC 2822 offset like -0000, +0100 + # NOTE: RFC 2822 timezone support is an aiohttp extension + # for issue #4493 - SimpleCookie does NOT support this + | # or + # ANSI C asctime() format: "Wed Jun 9 10:18:14 2021" + # NOTE: This is an aiohttp extension for issue #4327 - SimpleCookie does NOT support this format + \w{3}\s+\w{3}\s+[\s\d]\d\s+\d{2}:\d{2}:\d{2}\s+\d{4} + | # or + [\w\d!#%&'~_`><@,:/\$\*\+\-\.\^\|\)\(\?\}\{\=\[\]]* # Any word or empty string + ) # End of group 'val' + )? # End of optional value group + \s* # Any number of spaces. + (\s+|;|$) # Ending either at space, semicolon, or EOS. + """, + re.VERBOSE | re.ASCII, +) + + +def preserve_morsel_with_coded_value(cookie: Morsel[str]) -> Morsel[str]: + """ + Preserve a Morsel's coded_value exactly as received from the server. + + This function ensures that cookie encoding is preserved exactly as sent by + the server, which is critical for compatibility with old servers that have + strict requirements about cookie formats. + + This addresses the issue described in https://github.com/aio-libs/aiohttp/pull/1453 + where Python's SimpleCookie would re-encode cookies, breaking authentication + with certain servers. + + Args: + cookie: A Morsel object from SimpleCookie + + Returns: + A Morsel object with preserved coded_value + + """ + mrsl_val = cast("Morsel[str]", cookie.get(cookie.key, Morsel())) + # 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 + # setting protected attributes directly and unlikely to change since it would + # break pickling. + mrsl_val.__setstate__( # type: ignore[attr-defined] + {"key": cookie.key, "value": cookie.value, "coded_value": cookie.coded_value} + ) + return mrsl_val + + +def _unquote(text: str) -> str: + """ + Unquote a cookie value. + + Vendored from http.cookies._unquote to ensure compatibility. + """ + # If there are no quotes, return as-is + if len(text) < 2 or text[0] != '"' or text[-1] != '"': + return text + # Remove quotes and handle escaped characters + text = text[1:-1] + # Replace escaped quotes and backslashes + text = text.replace('\\"', '"').replace("\\\\", "\\") + return text + + +def parse_cookie_headers(headers: Sequence[str]) -> List[Tuple[str, Morsel[str]]]: + """ + Parse cookie headers using a vendored version of SimpleCookie parsing. + + This implementation is based on SimpleCookie.__parse_string to ensure + compatibility with how SimpleCookie parses cookies, including handling + of malformed cookies with missing semicolons. + + This function is used for both Cookie and Set-Cookie headers in order to be + forgiving. Ideally we would have followed RFC 6265 Section 5.2 (for Cookie + headers) and RFC 6265 Section 4.2.1 (for Set-Cookie headers), but the + real world data makes it impossible since we need to be a bit more forgiving. + + NOTE: This implementation differs from SimpleCookie in handling unmatched quotes. + SimpleCookie will stop parsing when it encounters a cookie value with an unmatched + quote (e.g., 'cookie="value'), causing subsequent cookies to be silently dropped. + This implementation handles unmatched quotes more gracefully to prevent cookie loss. + See https://github.com/aio-libs/aiohttp/issues/7993 + """ + parsed_cookies: List[Tuple[str, Morsel[str]]] = [] + + for header in headers: + if not header: + continue + + # Parse cookie string using SimpleCookie's algorithm + i = 0 + n = len(header) + current_morsel: Optional[Morsel[str]] = None + morsel_seen = False + + while 0 <= i < n: + # Start looking for a cookie + match = _COOKIE_PATTERN.match(header, i) + if not match: + # No more cookies + break + + key, value = match.group("key"), match.group("val") + i = match.end(0) + lower_key = key.lower() + + if key[0] == "$": + if not morsel_seen: + # We ignore attributes which pertain to the cookie + # mechanism as a whole, such as "$Version". + continue + # Process as attribute + if current_morsel is not None: + attr_lower_key = lower_key[1:] + if attr_lower_key in _COOKIE_KNOWN_ATTRS: + current_morsel[attr_lower_key] = value or "" + elif lower_key in _COOKIE_KNOWN_ATTRS: + if not morsel_seen: + # Invalid cookie string - attribute before cookie + break + if lower_key in _COOKIE_BOOL_ATTRS: + # Boolean attribute with any value should be True + if current_morsel is not None: + if lower_key == "partitioned" and sys.version_info < (3, 14): + dict.__setitem__(current_morsel, lower_key, True) + else: + current_morsel[lower_key] = True + elif value is None: + # Invalid cookie string - non-boolean attribute without value + break + elif current_morsel is not None: + # Regular attribute with value + current_morsel[lower_key] = _unquote(value) + elif value is not None: + # This is a cookie name=value pair + # Validate the name + if key in _COOKIE_KNOWN_ATTRS or not _COOKIE_NAME_RE.match(key): + internal_logger.warning( + "Can not load cookies: Illegal cookie name %r", key + ) + current_morsel = None + else: + # Create new morsel + current_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 + # setting protected attributes directly and unlikely to change since it would + # break pickling. + current_morsel.__setstate__( # type: ignore[attr-defined] + {"key": key, "value": _unquote(value), "coded_value": value} + ) + parsed_cookies.append((key, current_morsel)) + morsel_seen = True + else: + # Invalid cookie string - no value for non-attribute + break + + return parsed_cookies diff --git a/aiohttp/abc.py b/aiohttp/abc.py index 0b584eba4d3..0f396414a8e 100644 --- a/aiohttp/abc.py +++ b/aiohttp/abc.py @@ -2,7 +2,7 @@ import socket from abc import ABC, abstractmethod from collections.abc import Sized -from http.cookies import BaseCookie, CookieError, Morsel, SimpleCookie +from http.cookies import BaseCookie, Morsel from typing import ( TYPE_CHECKING, Any, @@ -22,7 +22,7 @@ from multidict import CIMultiDict from yarl import URL -from .log import client_logger +from ._cookie_helpers import parse_cookie_headers from .typedefs import LooseCookies if TYPE_CHECKING: @@ -193,26 +193,8 @@ def update_cookies(self, cookies: LooseCookies, response_url: URL = URL()) -> No def update_cookies_from_headers( self, headers: Sequence[str], response_url: URL ) -> None: - """ - Update cookies from raw Set-Cookie headers. - - Default implementation parses each header separately to preserve - cookies with same name but different domain/path. - """ - # Default implementation for backward compatibility - cookies_to_update: List[Tuple[str, Morsel[str]]] = [] - for cookie_header in headers: - tmp_cookie = SimpleCookie() - try: - tmp_cookie.load(cookie_header) - # Collect all cookies as tuples (name, morsel) - for name, morsel in tmp_cookie.items(): - cookies_to_update.append((name, morsel)) - except CookieError as exc: - client_logger.warning("Can not load response cookies: %s", exc) - - # Update all cookies at once for efficiency - if cookies_to_update: + """Update cookies from raw Set-Cookie headers.""" + if headers and (cookies_to_update := parse_cookie_headers(headers)): self.update_cookies(cookies_to_update, response_url) @abstractmethod diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 957d8c25562..88f81326215 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -9,7 +9,7 @@ import warnings from collections.abc import Mapping from hashlib import md5, sha1, sha256 -from http.cookies import CookieError, Morsel, SimpleCookie +from http.cookies import Morsel, SimpleCookie from types import MappingProxyType, TracebackType from typing import ( TYPE_CHECKING, @@ -30,6 +30,7 @@ from yarl import URL from . import hdrs, helpers, http, multipart, payload +from ._cookie_helpers import parse_cookie_headers, preserve_morsel_with_coded_value from .abc import AbstractStreamWriter from .client_exceptions import ( ClientConnectionError, @@ -64,7 +65,6 @@ HttpVersion11, StreamWriter, ) -from .log import client_logger from .streams import StreamReader from .typedefs import ( DEFAULT_JSON_DECODER, @@ -313,11 +313,9 @@ def cookies(self) -> SimpleCookie: if self._raw_cookie_headers is not None: # Parse cookies for response.cookies (SimpleCookie for backward compatibility) cookies = SimpleCookie() - for hdr in self._raw_cookie_headers: - try: - cookies.load(hdr) - except CookieError as exc: - client_logger.warning("Can not load response cookies: %s", exc) + # Use parse_cookie_headers for more lenient parsing that handles + # malformed cookies better than SimpleCookie.load + cookies.update(parse_cookie_headers(self._raw_cookie_headers)) self._cookies = cookies else: self._cookies = SimpleCookie() @@ -1016,7 +1014,8 @@ def update_cookies(self, cookies: Optional[LooseCookies]) -> None: c = SimpleCookie() if hdrs.COOKIE in self.headers: - c.load(self.headers.get(hdrs.COOKIE, "")) + # parse_cookie_headers already preserves coded values + c.update(parse_cookie_headers((self.headers.get(hdrs.COOKIE, ""),))) del self.headers[hdrs.COOKIE] if isinstance(cookies, Mapping): @@ -1025,10 +1024,8 @@ def update_cookies(self, cookies: Optional[LooseCookies]) -> None: iter_cookies = cookies # type: ignore[assignment] for name, value in iter_cookies: if isinstance(value, Morsel): - # Preserve coded_value - mrsl_val = value.get(value.key, Morsel()) - mrsl_val.set(value.key, value.value, value.coded_value) - c[name] = mrsl_val + # Use helper to preserve coded_value exactly as sent by server + c[name] = preserve_morsel_with_coded_value(value) else: c[name] = value # type: ignore[assignment] diff --git a/aiohttp/cookiejar.py b/aiohttp/cookiejar.py index 50ae9b055b0..7e8b206076e 100644 --- a/aiohttp/cookiejar.py +++ b/aiohttp/cookiejar.py @@ -23,11 +23,11 @@ Set, Tuple, Union, - cast, ) from yarl import URL +from ._cookie_helpers import preserve_morsel_with_coded_value from .abc import AbstractCookieJar, ClearCookiePredicate from .helpers import is_ip_address from .typedefs import LooseCookies, PathLike, StrOrURL @@ -45,6 +45,7 @@ # the expiration heap. This is a performance optimization to avoid cleaning up the # heap too often when there are only a few scheduled expirations. _MIN_SCHEDULED_COOKIE_EXPIRATION = 100 +_SIMPLE_COOKIE = SimpleCookie() class CookieJar(AbstractCookieJar): @@ -313,9 +314,10 @@ def filter_cookies(self, request_url: URL) -> "BaseCookie[str]": DeprecationWarning, ) request_url = URL(request_url) - filtered: Union[SimpleCookie, "BaseCookie[str]"] = ( - SimpleCookie() if self._quote_cookie else BaseCookie() - ) + # We always use BaseCookie now since all + # cookies set on on filtered are fully constructed + # Morsels, not just names and values. + filtered: BaseCookie[str] = BaseCookie() if not self._cookies: # Skip do_expiration() if there are no cookies. return filtered @@ -333,8 +335,17 @@ def filter_cookies(self, request_url: URL) -> "BaseCookie[str]": is_not_secure = request_origin not in self._treat_as_secure_origin # Send shared cookie - for c in self._cookies[("", "")].values(): - filtered[c.key] = c.value + key = ("", "") + for c in self._cookies[key].values(): + # Check cache first + if c.key in self._morsel_cache[key]: + filtered[c.key] = self._morsel_cache[key][c.key] + continue + + # Build and cache the morsel + mrsl_val = self._build_morsel(c) + self._morsel_cache[key][c.key] = mrsl_val + filtered[c.key] = mrsl_val if is_ip_address(hostname): if not self._unsafe: @@ -374,15 +385,29 @@ def filter_cookies(self, request_url: URL) -> "BaseCookie[str]": filtered[name] = self._morsel_cache[p][name] continue - # It's critical we use the Morsel so the coded_value - # (based on cookie version) is preserved - mrsl_val = cast("Morsel[str]", cookie.get(cookie.key, Morsel())) - mrsl_val.set(cookie.key, cookie.value, cookie.coded_value) + # Build and cache the morsel + mrsl_val = self._build_morsel(cookie) self._morsel_cache[p][name] = mrsl_val filtered[name] = mrsl_val return filtered + def _build_morsel(self, cookie: Morsel[str]) -> Morsel[str]: + """Build a morsel for sending, respecting quote_cookie setting.""" + if self._quote_cookie and cookie.coded_value and cookie.coded_value[0] == '"': + return preserve_morsel_with_coded_value(cookie) + morsel: Morsel[str] = Morsel() + if self._quote_cookie: + value, coded_value = _SIMPLE_COOKIE.value_encode(cookie.value) + else: + coded_value = value = cookie.value + # 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 + # setting protected attributes directly and unlikely to change since it would + # break pickling. + morsel.__setstate__({"key": cookie.key, "value": value, "coded_value": coded_value}) # type: ignore[attr-defined] + return morsel + @staticmethod def _is_domain_match(domain: str, hostname: str) -> bool: """Implements domain matching adhering to RFC 6265.""" diff --git a/aiohttp/web_request.py b/aiohttp/web_request.py index 48cf0d2f229..dfd5a530e3b 100644 --- a/aiohttp/web_request.py +++ b/aiohttp/web_request.py @@ -7,7 +7,6 @@ import sys import tempfile import types -from http.cookies import SimpleCookie from types import MappingProxyType from typing import ( TYPE_CHECKING, @@ -29,6 +28,7 @@ from yarl import URL from . import hdrs +from ._cookie_helpers import parse_cookie_headers from .abc import AbstractStreamWriter from .helpers import ( _SENTINEL, @@ -556,9 +556,10 @@ def cookies(self) -> Mapping[str, str]: A read-only dictionary-like object. """ - raw = self.headers.get(hdrs.COOKIE, "") - parsed = SimpleCookie(raw) - return MappingProxyType({key: val.value for key, val in parsed.items()}) + # Use parse_cookie_headers for more lenient parsing that accepts + # special characters in cookie names (fixes #2683) + parsed = parse_cookie_headers((self.headers.get(hdrs.COOKIE, ""),)) + return MappingProxyType({name: morsel.value for name, morsel in parsed}) @reify def http_range(self) -> "slice[int, int, int]": diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index d5087ec7c52..caed7286c91 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -5,6 +5,7 @@ import http.cookies import io import json +import logging import pathlib import socket import ssl @@ -2691,15 +2692,16 @@ async def handler(request: web.Request) -> web.Response: assert 200 == resp.status -async def test_set_cookies(aiohttp_client: AiohttpClient) -> None: +async def test_set_cookies( + aiohttp_client: AiohttpClient, caplog: pytest.LogCaptureFixture +) -> None: async def handler(request: web.Request) -> web.Response: ret = web.Response() ret.set_cookie("c1", "cookie1") ret.set_cookie("c2", "cookie2") ret.headers.add( "Set-Cookie", - "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=" - "{925EC0B8-CB17-4BEB-8A35-1033813B0523}; " + "invalid,cookie=value; " # Comma character is not allowed "HttpOnly; Path=/", ) return ret @@ -2708,14 +2710,38 @@ async def handler(request: web.Request) -> web.Response: app.router.add_get("/", handler) client = await aiohttp_client(app) - with mock.patch("aiohttp.client_reqrep.client_logger") as m_log: + with caplog.at_level(logging.WARNING): async with client.get("/") as resp: assert 200 == resp.status cookie_names = {c.key for c in client.session.cookie_jar} _ = resp.cookies assert cookie_names == {"c1", "c2"} - m_log.warning.assert_called_with("Can not load response cookies: %s", mock.ANY) + assert "Can not load cookies: Illegal cookie name 'invalid,cookie'" in caplog.text + + +async def test_set_cookies_with_curly_braces(aiohttp_client: AiohttpClient) -> None: + """Test that cookies with curly braces in names are now accepted (#2683).""" + + async def handler(request: web.Request) -> web.Response: + ret = web.Response() + ret.set_cookie("c1", "cookie1") + ret.headers.add( + "Set-Cookie", + "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=" + "{925EC0B8-CB17-4BEB-8A35-1033813B0523}; " + "HttpOnly; Path=/", + ) + return ret + + app = web.Application() + app.router.add_get("/", handler) + client = await aiohttp_client(app) + + async with client.get("/") as resp: + assert 200 == resp.status + cookie_names = {c.key for c in client.session.cookie_jar} + assert cookie_names == {"c1", "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}"} async def test_set_cookies_expired(aiohttp_client: AiohttpClient) -> None: diff --git a/tests/test_client_request.py b/tests/test_client_request.py index 99d0722e1c7..f51aea2e8e9 100644 --- a/tests/test_client_request.py +++ b/tests/test_client_request.py @@ -599,6 +599,51 @@ def test_cookie_coded_value_preserved(loop: asyncio.AbstractEventLoop) -> None: assert req.headers["COOKIE"] == 'ip-cookie="second"' +def test_update_cookies_with_special_chars_in_existing_header( + loop: asyncio.AbstractEventLoop, +) -> None: + """Test that update_cookies handles existing cookies with special characters.""" + # Create request with a cookie that has special characters (real-world example) + req = ClientRequest( + "get", + URL("http://python.org"), + headers={"Cookie": "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=value1"}, + loop=loop, + ) + + # Update with another cookie + req.update_cookies(cookies={"normal_cookie": "value2"}) + + # Both cookies should be preserved in the exact order + assert ( + req.headers["COOKIE"] + == "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=value1; normal_cookie=value2" + ) + + +def test_update_cookies_with_quoted_existing_header( + loop: asyncio.AbstractEventLoop, +) -> None: + """Test that update_cookies handles existing cookies with quoted values.""" + # Create request with cookies that have quoted values + req = ClientRequest( + "get", + URL("http://python.org"), + headers={"Cookie": 'session="value;with;semicolon"; token=abc123'}, + loop=loop, + ) + + # Update with another cookie + req.update_cookies(cookies={"new_cookie": "new_value"}) + + # All cookies should be preserved with their original coded values + # The quoted value should be preserved as-is + assert ( + req.headers["COOKIE"] + == 'new_cookie=new_value; session="value;with;semicolon"; token=abc123' + ) + + async def test_connection_header( loop: asyncio.AbstractEventLoop, conn: mock.Mock ) -> None: diff --git a/tests/test_cookie_helpers.py b/tests/test_cookie_helpers.py new file mode 100644 index 00000000000..7a2ac7493ee --- /dev/null +++ b/tests/test_cookie_helpers.py @@ -0,0 +1,1031 @@ +"""Tests for internal cookie helper functions.""" + +from http.cookies import CookieError, Morsel, SimpleCookie + +import pytest + +from aiohttp import _cookie_helpers as helpers +from aiohttp._cookie_helpers import ( + parse_cookie_headers, + preserve_morsel_with_coded_value, +) + + +def test_known_attrs_is_superset_of_morsel_reserved() -> None: + """Test that _COOKIE_KNOWN_ATTRS contains all Morsel._reserved attributes.""" + # Get Morsel._reserved attributes (lowercase) + morsel_reserved = {attr.lower() for attr in Morsel._reserved} # type: ignore[attr-defined] + + # _COOKIE_KNOWN_ATTRS should be a superset of morsel_reserved + assert ( + helpers._COOKIE_KNOWN_ATTRS >= morsel_reserved + ), f"_COOKIE_KNOWN_ATTRS is missing: {morsel_reserved - helpers._COOKIE_KNOWN_ATTRS}" + + +def test_bool_attrs_is_superset_of_morsel_flags() -> None: + """Test that _COOKIE_BOOL_ATTRS contains all Morsel._flags attributes.""" + # Get Morsel._flags attributes (lowercase) + morsel_flags = {attr.lower() for attr in Morsel._flags} # type: ignore[attr-defined] + + # _COOKIE_BOOL_ATTRS should be a superset of morsel_flags + assert ( + helpers._COOKIE_BOOL_ATTRS >= morsel_flags + ), f"_COOKIE_BOOL_ATTRS is missing: {morsel_flags - helpers._COOKIE_BOOL_ATTRS}" + + +def test_preserve_morsel_with_coded_value() -> None: + """Test preserve_morsel_with_coded_value preserves coded_value exactly.""" + # Create a cookie with a coded_value different from value + cookie: Morsel[str] = Morsel() + cookie.set("test_cookie", "decoded value", "encoded%20value") + + # Preserve the coded_value + result = preserve_morsel_with_coded_value(cookie) + + # Check that all values are preserved + assert result.key == "test_cookie" + assert result.value == "decoded value" + assert result.coded_value == "encoded%20value" + + # Should be a different Morsel instance + assert result is not cookie + + +def test_preserve_morsel_with_coded_value_no_coded_value() -> None: + """Test preserve_morsel_with_coded_value when coded_value is same as value.""" + cookie: Morsel[str] = Morsel() + cookie.set("test_cookie", "simple_value", "simple_value") + + result = preserve_morsel_with_coded_value(cookie) + + assert result.key == "test_cookie" + assert result.value == "simple_value" + assert result.coded_value == "simple_value" + + +def test_parse_cookie_headers_simple() -> None: + """Test parse_cookie_headers with simple cookies.""" + headers = ["name=value", "session=abc123"] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + assert result[0][0] == "name" + assert result[0][1].key == "name" + assert result[0][1].value == "value" + assert result[1][0] == "session" + assert result[1][1].key == "session" + assert result[1][1].value == "abc123" + + +def test_parse_cookie_headers_with_attributes() -> None: + """Test parse_cookie_headers with cookie attributes.""" + headers = [ + "sessionid=value123; Path=/; HttpOnly; Secure", + "user=john; Domain=.example.com; Max-Age=3600", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + + # First cookie + name1, morsel1 = result[0] + assert name1 == "sessionid" + assert morsel1.value == "value123" + assert morsel1["path"] == "/" + assert morsel1["httponly"] is True + assert morsel1["secure"] is True + + # Second cookie + name2, morsel2 = result[1] + assert name2 == "user" + assert morsel2.value == "john" + assert morsel2["domain"] == ".example.com" + assert morsel2["max-age"] == "3600" + + +def test_parse_cookie_headers_special_chars_in_names() -> None: + """Test parse_cookie_headers accepts special characters in names (#2683).""" + # These should be accepted with relaxed validation + headers = [ + "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=value1", + "cookie[index]=value2", + "cookie(param)=value3", + "cookie:name=value4", + "cookie@domain=value5", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 5 + expected_names = [ + "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}", + "cookie[index]", + "cookie(param)", + "cookie:name", + "cookie@domain", + ] + + for i, (name, morsel) in enumerate(result): + assert name == expected_names[i] + assert morsel.key == expected_names[i] + assert morsel.value == f"value{i+1}" + + +def test_parse_cookie_headers_invalid_names() -> None: + """Test parse_cookie_headers rejects truly invalid cookie names.""" + # These should be rejected even with relaxed validation + headers = [ + "invalid\tcookie=value", # Tab character + "invalid\ncookie=value", # Newline + "invalid\rcookie=value", # Carriage return + "\x00badname=value", # Null character + "name with spaces=value", # Spaces in name + ] + + result = parse_cookie_headers(headers) + + # All should be skipped + assert len(result) == 0 + + +def test_parse_cookie_headers_empty_and_invalid() -> None: + """Test parse_cookie_headers handles empty and invalid formats.""" + headers = [ + "", # Empty header + " ", # Whitespace only + "=value", # No name + "name=", # Empty value (should be accepted) + "justname", # No value (should be skipped) + "path=/", # Reserved attribute as name (should be skipped) + "Domain=.com", # Reserved attribute as name (should be skipped) + ] + + result = parse_cookie_headers(headers) + + # Only "name=" should be accepted + assert len(result) == 1 + assert result[0][0] == "name" + assert result[0][1].value == "" + + +def test_parse_cookie_headers_quoted_values() -> None: + """Test parse_cookie_headers handles quoted values correctly.""" + headers = [ + 'name="quoted value"', + 'session="with;semicolon"', + 'data="with\\"escaped\\""', + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 3 + assert result[0][1].value == "quoted value" + assert result[1][1].value == "with;semicolon" + assert result[2][1].value == 'with"escaped"' + + +@pytest.mark.parametrize( + "header", + [ + 'session="abc;xyz"; token=123', + 'data="value;with;multiple;semicolons"; next=cookie', + 'complex="a=b;c=d"; simple=value', + ], +) +def test_parse_cookie_headers_semicolon_in_quoted_values(header: str) -> None: + """ + Test that semicolons inside properly quoted values are handled correctly. + + Cookie values can contain semicolons when properly quoted. This test ensures + that our parser handles these cases correctly, matching SimpleCookie behavior. + """ + # Test with SimpleCookie + sc = SimpleCookie() + sc.load(header) + + # Test with our parser + result = parse_cookie_headers([header]) + + # Should parse the same number of cookies + assert len(result) == len(sc) + + # Verify each cookie matches SimpleCookie + for (name, morsel), (sc_name, sc_morsel) in zip(result, sc.items()): + assert name == sc_name + assert morsel.value == sc_morsel.value + + +def test_parse_cookie_headers_multiple_cookies_same_header() -> None: + """Test parse_cookie_headers with multiple cookies in one header.""" + # Note: SimpleCookie includes the comma as part of the first cookie's value + headers = ["cookie1=value1, cookie2=value2"] + + result = parse_cookie_headers(headers) + + # Should parse as two separate cookies + assert len(result) == 2 + assert result[0][0] == "cookie1" + assert result[0][1].value == "value1," # Comma is included in the value + assert result[1][0] == "cookie2" + assert result[1][1].value == "value2" + + +@pytest.mark.parametrize( + "header", + [ + # Standard cookies + "session=abc123", + "user=john; Path=/", + "token=xyz; Secure; HttpOnly", + # Empty values + "empty=", + # Quoted values + 'quoted="value with spaces"', + # Multiple attributes + "complex=value; Domain=.example.com; Path=/app; Max-Age=3600", + ], +) +def test_parse_cookie_headers_compatibility_with_simple_cookie(header: str) -> None: + """Test parse_cookie_headers is bug-for-bug compatible with SimpleCookie.load.""" + # Parse with SimpleCookie + sc = SimpleCookie() + sc.load(header) + + # Parse with our function + result = parse_cookie_headers([header]) + + # Should have same number of cookies + assert len(result) == len(sc) + + # Compare each cookie + for name, morsel in result: + assert name in sc + sc_morsel = sc[name] + + # Compare values + assert morsel.value == sc_morsel.value + assert morsel.key == sc_morsel.key + + # Compare attributes (only those that SimpleCookie would set) + for attr in ["path", "domain", "max-age"]: + assert morsel.get(attr) == sc_morsel.get(attr) + + # Boolean attributes are handled differently + # SimpleCookie sets them to empty string when not present, True when present + for bool_attr in ["secure", "httponly"]: + # Only check if SimpleCookie has the attribute set to True + if sc_morsel.get(bool_attr) is True: + assert morsel.get(bool_attr) is True + + +def test_parse_cookie_headers_relaxed_validation_differences() -> None: + """Test where parse_cookie_headers differs from SimpleCookie (relaxed validation).""" + # Test cookies that SimpleCookie rejects with CookieError + rejected_by_simplecookie = [ + ("cookie{with}braces=value1", "cookie{with}braces", "value1"), + ("cookie(with)parens=value3", "cookie(with)parens", "value3"), + ("cookie@with@at=value5", "cookie@with@at", "value5"), + ] + + for header, expected_name, expected_value in rejected_by_simplecookie: + # SimpleCookie should reject these with CookieError + sc = SimpleCookie() + with pytest.raises(CookieError): + sc.load(header) + + # Our parser should accept them + result = parse_cookie_headers([header]) + assert len(result) == 1 # We accept + assert result[0][0] == expected_name + assert result[0][1].value == expected_value + + # Test cookies that SimpleCookie accepts (but we handle more consistently) + accepted_by_simplecookie = [ + ("cookie[with]brackets=value2", "cookie[with]brackets", "value2"), + ("cookie:with:colons=value4", "cookie:with:colons", "value4"), + ] + + for header, expected_name, expected_value in accepted_by_simplecookie: + # SimpleCookie accepts these + sc = SimpleCookie() + sc.load(header) + # May or may not parse correctly in SimpleCookie + + # Our parser should accept them consistently + result = parse_cookie_headers([header]) + assert len(result) == 1 + assert result[0][0] == expected_name + assert result[0][1].value == expected_value + + +def test_parse_cookie_headers_case_insensitive_attrs() -> None: + """Test that known attributes are handled case-insensitively.""" + headers = [ + "cookie1=value1; PATH=/test; DOMAIN=example.com", + "cookie2=value2; Secure; HTTPONLY; max-AGE=60", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + + # First cookie - attributes should be recognized despite case + assert result[0][1]["path"] == "/test" + assert result[0][1]["domain"] == "example.com" + + # Second cookie + assert result[1][1]["secure"] is True + assert result[1][1]["httponly"] is True + assert result[1][1]["max-age"] == "60" + + +def test_parse_cookie_headers_unknown_attrs_ignored() -> None: + """Test that unknown attributes are treated as new cookies (same as SimpleCookie).""" + headers = [ + "cookie=value; Path=/; unknownattr=ignored; HttpOnly", + ] + + result = parse_cookie_headers(headers) + + # SimpleCookie treats unknown attributes with values as new cookies + assert len(result) == 2 + + # First cookie + assert result[0][0] == "cookie" + assert result[0][1]["path"] == "/" + assert result[0][1]["httponly"] == "" # Not set on first cookie + + # Second cookie (the unknown attribute) + assert result[1][0] == "unknownattr" + assert result[1][1].value == "ignored" + assert result[1][1]["httponly"] is True # HttpOnly applies to this cookie + + +def test_parse_cookie_headers_complex_real_world() -> None: + """Test parse_cookie_headers with complex real-world examples.""" + headers = [ + # AWS ELB cookie + "AWSELB=ABCDEF1234567890ABCDEF1234567890ABCDEF1234567890; Path=/", + # Google Analytics + "_ga=GA1.2.1234567890.1234567890; Domain=.example.com; Path=/; Expires=Thu, 31-Dec-2025 23:59:59 GMT", + # Session with all attributes + "session_id=s%3AabcXYZ123.signature123; Path=/; Secure; HttpOnly; SameSite=Strict", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 3 + + # Check each cookie parsed correctly + assert result[0][0] == "AWSELB" + assert result[1][0] == "_ga" + assert result[2][0] == "session_id" + + # Session cookie should have all attributes + session_morsel = result[2][1] + assert session_morsel["secure"] is True + assert session_morsel["httponly"] is True + assert session_morsel.get("samesite") == "Strict" + + +def test_parse_cookie_headers_boolean_attrs() -> None: + """Test that boolean attributes (secure, httponly) work correctly.""" + # Test secure attribute variations + headers = [ + "cookie1=value1; Secure", + "cookie2=value2; Secure=", + "cookie3=value3; Secure=true", # Non-standard but might occur + ] + + result = parse_cookie_headers(headers) + assert len(result) == 3 + + # All should have secure=True + for name, morsel in result: + assert morsel.get("secure") is True, f"{name} should have secure=True" + + # Test httponly attribute variations + headers = [ + "cookie4=value4; HttpOnly", + "cookie5=value5; HttpOnly=", + ] + + result = parse_cookie_headers(headers) + assert len(result) == 2 + + # All should have httponly=True + for name, morsel in result: + assert morsel.get("httponly") is True, f"{name} should have httponly=True" + + +def test_parse_cookie_headers_boolean_attrs_with_partitioned() -> None: + """Test that boolean attributes including partitioned work correctly.""" + # Test secure attribute variations + secure_headers = [ + "cookie1=value1; Secure", + "cookie2=value2; Secure=", + "cookie3=value3; Secure=true", # Non-standard but might occur + ] + + result = parse_cookie_headers(secure_headers) + assert len(result) == 3 + for name, morsel in result: + assert morsel.get("secure") is True, f"{name} should have secure=True" + + # Test httponly attribute variations + httponly_headers = [ + "cookie4=value4; HttpOnly", + "cookie5=value5; HttpOnly=", + ] + + result = parse_cookie_headers(httponly_headers) + assert len(result) == 2 + for name, morsel in result: + assert morsel.get("httponly") is True, f"{name} should have httponly=True" + + # Test partitioned attribute variations + partitioned_headers = [ + "cookie6=value6; Partitioned", + "cookie7=value7; Partitioned=", + "cookie8=value8; Partitioned=yes", # Non-standard but might occur + ] + + result = parse_cookie_headers(partitioned_headers) + assert len(result) == 3 + for name, morsel in result: + assert morsel.get("partitioned") is True, f"{name} should have partitioned=True" + + +def test_parse_cookie_headers_encoded_values() -> None: + """Test that parse_cookie_headers preserves encoded values.""" + headers = [ + "encoded=hello%20world", + "url=https%3A%2F%2Fexample.com%2Fpath", + "special=%21%40%23%24%25%5E%26*%28%29", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 3 + # Values should be preserved as-is (not decoded) + assert result[0][1].value == "hello%20world" + assert result[1][1].value == "https%3A%2F%2Fexample.com%2Fpath" + assert result[2][1].value == "%21%40%23%24%25%5E%26*%28%29" + + +def test_parse_cookie_headers_partitioned() -> None: + """ + Test that parse_cookie_headers handles partitioned attribute correctly. + + This tests the fix for issue #10380 - partitioned cookies support. + The partitioned attribute is a boolean flag like secure and httponly. + """ + headers = [ + "cookie1=value1; Partitioned", + "cookie2=value2; Partitioned=", + "cookie3=value3; Partitioned=true", # Non-standard but might occur + "cookie4=value4; Secure; Partitioned; HttpOnly", + "cookie5=value5; Domain=.example.com; Path=/; Partitioned", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 5 + + # All cookies should have partitioned=True + for i, (name, morsel) in enumerate(result): + assert ( + morsel.get("partitioned") is True + ), f"Cookie {i+1} should have partitioned=True" + assert name == f"cookie{i+1}" + assert morsel.value == f"value{i+1}" + + # Cookie 4 should also have secure and httponly + assert result[3][1].get("secure") is True + assert result[3][1].get("httponly") is True + + # Cookie 5 should also have domain and path + assert result[4][1].get("domain") == ".example.com" + assert result[4][1].get("path") == "/" + + +def test_parse_cookie_headers_partitioned_case_insensitive() -> None: + """Test that partitioned attribute is recognized case-insensitively.""" + headers = [ + "cookie1=value1; partitioned", # lowercase + "cookie2=value2; PARTITIONED", # uppercase + "cookie3=value3; Partitioned", # title case + "cookie4=value4; PaRtItIoNeD", # mixed case + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 4 + + # All should be recognized as partitioned + for i, (_, morsel) in enumerate(result): + assert ( + morsel.get("partitioned") is True + ), f"Cookie {i+1} should have partitioned=True" + + +def test_parse_cookie_headers_partitioned_not_set() -> None: + """Test that cookies without partitioned attribute don't have it set.""" + headers = [ + "normal=value; Secure; HttpOnly", + "regular=cookie; Path=/", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + + # Check that partitioned is not set (empty string is the default for flags in Morsel) + assert result[0][1].get("partitioned", "") == "" + assert result[1][1].get("partitioned", "") == "" + + +# Tests that don't require partitioned support in SimpleCookie +def test_parse_cookie_headers_partitioned_with_other_attrs_manual() -> None: + """ + Test parsing logic for partitioned cookies combined with all other attributes. + + This test verifies our parsing logic handles partitioned correctly as a boolean + attribute regardless of SimpleCookie support. + """ + # Test that our parser recognizes partitioned in _COOKIE_KNOWN_ATTRS and _COOKIE_BOOL_ATTRS + assert "partitioned" in helpers._COOKIE_KNOWN_ATTRS + assert "partitioned" in helpers._COOKIE_BOOL_ATTRS + + # Test a simple case that won't trigger SimpleCookie errors + headers = ["session=abc123; Secure; HttpOnly"] + result = parse_cookie_headers(headers) + + assert len(result) == 1 + assert result[0][0] == "session" + assert result[0][1]["secure"] is True + assert result[0][1]["httponly"] is True + + +def test_cookie_helpers_constants_include_partitioned() -> None: + """Test that cookie helper constants include partitioned attribute.""" + # Test our constants include partitioned + assert "partitioned" in helpers._COOKIE_KNOWN_ATTRS + assert "partitioned" in helpers._COOKIE_BOOL_ATTRS + + +@pytest.mark.parametrize( + "test_string", + [ + " Partitioned ", + " partitioned ", + " PARTITIONED ", + " Partitioned; ", + " Partitioned= ", + " Partitioned=true ", + ], +) +def test_cookie_pattern_matches_partitioned_attribute(test_string: str) -> None: + """Test that the cookie pattern regex matches various partitioned attribute formats.""" + pattern = helpers._COOKIE_PATTERN + match = pattern.match(test_string) + assert match is not None, f"Pattern should match '{test_string}'" + assert match.group("key").lower() == "partitioned" + + +def test_parse_cookie_headers_issue_7993_double_quotes() -> None: + """ + Test that cookies with unmatched opening quotes don't break parsing of subsequent cookies. + + This reproduces issue #7993 where a cookie containing an unmatched opening double quote + causes subsequent cookies to be silently dropped. + NOTE: This only fixes the specific case where a value starts with a quote but doesn't + end with one (e.g., 'cookie="value'). Other malformed quote cases still behave like + SimpleCookie for compatibility. + """ + # Test case from the issue + headers = ['foo=bar; baz="qux; foo2=bar2'] + + result = parse_cookie_headers(headers) + + # Should parse all cookies correctly + assert len(result) == 3 + assert result[0][0] == "foo" + assert result[0][1].value == "bar" + assert result[1][0] == "baz" + assert result[1][1].value == '"qux' # Unmatched quote included + assert result[2][0] == "foo2" + assert result[2][1].value == "bar2" + + +def test_parse_cookie_headers_empty_headers() -> None: + """Test handling of empty headers in the sequence.""" + # Empty header should be skipped + result = parse_cookie_headers(["", "name=value"]) + assert len(result) == 1 + assert result[0][0] == "name" + assert result[0][1].value == "value" + + # Multiple empty headers + result = parse_cookie_headers(["", "", ""]) + assert result == [] + + # Empty headers mixed with valid cookies + result = parse_cookie_headers(["", "a=1", "", "b=2", ""]) + assert len(result) == 2 + assert result[0][0] == "a" + assert result[1][0] == "b" + + +def test_parse_cookie_headers_invalid_cookie_syntax() -> None: + """Test handling of invalid cookie syntax.""" + # No valid cookie pattern + result = parse_cookie_headers(["@#$%^&*()"]) + assert result == [] + + # Cookie name without value + result = parse_cookie_headers(["name"]) + assert result == [] + + # Multiple invalid patterns + result = parse_cookie_headers(["!!!!", "????", "name", "@@@"]) + assert result == [] + + +def test_parse_cookie_headers_illegal_cookie_names( + caplog: pytest.LogCaptureFixture, +) -> None: + """ + Test that illegal cookie names are rejected. + + Note: When a known attribute name is used as a cookie name at the start, + parsing stops early (before any warning can be logged). Warnings are only + logged when illegal names appear after a valid cookie. + """ + # Cookie name that is a known attribute (illegal) - parsing stops early + result = parse_cookie_headers(["path=value; domain=test"]) + assert result == [] + + # Cookie name that doesn't match the pattern + result = parse_cookie_headers(["=value"]) + assert result == [] + + # Valid cookie after illegal one - parsing stops at illegal + result = parse_cookie_headers(["domain=bad; good=value"]) + assert result == [] + + # Illegal cookie name that appears after a valid cookie triggers warning + result = parse_cookie_headers(["good=value; Path=/; invalid,cookie=value;"]) + assert len(result) == 1 + assert result[0][0] == "good" + assert "Illegal cookie name 'invalid,cookie'" in caplog.text + + +def test_parse_cookie_headers_attributes_before_cookie() -> None: + """Test that attributes before any cookie are invalid.""" + # Path attribute before cookie + result = parse_cookie_headers(["Path=/; name=value"]) + assert result == [] + + # Domain attribute before cookie + result = parse_cookie_headers(["Domain=.example.com; name=value"]) + assert result == [] + + # Multiple attributes before cookie + result = parse_cookie_headers(["Path=/; Domain=.example.com; Secure; name=value"]) + assert result == [] + + +def test_parse_cookie_headers_attributes_without_values() -> None: + """Test handling of attributes with missing values.""" + # Boolean attribute without value (valid) + result = parse_cookie_headers(["name=value; Secure"]) + assert len(result) == 1 + assert result[0][1]["secure"] is True + + # Non-boolean attribute without value (invalid, stops parsing) + result = parse_cookie_headers(["name=value; Path"]) + assert len(result) == 1 + # Path without value stops further attribute parsing + + # Multiple cookies, invalid attribute in middle + result = parse_cookie_headers(["name=value; Path; Secure"]) + assert len(result) == 1 + # Secure is not parsed because Path without value stops parsing + + +def test_parse_cookie_headers_dollar_prefixed_names() -> None: + """Test handling of cookie names starting with $.""" + # $Version without preceding cookie (ignored) + result = parse_cookie_headers(["$Version=1; name=value"]) + assert len(result) == 1 + assert result[0][0] == "name" + + # Multiple $ prefixed without cookie (all ignored) + result = parse_cookie_headers(["$Version=1; $Path=/; $Domain=.com; name=value"]) + assert len(result) == 1 + assert result[0][0] == "name" + + # $ prefix at start is ignored, cookie follows + result = parse_cookie_headers(["$Unknown=123; valid=cookie"]) + assert len(result) == 1 + assert result[0][0] == "valid" + + +def test_parse_cookie_headers_dollar_attributes() -> None: + """Test handling of $ prefixed attributes after cookies.""" + # Test multiple $ attributes with cookie (case-insensitive like SimpleCookie) + result = parse_cookie_headers(["name=value; $Path=/test; $Domain=.example.com"]) + assert len(result) == 1 + assert result[0][0] == "name" + assert result[0][1]["path"] == "/test" + assert result[0][1]["domain"] == ".example.com" + + # Test unknown $ attribute (should be ignored) + result = parse_cookie_headers(["name=value; $Unknown=test"]) + assert len(result) == 1 + assert result[0][0] == "name" + # $Unknown should not be set + + # Test $ attribute with empty value + result = parse_cookie_headers(["name=value; $Path="]) + assert len(result) == 1 + assert result[0][1]["path"] == "" + + # Test case sensitivity compatibility with SimpleCookie + result = parse_cookie_headers(["test=value; $path=/lower; $PATH=/upper"]) + assert len(result) == 1 + # Last one wins, and it's case-insensitive + assert result[0][1]["path"] == "/upper" + + +def test_parse_cookie_headers_attributes_after_illegal_cookie() -> None: + """ + Test that attributes after an illegal cookie name are handled correctly. + + This covers the branches where current_morsel is None because an illegal + cookie name was encountered. + """ + # Illegal cookie followed by $ attribute + result = parse_cookie_headers(["good=value; invalid,cookie=bad; $Path=/test"]) + assert len(result) == 1 + assert result[0][0] == "good" + # $Path should be ignored since current_morsel is None after illegal cookie + + # Illegal cookie followed by boolean attribute + result = parse_cookie_headers(["good=value; invalid,cookie=bad; HttpOnly"]) + assert len(result) == 1 + assert result[0][0] == "good" + # HttpOnly should be ignored since current_morsel is None + + # Illegal cookie followed by regular attribute with value + result = parse_cookie_headers(["good=value; invalid,cookie=bad; Max-Age=3600"]) + assert len(result) == 1 + assert result[0][0] == "good" + # Max-Age should be ignored since current_morsel is None + + # Multiple attributes after illegal cookie + result = parse_cookie_headers( + ["good=value; invalid,cookie=bad; $Path=/; HttpOnly; Max-Age=60; Domain=.com"] + ) + assert len(result) == 1 + assert result[0][0] == "good" + # All attributes should be ignored after illegal cookie + + +def test_parse_cookie_headers_unmatched_quotes_compatibility() -> None: + """ + Test that most unmatched quote scenarios behave like SimpleCookie. + + For compatibility, we only handle the specific case of unmatched opening quotes + (e.g., 'cookie="value'). Other cases behave the same as SimpleCookie. + """ + # Cases that SimpleCookie and our parser both fail to parse completely + incompatible_cases = [ + 'cookie1=val"ue; cookie2=value2', # codespell:ignore + 'cookie1=value"; cookie2=value2', + 'cookie1=va"l"ue"; cookie2=value2', # codespell:ignore + 'cookie1=value1; cookie2=val"ue; cookie3=value3', # codespell:ignore + ] + + for header in incompatible_cases: + # Test SimpleCookie behavior + sc = SimpleCookie() + sc.load(header) + sc_cookies = list(sc.items()) + + # Test our parser behavior + result = parse_cookie_headers([header]) + + # Both should parse the same cookies (partial parsing) + assert len(result) == len(sc_cookies), ( + f"Header: {header}\n" + f"SimpleCookie parsed: {len(sc_cookies)} cookies\n" + f"Our parser parsed: {len(result)} cookies" + ) + + # The case we specifically fix (unmatched opening quote) + fixed_case = 'cookie1=value1; cookie2="unmatched; cookie3=value3' + + # SimpleCookie fails to parse cookie3 + sc = SimpleCookie() + sc.load(fixed_case) + assert len(sc) == 1 # Only cookie1 + + # Our parser handles it better + result = parse_cookie_headers([fixed_case]) + assert len(result) == 3 # All three cookies + assert result[0][0] == "cookie1" + assert result[0][1].value == "value1" + assert result[1][0] == "cookie2" + assert result[1][1].value == '"unmatched' + assert result[2][0] == "cookie3" + assert result[2][1].value == "value3" + + +def test_parse_cookie_headers_expires_attribute() -> None: + """Test parse_cookie_headers handles expires attribute with date formats.""" + headers = [ + "session=abc; Expires=Wed, 09 Jun 2021 10:18:14 GMT", + "user=xyz; expires=Wednesday, 09-Jun-21 10:18:14 GMT", + "token=123; EXPIRES=Wed, 09 Jun 2021 10:18:14 GMT", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 3 + for _, morsel in result: + assert "expires" in morsel + assert "GMT" in morsel["expires"] + + +def test_parse_cookie_headers_edge_cases() -> None: + """Test various edge cases.""" + # Very long cookie values + long_value = "x" * 4096 + result = parse_cookie_headers([f"name={long_value}"]) + assert len(result) == 1 + assert result[0][1].value == long_value + + +def test_parse_cookie_headers_various_date_formats_issue_4327() -> None: + """ + Test that parse_cookie_headers handles various date formats per RFC 6265. + + This tests the fix for issue #4327 - support for RFC 822, RFC 850, + and ANSI C asctime() date formats in cookie expiration. + """ + # Test various date formats + headers = [ + # RFC 822 format (preferred format) + "cookie1=value1; Expires=Wed, 09 Jun 2021 10:18:14 GMT", + # RFC 850 format (obsolete but still used) + "cookie2=value2; Expires=Wednesday, 09-Jun-21 10:18:14 GMT", + # RFC 822 with dashes + "cookie3=value3; Expires=Wed, 09-Jun-2021 10:18:14 GMT", + # ANSI C asctime() format (aiohttp extension - not supported by SimpleCookie) + "cookie4=value4; Expires=Wed Jun 9 10:18:14 2021", + # Various other formats seen in the wild + "cookie5=value5; Expires=Thu, 01 Jan 2030 00:00:00 GMT", + "cookie6=value6; Expires=Mon, 31-Dec-99 23:59:59 GMT", + "cookie7=value7; Expires=Tue, 01-Jan-30 00:00:00 GMT", + ] + + result = parse_cookie_headers(headers) + + # All cookies should be parsed + assert len(result) == 7 + + # Check each cookie was parsed with its expires attribute + expected_cookies = [ + ("cookie1", "value1", "Wed, 09 Jun 2021 10:18:14 GMT"), + ("cookie2", "value2", "Wednesday, 09-Jun-21 10:18:14 GMT"), + ("cookie3", "value3", "Wed, 09-Jun-2021 10:18:14 GMT"), + ("cookie4", "value4", "Wed Jun 9 10:18:14 2021"), + ("cookie5", "value5", "Thu, 01 Jan 2030 00:00:00 GMT"), + ("cookie6", "value6", "Mon, 31-Dec-99 23:59:59 GMT"), + ("cookie7", "value7", "Tue, 01-Jan-30 00:00:00 GMT"), + ] + + for (name, morsel), (exp_name, exp_value, exp_expires) in zip( + result, expected_cookies + ): + assert name == exp_name + assert morsel.value == exp_value + assert morsel.get("expires") == exp_expires + + +def test_parse_cookie_headers_ansi_c_asctime_format() -> None: + """ + Test parsing of ANSI C asctime() format. + + This tests support for ANSI C asctime() format (e.g., "Wed Jun 9 10:18:14 2021"). + NOTE: This is an aiohttp extension - SimpleCookie does NOT support this format. + """ + headers = ["cookie1=value1; Expires=Wed Jun 9 10:18:14 2021"] + + result = parse_cookie_headers(headers) + + # Should parse correctly with the expires attribute preserved + assert len(result) == 1 + assert result[0][0] == "cookie1" + assert result[0][1].value == "value1" + assert result[0][1]["expires"] == "Wed Jun 9 10:18:14 2021" + + +def test_parse_cookie_headers_rfc2822_timezone_issue_4493() -> None: + """ + Test that parse_cookie_headers handles RFC 2822 timezone formats. + + This tests the fix for issue #4493 - support for RFC 2822-compliant dates + with timezone offsets like -0000, +0100, etc. + NOTE: This is an aiohttp extension - SimpleCookie does NOT support this format. + """ + headers = [ + # RFC 2822 with -0000 timezone (common in some APIs) + "hello=world; expires=Wed, 15 Jan 2020 09:45:07 -0000", + # RFC 2822 with positive offset + "session=abc123; expires=Thu, 01 Feb 2024 14:30:00 +0100", + # RFC 2822 with negative offset + "token=xyz789; expires=Fri, 02 Mar 2025 08:15:30 -0500", + # Standard GMT for comparison + "classic=cookie; expires=Sat, 03 Apr 2026 12:00:00 GMT", + ] + + result = parse_cookie_headers(headers) + + # All cookies should be parsed + assert len(result) == 4 + + # Check each cookie was parsed with its expires attribute + assert result[0][0] == "hello" + assert result[0][1].value == "world" + assert result[0][1]["expires"] == "Wed, 15 Jan 2020 09:45:07 -0000" + + assert result[1][0] == "session" + assert result[1][1].value == "abc123" + assert result[1][1]["expires"] == "Thu, 01 Feb 2024 14:30:00 +0100" + + assert result[2][0] == "token" + assert result[2][1].value == "xyz789" + assert result[2][1]["expires"] == "Fri, 02 Mar 2025 08:15:30 -0500" + + assert result[3][0] == "classic" + assert result[3][1].value == "cookie" + assert result[3][1]["expires"] == "Sat, 03 Apr 2026 12:00:00 GMT" + + +def test_parse_cookie_headers_rfc2822_with_attributes() -> None: + """Test that RFC 2822 dates work correctly with other cookie attributes.""" + headers = [ + "session=abc123; expires=Wed, 15 Jan 2020 09:45:07 -0000; Path=/; HttpOnly; Secure", + "token=xyz789; expires=Thu, 01 Feb 2024 14:30:00 +0100; Domain=.example.com; SameSite=Strict", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + + # First cookie + assert result[0][0] == "session" + assert result[0][1].value == "abc123" + assert result[0][1]["expires"] == "Wed, 15 Jan 2020 09:45:07 -0000" + assert result[0][1]["path"] == "/" + assert result[0][1]["httponly"] is True + assert result[0][1]["secure"] is True + + # Second cookie + assert result[1][0] == "token" + assert result[1][1].value == "xyz789" + assert result[1][1]["expires"] == "Thu, 01 Feb 2024 14:30:00 +0100" + assert result[1][1]["domain"] == ".example.com" + assert result[1][1]["samesite"] == "Strict" + + +def test_parse_cookie_headers_date_formats_with_attributes() -> None: + """Test that date formats work correctly with other cookie attributes.""" + headers = [ + "session=abc123; Expires=Wed, 09 Jun 2030 10:18:14 GMT; Path=/; HttpOnly; Secure", + "token=xyz789; Expires=Wednesday, 09-Jun-30 10:18:14 GMT; Domain=.example.com; SameSite=Strict", + ] + + result = parse_cookie_headers(headers) + + assert len(result) == 2 + + # First cookie + assert result[0][0] == "session" + assert result[0][1].value == "abc123" + assert result[0][1]["expires"] == "Wed, 09 Jun 2030 10:18:14 GMT" + assert result[0][1]["path"] == "/" + assert result[0][1]["httponly"] is True + assert result[0][1]["secure"] is True + + # Second cookie + assert result[1][0] == "token" + assert result[1][1].value == "xyz789" + assert result[1][1]["expires"] == "Wednesday, 09-Jun-30 10:18:14 GMT" + assert result[1][1]["domain"] == ".example.com" + assert result[1][1]["samesite"] == "Strict" diff --git a/tests/test_cookiejar.py b/tests/test_cookiejar.py index 3d3fed31f13..4f34e957a28 100644 --- a/tests/test_cookiejar.py +++ b/tests/test_cookiejar.py @@ -388,6 +388,8 @@ async def test_domain_filter_ip_cookie_receive( ("custom-cookie=value/one;", 'Cookie: custom-cookie="value/one"', True), ("custom-cookie=value1;", "Cookie: custom-cookie=value1", True), ("custom-cookie=value/one;", "Cookie: custom-cookie=value/one", False), + ('foo="quoted_value"', 'Cookie: foo="quoted_value"', True), + ('foo="quoted_value"; domain=127.0.0.1', 'Cookie: foo="quoted_value"', True), ], ids=( "IP domain preserved", @@ -395,6 +397,8 @@ async def test_domain_filter_ip_cookie_receive( "quoted cookie with special char", "quoted cookie w/o special char", "unquoted cookie with special char", + "pre-quoted cookie", + "pre-quoted cookie with domain", ), ) async def test_quotes_correctly_based_on_input( @@ -1248,7 +1252,7 @@ def test_update_cookies_from_headers_duplicate_names() -> None: url: URL = URL("http://www.example.com/") # Headers with duplicate names but different domains - headers: List[str] = [ + headers = [ "session-id=123456; Domain=.example.com; Path=/", "session-id=789012; Domain=.www.example.com; Path=/", "user-pref=light; Domain=.example.com", @@ -1278,11 +1282,10 @@ def test_update_cookies_from_headers_invalid_cookies( url: URL = URL("http://example.com/") # Mix of valid and invalid cookies - headers: List[str] = [ + headers = [ "valid-cookie=value123", - "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=" - "{925EC0B8-CB17-4BEB-8A35-1033813B0523}; " - "HttpOnly; Path=/", # This cookie with curly braces causes CookieError + "invalid,cookie=value; " # Comma character is not allowed + "HttpOnly; Path=/", "another-valid=value456", ] @@ -1291,7 +1294,7 @@ def test_update_cookies_from_headers_invalid_cookies( jar.update_cookies_from_headers(headers, url) # Check that we logged warnings for invalid cookies - assert "Can not load response cookies" in caplog.text + assert "Can not load cookies" in caplog.text # Valid cookies should still be added assert len(jar) >= 2 # At least the two clearly valid cookies @@ -1300,6 +1303,52 @@ def test_update_cookies_from_headers_invalid_cookies( assert "another-valid" in filtered +def test_update_cookies_from_headers_with_curly_braces() -> None: + """Test that cookies with curly braces in names are now accepted (#2683).""" + jar: CookieJar = CookieJar() + url: URL = URL("http://example.com/") + + # Cookie names with curly braces should now be accepted + headers = [ + "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}=" + "{925EC0B8-CB17-4BEB-8A35-1033813B0523}; " + "HttpOnly; Path=/", + "regular-cookie=value123", + ] + + jar.update_cookies_from_headers(headers, url) + + # Both cookies should be added + assert len(jar) == 2 + filtered: BaseCookie[str] = jar.filter_cookies(url) + assert "ISAWPLB{A7F52349-3531-4DA9-8776-F74BC6F4F1BB}" in filtered + assert "regular-cookie" in filtered + + +def test_update_cookies_from_headers_with_special_chars() -> None: + """Test that cookies with various special characters are accepted.""" + jar: CookieJar = CookieJar() + url: URL = URL("http://example.com/") + + # Various special characters that should now be accepted + headers = [ + "cookie_with_parens=(value)=test123", + "cookie-with-brackets[index]=value456", + "cookie@with@at=value789", + "cookie:with:colons=value000", + ] + + jar.update_cookies_from_headers(headers, url) + + # All cookies should be added + assert len(jar) == 4 + filtered: BaseCookie[str] = jar.filter_cookies(url) + assert "cookie_with_parens" in filtered + assert "cookie-with-brackets[index]" in filtered + assert "cookie@with@at" in filtered + assert "cookie:with:colons" in filtered + + def test_update_cookies_from_headers_empty_list() -> None: """Test that empty header list is handled gracefully.""" jar: CookieJar = CookieJar() @@ -1316,7 +1365,7 @@ def test_update_cookies_from_headers_with_attributes() -> None: jar: CookieJar = CookieJar() url: URL = URL("https://secure.example.com/app/page") - headers: List[str] = [ + headers = [ "secure-cookie=value1; Secure; HttpOnly; SameSite=Strict", "expiring-cookie=value2; Max-Age=3600; Path=/app", "domain-cookie=value3; Domain=.example.com; Path=/", @@ -1371,7 +1420,7 @@ def test_update_cookies_from_headers_preserves_existing() -> None: ) # Add more cookies via headers - headers: List[str] = [ + headers = [ "new-cookie1=value3", "new-cookie2=value4", ] @@ -1396,7 +1445,7 @@ def test_update_cookies_from_headers_overwrites_same_cookie() -> None: jar.update_cookies({"session": "old-value"}, url) # Update with new value via headers - headers: List[str] = ["session=new-value"] + headers = ["session=new-value"] jar.update_cookies_from_headers(headers, url) # Should still have just 1 cookie with updated value @@ -1410,7 +1459,7 @@ def test_dummy_cookie_jar_update_cookies_from_headers() -> None: jar: DummyCookieJar = DummyCookieJar() url: URL = URL("http://example.com/") - headers: List[str] = [ + headers = [ "cookie1=value1", "cookie2=value2", ] @@ -1421,3 +1470,159 @@ def test_dummy_cookie_jar_update_cookies_from_headers() -> None: assert len(jar) == 0 filtered: BaseCookie[str] = jar.filter_cookies(url) assert len(filtered) == 0 + + +async def test_shared_cookie_cache_population() -> None: + """Test that shared cookies are cached correctly.""" + jar = CookieJar(unsafe=True) + + # Create a shared cookie (no domain/path restrictions) + sc = SimpleCookie() + sc["shared"] = "value" + sc["shared"]["path"] = "/" # Will be stripped to "" + + # Update with empty URL to avoid domain being set + jar.update_cookies(sc, URL()) + + # Verify cookie is stored at shared key + assert ("", "") in jar._cookies + assert "shared" in jar._cookies[("", "")] + + # Filter cookies to populate cache + filtered = jar.filter_cookies(URL("http://example.com/")) + assert "shared" in filtered + assert filtered["shared"].value == "value" + + # Verify cache was populated + assert ("", "") in jar._morsel_cache + assert "shared" in jar._morsel_cache[("", "")] + + # Verify the cached morsel is the same one returned + cached_morsel = jar._morsel_cache[("", "")]["shared"] + assert cached_morsel is filtered["shared"] + + +async def test_shared_cookie_cache_clearing_on_update() -> None: + """Test that shared cookie cache is cleared when cookie is updated.""" + jar = CookieJar(unsafe=True) + + # Create initial shared cookie + sc = SimpleCookie() + sc["shared"] = "value1" + sc["shared"]["path"] = "/" + jar.update_cookies(sc, URL()) + + # Filter to populate cache + filtered1 = jar.filter_cookies(URL("http://example.com/")) + assert filtered1["shared"].value == "value1" + assert "shared" in jar._morsel_cache[("", "")] + + # Update the cookie with new value + sc2 = SimpleCookie() + sc2["shared"] = "value2" + sc2["shared"]["path"] = "/" + jar.update_cookies(sc2, URL()) + + # Verify cache was cleared + assert "shared" not in jar._morsel_cache[("", "")] + + # Filter again to verify new value + filtered2 = jar.filter_cookies(URL("http://example.com/")) + assert filtered2["shared"].value == "value2" + + # Verify cache was repopulated with new value + assert "shared" in jar._morsel_cache[("", "")] + + +async def test_shared_cookie_cache_clearing_on_delete() -> None: + """Test that shared cookie cache is cleared when cookies are deleted.""" + jar = CookieJar(unsafe=True) + + # Create multiple shared cookies + sc = SimpleCookie() + sc["shared1"] = "value1" + sc["shared1"]["path"] = "/" + sc["shared2"] = "value2" + sc["shared2"]["path"] = "/" + jar.update_cookies(sc, URL()) + + # Filter to populate cache + jar.filter_cookies(URL("http://example.com/")) + assert "shared1" in jar._morsel_cache[("", "")] + assert "shared2" in jar._morsel_cache[("", "")] + + # Delete one cookie using internal method + jar._delete_cookies([("", "", "shared1")]) + + # Verify cookie and its cache entry were removed + assert "shared1" not in jar._cookies[("", "")] + assert "shared1" not in jar._morsel_cache[("", "")] + + # Verify other cookie remains + assert "shared2" in jar._cookies[("", "")] + assert "shared2" in jar._morsel_cache[("", "")] + + +async def test_shared_cookie_cache_clearing_on_clear() -> None: + """Test that shared cookie cache is cleared when jar is cleared.""" + jar = CookieJar(unsafe=True) + + # Create shared and domain-specific cookies + # Shared cookie + sc1 = SimpleCookie() + sc1["shared"] = "shared_value" + sc1["shared"]["path"] = "/" + jar.update_cookies(sc1, URL()) + + # Domain-specific cookie + sc2 = SimpleCookie() + sc2["domain_cookie"] = "domain_value" + jar.update_cookies(sc2, URL("http://example.com/")) + + # Filter to populate caches + jar.filter_cookies(URL("http://example.com/")) + + # Verify caches are populated + assert ("", "") in jar._morsel_cache + assert "shared" in jar._morsel_cache[("", "")] + assert ("example.com", "") in jar._morsel_cache + assert "domain_cookie" in jar._morsel_cache[("example.com", "")] + + # Clear all cookies + jar.clear() + + # Verify all caches are cleared + assert len(jar._morsel_cache) == 0 + assert len(jar._cookies) == 0 + + # Verify filtering returns no cookies + filtered = jar.filter_cookies(URL("http://example.com/")) + assert len(filtered) == 0 + + +async def test_shared_cookie_with_multiple_domains() -> None: + """Test that shared cookies work across different domains.""" + jar = CookieJar(unsafe=True) + + # Create a truly shared cookie + sc = SimpleCookie() + sc["universal"] = "everywhere" + sc["universal"]["path"] = "/" + jar.update_cookies(sc, URL()) + + # Test filtering for different domains + domains = [ + "http://example.com/", + "http://test.org/", + "http://localhost/", + "http://192.168.1.1/", # IP address (requires unsafe=True) + ] + + for domain_url in domains: + filtered = jar.filter_cookies(URL(domain_url)) + assert "universal" in filtered + assert filtered["universal"].value == "everywhere" + + # Verify cache is reused efficiently + assert ("", "") in jar._morsel_cache + assert "universal" in jar._morsel_cache[("", "")] diff --git a/tests/test_web_request.py b/tests/test_web_request.py index 932e3efd02d..bac910ac0af 100644 --- a/tests/test_web_request.py +++ b/tests/test_web_request.py @@ -279,6 +279,157 @@ def test_request_cookie__set_item() -> None: req.cookies["my"] = "value" # type: ignore[index] +def test_request_cookies_with_special_characters() -> None: + """Test that cookies with special characters in names are accepted. + + This tests the fix for issue #2683 where cookies with special characters + like {, }, / in their names would cause a 500 error. The fix makes the + cookie parser more tolerant to handle real-world cookies. + """ + # Test cookie names with curly braces (e.g., ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}) + headers = CIMultiDict(COOKIE="{test}=value1; normal=value2") + req = make_mocked_request("GET", "/", headers=headers) + # Both cookies should be parsed successfully + assert req.cookies == {"{test}": "value1", "normal": "value2"} + + # Test cookie names with forward slash + headers = CIMultiDict(COOKIE="test/name=value1; valid=value2") + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"test/name": "value1", "valid": "value2"} + + # Test cookie names with various special characters + headers = CIMultiDict( + COOKIE="test{foo}bar=value1; test/path=value2; normal_cookie=value3" + ) + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == { + "test{foo}bar": "value1", + "test/path": "value2", + "normal_cookie": "value3", + } + + +def test_request_cookies_real_world_examples() -> None: + """Test handling of real-world cookie examples from issue #2683.""" + # Example from the issue: ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E} + headers = CIMultiDict( + COOKIE="ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}=val1; normal_cookie=val2" + ) + req = make_mocked_request("GET", "/", headers=headers) + # All cookies should be parsed successfully + assert req.cookies == { + "ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}": "val1", + "normal_cookie": "val2", + } + + # Multiple cookies with special characters + headers = CIMultiDict( + COOKIE="{cookie1}=val1; cookie/2=val2; cookie[3]=val3; cookie(4)=val4" + ) + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == { + "{cookie1}": "val1", + "cookie/2": "val2", + "cookie[3]": "val3", + "cookie(4)": "val4", + } + + +def test_request_cookies_edge_cases() -> None: + """Test edge cases for cookie parsing.""" + # Empty cookie value + headers = CIMultiDict(COOKIE="test=; normal=value") + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"test": "", "normal": "value"} + + # Cookie with quoted value + headers = CIMultiDict(COOKIE='test="quoted value"; normal=unquoted') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"test": "quoted value", "normal": "unquoted"} + + +def test_request_cookies_no_500_error() -> None: + """Test that cookies with special characters don't cause 500 errors. + + This specifically tests that issue #2683 is fixed - previously cookies + with characters like { } would cause CookieError and 500 responses. + """ + # This cookie format previously caused 500 errors + headers = CIMultiDict(COOKIE="ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}=test") + + # Should not raise any exception when accessing cookies + req = make_mocked_request("GET", "/", headers=headers) + cookies = req.cookies # This used to raise CookieError + + # Verify the cookie was parsed successfully + assert "ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}" in cookies + assert cookies["ISAWPLB{DB45DF86-F806-407C-932C-D52A60E4019E}"] == "test" + + +def test_request_cookies_quoted_values() -> None: + """Test that quoted cookie values are handled consistently. + + This tests the fix for issue #5397 where quoted cookie values were + handled inconsistently based on whether domain attributes were present. + The new parser should always unquote cookie values consistently. + """ + # Test simple quoted cookie value + headers = CIMultiDict(COOKIE='sess="quoted_value"') + req = make_mocked_request("GET", "/", headers=headers) + # Quotes should be removed consistently + assert req.cookies == {"sess": "quoted_value"} + + # Test quoted cookie with semicolon in value + headers = CIMultiDict(COOKIE='data="value;with;semicolons"') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"data": "value;with;semicolons"} + + # Test mixed quoted and unquoted cookies + headers = CIMultiDict( + COOKIE='quoted="value1"; unquoted=value2; also_quoted="value3"' + ) + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == { + "quoted": "value1", + "unquoted": "value2", + "also_quoted": "value3", + } + + # Test escaped quotes in cookie value + headers = CIMultiDict(COOKIE=r'escaped="value with \" quote"') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"escaped": 'value with " quote'} + + # Test empty quoted value + headers = CIMultiDict(COOKIE='empty=""') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"empty": ""} + + +def test_request_cookies_with_attributes() -> None: + """Test that cookie attributes don't affect value parsing. + + Related to issue #5397 - ensures that the presence of domain or other + attributes doesn't change how cookie values are parsed. + """ + # Cookie with domain attribute - quotes should still be removed + headers = CIMultiDict(COOKIE='sess="quoted_value"; Domain=.example.com') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"sess": "quoted_value"} + + # Cookie with multiple attributes + headers = CIMultiDict(COOKIE='token="abc123"; Path=/; Secure; HttpOnly') + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"token": "abc123"} + + # Multiple cookies with different attributes + headers = CIMultiDict( + COOKIE='c1="v1"; Domain=.example.com; c2="v2"; Path=/api; c3=v3; Secure' + ) + req = make_mocked_request("GET", "/", headers=headers) + assert req.cookies == {"c1": "v1", "c2": "v2", "c3": "v3"} + + def test_match_info() -> None: req = make_mocked_request("GET", "/") assert req._match_info is req.match_info