[pull] master from aio-libs:master

aiohttp/_cookie_helpers.py

@@ -52,7 +52,7 @@
     \s*                            # Optional whitespace at start of cookie
     (?P<key>                       # Start of group 'key'
     # aiohttp has extended to include [] for compatibility with real-world cookies
-    [\w\d!#%&'~_`><@,:/\$\*\+\-\.\^\|\)\(\?\}\{\=\[\]]+?   # Any word of at least one letter
+    [\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

aiohttp/_http_parser.pyx

@@ -419,7 +419,8 @@ cdef class HttpParser:
         headers = CIMultiDictProxy(CIMultiDict(self._headers))
 
         if self._cparser.type == cparser.HTTP_REQUEST:
-            allowed = upgrade and headers.get("upgrade", "").lower() in ALLOWED_UPGRADES
+            h_upg = headers.get("upgrade", "")
+            allowed = upgrade and h_upg.isascii() and h_upg.lower() in ALLOWED_UPGRADES
             if allowed or self._cparser.method == cparser.HTTP_CONNECT:
                 self._upgraded = True
         else:
@@ -434,8 +435,7 @@ cdef class HttpParser:
         enc = self._content_encoding
         if enc is not None:
             self._content_encoding = None
-            enc = enc.lower()
-            if enc in ('gzip', 'deflate', 'br', 'zstd'):
+            if enc.isascii() and enc.lower() in {"gzip", "deflate", "br", "zstd"}:
                 encoding = enc
 
         if self._cparser.type == cparser.HTTP_REQUEST:

aiohttp/client_middleware_digest_auth.py

@@ -10,6 +10,7 @@
 import hashlib
 import os
 import re
+import sys
 import time
 from collections.abc import Callable
 from typing import Final, Literal, TypedDict
@@ -51,24 +52,27 @@ class DigestAuthChallenge(TypedDict, total=False):
 
 # Compile the regex pattern once at module level for performance
 _HEADER_PAIRS_PATTERN = re.compile(
-    r'(\w+)\s*=\s*(?:"((?:[^"\\]|\\.)*)"|([^\s,]+))'
-    # |    |  | | |  |    |      |    |  ||     |
-    # +----|--|-|-|--|----|------|----|--||-----|--> alphanumeric key
-    #      +--|-|-|--|----|------|----|--||-----|--> maybe whitespace
-    #         | | |  |    |      |    |  ||     |
-    #         +-|-|--|----|------|----|--||-----|--> = (delimiter)
-    #           +-|--|----|------|----|--||-----|--> maybe whitespace
-    #             |  |    |      |    |  ||     |
-    #             +--|----|------|----|--||-----|--> group quoted or unquoted
-    #                |    |      |    |  ||     |
-    #                +----|------|----|--||-----|--> if quoted...
-    #                     +------|----|--||-----|--> anything but " or \
-    #                            +----|--||-----|--> escaped characters allowed
-    #                                 +--||-----|--> or can be empty string
-    #                                    ||     |
-    #                                    +|-----|--> if unquoted...
-    #                                     +-----|--> anything but , or <space>
-    #                                           +--> at least one char req'd
+    r'(?:^|\s|,\s*)(\w+)\s*=\s*(?:"((?:[^"\\]|\\.)*)"|([^\s,]+))'
+    if sys.version_info < (3, 11)
+    else r'(?:^|\s|,\s*)((?>\w+))\s*=\s*(?:"((?:[^"\\]|\\.)*)"|([^\s,]+))'
+    # +------------|--------|--|-|-|--|----|------|----|--||-----|-> Match valid start/sep
+    #              +--------|--|-|-|--|----|------|----|--||-----|-> alphanumeric key (atomic
+    #                       |  | | |  |    |      |    |  ||     |   group reduces backtracking)
+    #                       +--|-|-|--|----|------|----|--||-----|-> maybe whitespace
+    #                          | | |  |    |      |    |  ||     |
+    #                          +-|-|--|----|------|----|--||-----|-> = (delimiter)
+    #                            +-|--|----|------|----|--||-----|-> maybe whitespace
+    #                              |  |    |      |    |  ||     |
+    #                              +--|----|------|----|--||-----|-> group quoted or unquoted
+    #                                 |    |      |    |  ||     |
+    #                                 +----|------|----|--||-----|-> if quoted...
+    #                                      +------|----|--||-----|-> anything but " or \
+    #                                             +----|--||-----|-> escaped characters allowed
+    #                                                  +--||-----|-> or can be empty string
+    #                                                     ||     |
+    #                                                     +|-----|-> if unquoted...
+    #                                                      +-----|-> anything but , or <space>
+    #                                                            +-> at least one char req'd
 )
 
 

aiohttp/http_parser.py

@@ -217,7 +217,9 @@ def parse_headers(
 
 def _is_supported_upgrade(headers: CIMultiDictProxy[str]) -> bool:
     """Check if the upgrade header is supported."""
-    return headers.get(hdrs.UPGRADE, "").lower() in {"tcp", "websocket"}
+    u = headers.get(hdrs.UPGRADE, "")
+    # .lower() can transform non-ascii characters.
+    return u.isascii() and u.lower() in {"tcp", "websocket"}
 
 
 class HttpParser(abc.ABC, Generic[_MsgT]):
@@ -522,11 +524,9 @@ def parse_headers(
                 upgrade = True
 
         # encoding
-        enc = headers.get(hdrs.CONTENT_ENCODING)
-        if enc:
-            enc = enc.lower()
-            if enc in ("gzip", "deflate", "br", "zstd"):
-                encoding = enc
+        enc = headers.get(hdrs.CONTENT_ENCODING, "")
+        if enc.isascii() and enc.lower() in {"gzip", "deflate", "br", "zstd"}:
+            encoding = enc
 
         # chunking
         te = headers.get(hdrs.TRANSFER_ENCODING)
@@ -643,7 +643,9 @@ def parse_message(self, lines: list[bytes]) -> RawRequestMessage:
         )
 
     def _is_chunked_te(self, te: str) -> bool:
-        if te.rsplit(",", maxsplit=1)[-1].strip(" \t").lower() == "chunked":
+        te = te.rsplit(",", maxsplit=1)[-1].strip(" \t")
+        # .lower() transforms some non-ascii chars, so must check first.
+        if te.isascii() and te.lower() == "chunked":
             return True
         # https://www.rfc-editor.org/rfc/rfc9112#section-6.3-2.4.3
         raise BadHttpMessage("Request has invalid `Transfer-Encoding`")

aiohttp/web_request.py

@@ -93,6 +93,7 @@ class FileField:
 
 _QUOTED_STRING: Final[str] = rf'"(?:{_QUOTED_PAIR}|{_QDTEXT})*"'
 
+# This does not have a ReDOS/performance concern as long as it used with re.match().
 _FORWARDED_PAIR: Final[str] = rf"({_TOKEN})=({_TOKEN}|{_QUOTED_STRING})(:\d{{1,4}})?"
 
 _QUOTED_PAIR_REPLACE_RE: Final[Pattern[str]] = re.compile(r"\\([\t !-~])")

tests/test_client_middleware_digest_auth.py

@@ -2,6 +2,7 @@
 
 import io
 import re
+import time
 from collections.abc import Generator
 from hashlib import md5, sha1
 from typing import Literal
@@ -13,6 +14,7 @@
 from aiohttp import ClientSession, hdrs
 from aiohttp.client_exceptions import ClientError
 from aiohttp.client_middleware_digest_auth import (
+    _HEADER_PAIRS_PATTERN,
     DigestAuthChallenge,
     DigestAuthMiddleware,
     DigestFunctions,
@@ -1327,3 +1329,15 @@ async def handler(request: Request) -> Response:
     assert request_count == 2  # Initial 401 + successful retry
     assert len(auth_algorithms) == 1
     assert auth_algorithms[0] == "MD5-sess"  # Not "MD5-SESS"
+
+
+def test_regex_performance() -> None:
+    value = "0" * 54773 + "\\0=a"
+    start = time.perf_counter()
+    matches = _HEADER_PAIRS_PATTERN.findall(value)
+    end = time.perf_counter()
+
+    # If this is taking more than 10ms, there's probably a performance/ReDoS issue.
+    assert (end - start) < 0.01
+    # This example probably shouldn't produce a match either.
+    assert not matches

tests/test_cookie_helpers.py

@@ -1,6 +1,7 @@
 """Tests for internal cookie helper functions."""
 
 import sys
+import time
 from http.cookies import (
     CookieError,
     Morsel,
@@ -635,6 +636,18 @@ def test_cookie_pattern_matches_partitioned_attribute(test_string: str) -> None:
     assert match.group("key").lower() == "partitioned"
 
 
+def test_cookie_pattern_performance() -> None:
+    value = "a" + "=" * 21651 + "\x00"
+    start = time.perf_counter()
+    match = helpers._COOKIE_PATTERN.match(value)
+    end = time.perf_counter()
+
+    # If this is taking more than 10ms, there's probably a performance/ReDoS issue.
+    assert (end - start) < 0.01
+    # This example shouldn't produce a match either.
+    assert match is None
+
+
 def test_parse_set_cookie_headers_issue_7993_double_quotes() -> None:
     """
     Test that cookies with unmatched opening quotes don't break parsing of subsequent cookies.
@@ -1299,11 +1312,9 @@ def test_parse_cookie_header_malformed() -> None:
     # Missing name
     header = "=value; name=value2"
     result = parse_cookie_header(header)
-    assert len(result) == 2
-    assert result[0][0] == "=value"
-    assert result[0][1].value == ""
-    assert result[1][0] == "name"
-    assert result[1][1].value == "value2"
+    assert len(result) == 1
+    assert result[0][0] == "name"
+    assert result[0][1].value == "value2"
 
 
 def test_parse_cookie_header_complex_quoted() -> None:

tests/test_http_parser.py

@@ -491,6 +491,20 @@ def test_request_chunked(parser: HttpRequestParser) -> None:
     assert isinstance(payload, streams.StreamReader)
 
 
+def test_te_header_non_ascii(parser: HttpRequestParser) -> None:
+    # K = Kelvin sign, not valid ascii.
+    text = "GET /test HTTP/1.1\r\nTransfer-Encoding: chunKed\r\n\r\n"
+    with pytest.raises(http_exceptions.BadHttpMessage):
+        parser.feed_data(text.encode())
+
+
+def test_upgrade_header_non_ascii(parser: HttpRequestParser) -> None:
+    # K = Kelvin sign, not valid ascii.
+    text = "GET /test HTTP/1.1\r\nUpgrade: websocKet\r\n\r\n"
+    messages, upgrade, tail = parser.feed_data(text.encode())
+    assert not upgrade
+
+
 def test_request_te_chunked_with_content_length(parser: HttpRequestParser) -> None:
     text = (
         b"GET /test HTTP/1.1\r\n"
@@ -589,6 +603,21 @@ def test_compression_zstd(parser: HttpRequestParser) -> None:
     assert msg.compression == "zstd"
 
 
+@pytest.mark.parametrize(
+    "enc",
+    (
+        "zstd".encode(),  # "st".upper() == "ST"
+        "deflate".encode(),  # "fl".upper() == "FL"
+    ),
+)
+def test_compression_non_ascii(parser: HttpRequestParser, enc: bytes) -> None:
+    text = b"GET /test HTTP/1.1\r\ncontent-encoding: " + enc + b"\r\n\r\n"
+    messages, upgrade, tail = parser.feed_data(text)
+    msg = messages[0][0]
+    # Non-ascii input should not evaluate to a valid encoding scheme.
+    assert msg.compression is None
+
+
 def test_compression_unknown(parser: HttpRequestParser) -> None:
     text = b"GET /test HTTP/1.1\r\ncontent-encoding: compress\r\n\r\n"
     messages, upgrade, tail = parser.feed_data(text)

tests/test_web_request.py

@@ -3,6 +3,7 @@
 import socket
 import ssl
 import sys
+import time
 import weakref
 from collections.abc import Iterator, MutableMapping
 from typing import NoReturn
@@ -18,6 +19,7 @@
 from aiohttp.pytest_plugin import AiohttpClient
 from aiohttp.streams import StreamReader
 from aiohttp.test_utils import make_mocked_request
+from aiohttp.web_request import _FORWARDED_PAIR_RE
 
 
 @pytest.fixture
@@ -574,6 +576,18 @@ def test_single_forwarded_header() -> None:
     assert req.forwarded[0]["proto"] == "identifier"
 
 
+def test_forwarded_re_performance() -> None:
+    value = "{" + "f" * 54773 + "z\x00a=v"
+    start = time.perf_counter()
+    match = _FORWARDED_PAIR_RE.match(value)
+    end = time.perf_counter()
+
+    # If this is taking more than 10ms, there's probably a performance/ReDoS issue.
+    assert (end - start) < 0.01
+    # This example shouldn't produce a match either.
+    assert match is None
+
+
 @pytest.mark.parametrize(
     "forward_for_in, forward_for_out",
     [

tests/test_web_urldispatcher.py

@@ -49,6 +49,26 @@
             b"my_file_in_dir</a></li>\n</ul>\n</body>\n</html>",
             id="index_subdir",
         ),
+        pytest.param(
+            True,
+            200,
+            "/static",
+            "/static/",
+            b"<html>\n<head>\n<title>Index of /.</title>\n</head>\n<body>\n<h1>Index of"
+            b' /.</h1>\n<ul>\n<li><a href="/static/my_dir">my_dir/</a></li>\n<li><a href="'
+            b'/static/my_file">my_file</a></li>\n</ul>\n</body>\n</html>',
+            id="index_static_trailing_slash",
+        ),
+        pytest.param(
+            True,
+            200,
+            "/static",
+            "/static/my_dir/",
+            b"<html>\n<head>\n<title>Index of /my_dir</title>\n</head>\n<body>\n<h1>"
+            b'Index of /my_dir</h1>\n<ul>\n<li><a href="/static/my_dir/my_file_in_dir">'
+            b"my_file_in_dir</a></li>\n</ul>\n</body>\n</html>",
+            id="index_subdir_trailing_slash",
+        ),
     ],
 )
 async def test_access_root_of_static_handler(