[pull] master from aio-libs:master

CHANGES/13855.bugfix.rst

@@ -0,0 +1,5 @@
+Resolved a redirect ``Location`` with the scheme of the current URL but
+without ``//``, such as ``http:/path`` or ``http:path``, against the
+current URL, as browsers do, instead of treating it as an absolute URL
+without a host
+-- by :user:`asvetlov`.

CHANGES/13858.bugfix.rst

@@ -0,0 +1,7 @@
+Rejected absolute-form request targets without ``//`` or with an empty
+host, such as ``http:/example.com/`` or ``http:///example.com/``, before
+parsing them with yarl, which reads a host from them in its WHATWG mode;
+RFC 9110 requires a host for ``http`` and ``https``. The invalid URL test
+data no longer uses ``http:///example.com``, which such a yarl version
+parses as ``http://example.com/``, as browsers do
+-- by :user:`asvetlov`.

CHANGES/13861.contrib.rst

@@ -0,0 +1,4 @@
+Changed the long host in the ``Host`` header tests to one that is not made
+only of digits, since yarl now parses such a host as an IP address in its
+default mode and rejects this one as out of range
+-- by :user:`asvetlov`.

THREAT_MODEL.md

@@ -268,7 +268,7 @@ into `StreamReader`) is then handed to `web_protocol.RequestHandler` and
 | 1.9 | Chunk-size DoS | The parser doesn't cap chunk size, but **server-side body length is bounded by `client_max_size` (default `1 MiB`)** in `web_request.py:BaseRequest.read`. Client-side responses are bounded by user-supplied `max_body_size` / streaming reads. | None. If a cap is ever needed at the parser level, plumb it through `HttpPayloadParser`. |
 | 1.10 | Chunk-extension DoS | Chunk-extension content is bounded by the same wire-level size constraints (it shares the chunk-size line with `max_line_size`). | **Add an explicit test that chunk-extension flooding cannot blow past `max_line_size`.** |
 | 1.11 | Parser error reflection | `http_parser.py` truncates to `[:100]` only for `LineTooLong`; `BadStatusLine` / `InvalidHeader` / `TransferEncodingError` carry the offending line up to `max_line_size` / `max_field_size`. `_http_parser.pyx` bounds its snippet to 50 bytes either side of the error position, so input with no CRLF to delimit the offending line is not quoted in full (`test_c_parser_error_message_bounded_for_crlf_free_input`). | **Audit any aiohttp path where `BadHttpMessage` content is reflected to the client unsanitised.** **User**: Review custom `web_log` configurations and any middleware that reflects parser exception messages back to the peer. |
-| 1.12 | Cython ⇄ pure-Python divergence | `tests/test_http_parser.py` parameterises tests over `REQUEST_PARSERS` / `RESPONSE_PARSERS` (pure-Python always; Cython when the extension imports). The high-leverage attack vectors are already covered under both backends: CL+TE (`test_content_length_transfer_encoding`), CL×N (`test_duplicate_singleton_header_rejected`), obs-fold (`test_reject_obsolete_line_folding`, `test_http_response_parser_obs_line_folding*`), CR/LF/NUL (`test_bad_headers`, `test_http_response_parser_null_byte_in_header_value`, `test_http_response_parser_bad_crlf`), version regex (`test_http_request_parser_bad_version*`, `test_http_response_parser_bad_version*`), bare-LF line endings (`test_reject_bare_lf_no_cross_request_leak`), control characters in the request target (`test_http_request_parser_ctl_in_request_target`). | None. When new attack vectors emerge, add them to the parameterised tests. |
+| 1.12 | Cython ⇄ pure-Python divergence | `tests/test_http_parser.py` parameterises tests over `REQUEST_PARSERS` / `RESPONSE_PARSERS` (pure-Python always; Cython when the extension imports). The high-leverage attack vectors are already covered under both backends: CL+TE (`test_content_length_transfer_encoding`), CL×N (`test_duplicate_singleton_header_rejected`), obs-fold (`test_reject_obsolete_line_folding`, `test_http_response_parser_obs_line_folding*`), CR/LF/NUL (`test_bad_headers`, `test_http_response_parser_null_byte_in_header_value`, `test_http_response_parser_bad_crlf`), version regex (`test_http_request_parser_bad_version*`, `test_http_response_parser_bad_version*`), bare-LF line endings (`test_reject_bare_lf_no_cross_request_leak`), control characters in the request target (`test_http_request_parser_ctl_in_request_target`), absolute-form targets without `//` or with an empty host (`test_url_absolute_form_empty_host_rejected`). | None. When new attack vectors emerge, add them to the parameterised tests. |
 | 1.13 | llhttp version drift | Manual upgrade via `make generate-llhttp`; vendor pinned in `vendor/llhttp/package.json`. | Track upstream releases (e.g. via Dependabot rule for `vendor/llhttp/package.json`), bump on every llhttp release, regenerate in CI. |
 | 1.14 | npm-side compromise of `llhttp` | The vendored output is checked into git, so a compromise during a future regen would be detectable in PR review. See [§5.19](#519-build--release-supply-chain). | **Make the llhttp build reproducible: pin Node.js version, commit the npm lockfile, and on every bump verify the regenerated C against upstream's release tarballs before committing.** |
 

aiohttp/_http_parser.pyx

@@ -31,7 +31,7 @@ from .http_exceptions import (
     PayloadEncodingError,
     TransferEncodingError,
 )
-from .http_parser import DeflateBuffer as _DeflateBuffer
+from .http_parser import DeflateBuffer as _DeflateBuffer, _has_authority
 from .http_writer import (
     HttpVersion as _HttpVersion,
     HttpVersion10 as _HttpVersion10,
@@ -793,11 +793,13 @@ cdef class HttpRequestParser(HttpParser):
             else:
                 # absolute-form for proxy maybe,
                 # https://datatracker.ietf.org/doc/html/rfc7230#section-5.3.2
-                try:
-                    self._url = URL(self._path, encoded=True)
-                    host = self._url.raw_host
-                except ValueError:
-                    host = None
+                host = None
+                if _has_authority(self._path):
+                    try:
+                        self._url = URL(self._path, encoded=True)
+                        host = self._url.raw_host
+                    except ValueError:
+                        pass
                 # https://www.rfc-editor.org/rfc/rfc9110#section-4.2.1-4
                 if host is None:
                     raise InvalidURLError(

aiohttp/client.py

@@ -168,6 +168,22 @@
     from typing import Unpack
 
 
+# URL parsers strip leading and trailing C0 control characters and spaces and
+# drop tabs and newlines before splitting a URL.
+_C0_CONTROL_OR_SPACE = "".join(map(chr, range(0x21)))
+_REMOVE_TAB_OR_NEWLINE = str.maketrans("", "", "\t\n\r")
+
+
+def _has_no_authority(location: str, scheme: str) -> bool:
+    """Tell if a URL with a scheme has no "//" after the scheme.
+
+    Browsers read a backslash like a slash there for http and https.
+    """
+    location = location.strip(_C0_CONTROL_OR_SPACE).translate(_REMOVE_TAB_OR_NEWLINE)
+    rest = location[len(scheme) + 1 : len(scheme) + 3]
+    return rest[:1] not in ("/", "\\") or rest[1:] not in ("/", "\\")
+
+
 class _RequestOptions(TypedDict, total=False):
     params: Query
     data: Any
@@ -834,7 +850,11 @@ async def _request(
                                 await req._body.close()
                             resp.close()
                             raise NonHttpUrlRedirectClientError(r_url)
-                        elif not scheme:
+                        elif not scheme or (
+                            scheme == url.scheme and _has_no_authority(r_url, scheme)
+                        ):
+                            # "http:/path" or "http:path" is a reference to
+                            # the current URL, as browsers resolve it.
                             parsed_redirect_url = url.join(parsed_redirect_url)
 
                         try:

aiohttp/http_parser.py

@@ -91,6 +91,18 @@
 DIGITS: Final[Pattern[str]] = re.compile(r"\d+", re.ASCII)
 HEXDIGITS: Final[Pattern[bytes]] = re.compile(rb"[0-9a-fA-F]+")
 
+
+def _has_authority(target: str) -> bool:
+    """Tell if an absolute-form request target has "//" and a non-empty authority.
+
+    RFC 9110 section 4.2 requires a host for http and https. yarl's default
+    WHATWG mode reads a host from "http:host/p" or "http:///host/p", so the
+    target is checked before it is parsed.
+    """
+    rest = target.partition(":")[2]
+    return rest[:2] == "//" and rest[2:3] not in ("", "/", "?", "#")
+
+
 # RFC 9110 singleton headers — duplicates are rejected in strict mode.
 # In lax mode (response parser default), the check is skipped entirely
 # since real-world servers (e.g. Google APIs, Werkzeug) commonly send
@@ -719,6 +731,10 @@ def parse_message(self, lines: list[bytes]) -> RawRequestMessage:
         else:
             # absolute-form for proxy maybe,
             # https://datatracker.ietf.org/doc/html/rfc7230#section-5.3.2
+            if not _has_authority(path):
+                raise InvalidURLError(
+                    path.encode(errors="surrogateescape").decode("latin1")
+                )
             try:
                 url = URL(path, encoded=True)
                 host = url.raw_host

tests/test_client_functional.py

@@ -1006,6 +1006,38 @@ async def handler_ok(request: web.Request) -> web.Response:
         assert resp.url.path == "/ok"
 
 
+@pytest.mark.parametrize(
+    ("location", "path", "query"),
+    (
+        ("http:/ok", "/ok", {}),
+        ("http:ok", "/ok", {}),
+        ("http:/ok?a=b", "/ok", {"a": "b"}),
+        ("http:/", "/", {}),
+    ),
+)
+async def test_redirect_same_scheme_without_authority(
+    aiohttp_client: AiohttpClient, location: str, path: str, query: dict[str, str]
+) -> None:
+    async def handler_redirect(request: web.Request) -> web.Response:
+        return web.Response(status=301, headers={"Location": location})
+
+    async def handler_ok(request: web.Request) -> web.Response:
+        assert dict(request.query) == query
+        return web.Response(status=200)
+
+    app = web.Application()
+    app.router.add_route("GET", path, handler_ok)
+    app.router.add_route("GET", "/redirect", handler_redirect)
+    client = await aiohttp_client(app)
+
+    async with client.get("/redirect") as resp:
+        assert resp.status == 200
+        assert resp.url.host == "127.0.0.1"
+        assert resp.url.path == path
+        assert dict(resp.url.query) == query
+        assert len(resp.history) == 1
+
+
 async def test_history(aiohttp_client: AiohttpClient) -> None:
     async def handler_redirect(request: web.Request) -> web.Response:
         return web.Response(status=301, headers={"Location": "/ok"})
@@ -3210,12 +3242,9 @@ async def handler_redirect(request: web.Request) -> web.Response:
     ("http://example.org:non_int_port/", "http://example.org:non_int_port/"),
 )
 
-INVALID_URL_WITH_ERROR_MESSAGE_YARL_ORIGIN = (
-    # # yarl.URL.origin raises ValueError
-    ("http:/", "http:///"),
-    ("http:/example.com", "http:///example.com"),
-    ("http:///example.com", "http:///example.com"),
-)
+# yarl.URL.origin raises ValueError. A redirect to "http:/" resolves against
+# the current URL instead, see test_redirect_same_scheme_without_authority().
+INVALID_URL_WITH_ERROR_MESSAGE_YARL_ORIGIN = (("http:/", "http:///"),)
 
 NON_HTTP_URL_WITH_ERROR_MESSAGE = (
     ("call:+380123456789", r"call:\+380123456789"),
@@ -3259,8 +3288,7 @@ async def test_invalid_and_non_http_url(
     (
         *(
             (url, message, InvalidUrlRedirectClientError)
-            for (url, message) in INVALID_URL_WITH_ERROR_MESSAGE_YARL_ORIGIN
-            + INVALID_URL_WITH_ERROR_MESSAGE_YARL_NEW
+            for (url, message) in INVALID_URL_WITH_ERROR_MESSAGE_YARL_NEW
         ),
         *(
             (url, message, NonHttpUrlRedirectClientError)
@@ -3294,8 +3322,7 @@ async def generate_redirecting_response(request: web.Request) -> web.Response:
     (
         *(
             (url, message, InvalidUrlRedirectClientError)
-            for (url, message) in INVALID_URL_WITH_ERROR_MESSAGE_YARL_ORIGIN
-            + INVALID_URL_WITH_ERROR_MESSAGE_YARL_NEW
+            for (url, message) in INVALID_URL_WITH_ERROR_MESSAGE_YARL_NEW
         ),
         *(
             (url, message, NonHttpUrlRedirectClientError)
@@ -5558,8 +5585,8 @@ async def test_invalid_redirect_origin_closes_payload(
     async def redirect_handler(request: web.Request) -> web.Response:
         # Read the payload to simulate server processing
         await request.read()
-        # Return a URL that will fail origin() check - using a relative URL without host
-        return web.Response(status=307, headers={hdrs.LOCATION: "http:///path"})
+        # Return a URL that will fail origin() check - using a URL without host
+        return web.Response(status=307, headers={hdrs.LOCATION: "http://"})
 
     app = web.Application()
     app.router.add_post("/redirect", redirect_handler)
@@ -5632,7 +5659,7 @@ async def stall(reader: asyncio.StreamReader, writer: asyncio.StreamWriter) -> N
 async def test_request_error_before_body_created_does_not_mask() -> None:
     async with aiohttp.ClientSession() as session:
         with pytest.raises(InvalidUrlClientError):
-            await session.get("http:///path")
+            await session.get("http://")
 
 
 async def test_amazon_like_cookie_scenario(aiohttp_client: AiohttpClient) -> None:

tests/test_client_request.py

@@ -631,29 +631,19 @@ async def test_params_empty_path_and_url(make_client_request: _RequestMaker) ->
     assert str(req_none.url) == "http://python.org"
 
 
+# A long host that is not all digits: WHATWG parses a host made only of
+# numbers, like this one without ".test", as an IPv4 address.
+LONG_HOST = "12345678901234567890123456789012345678901234567890.test"
+
+
 async def test_gen_netloc_all(make_client_request: _RequestMaker) -> None:
-    req = make_client_request(
-        "get",
-        URL(
-            "https://aiohttp:pwpwpw@12345678901234567890123456789012345678901234567890:8080"
-        ),
-    )
-    assert (
-        req.headers["HOST"]
-        == "12345678901234567890123456789" + "012345678901234567890:8080"
-    )
+    req = make_client_request("get", URL(f"https://aiohttp:pwpwpw@{LONG_HOST}:8080"))
+    assert req.headers["HOST"] == f"{LONG_HOST}:8080"
 
 
 async def test_gen_netloc_no_port(make_client_request: _RequestMaker) -> None:
-    req = make_client_request(
-        "get",
-        URL(
-            "https://aiohttp:pwpwpw@12345678901234567890123456789012345678901234567890/"
-        ),
-    )
-    assert (
-        req.headers["HOST"] == "12345678901234567890123456789" + "012345678901234567890"
-    )
+    req = make_client_request("get", URL(f"https://aiohttp:pwpwpw@{LONG_HOST}/"))
+    assert req.headers["HOST"] == LONG_HOST
 
 
 async def test_cookie_coded_value_preserved(make_client_request: _RequestMaker) -> None:

tests/test_client_session.py

@@ -1729,3 +1729,25 @@ async def test_netrc_auth_host_not_in_netrc(auth_server: TestServer) -> None:
         text = await resp.text()
         # Should not have auth since the host is not in netrc
         assert text == "no_auth"
+
+
+@pytest.mark.parametrize(
+    ("location", "expected"),
+    (
+        ("http:/ok", True),
+        ("http:ok", True),
+        ("http:", True),
+        ("http:/", True),
+        ("http://example.com/", False),
+        ("http:///example.com", False),
+        ("  http:///example.com", False),
+        ("\x00http:///example.com", False),
+        ("http:/\t/example.com", False),
+        ("http:/\n/example.com", False),
+        ("http:\\\\example.com", False),
+        ("http:\\/example.com", False),
+        ("http:/\\example.com", False),
+    ),
+)
+def test_has_no_authority(location: str, expected: bool) -> None:
+    assert client._has_no_authority(location, "http") is expected

tests/test_http_parser.py

@@ -1188,8 +1188,23 @@ def test_url_authority_form_only_connect(parser: HttpRequestParser) -> None:
         b"https:////protected",
         b"https://:80/protected",
         b"https://user@/protected",
+        b"https://",
+        b"https://?q",
+        b"http:protected/x",
+        b"http:/protected/x",
+        b"http:\\\\protected/x",
+    ),
+    ids=(
+        "empty-host",
+        "empty-host-extra-slash",
+        "port-only",
+        "userinfo-only",
+        "no-authority",
+        "empty-host-query",
+        "no-slashes",
+        "one-slash",
+        "backslashes",
     ),
-    ids=("empty-host", "empty-host-extra-slash", "port-only", "userinfo-only"),
 )
 def test_url_absolute_form_empty_host_rejected(
     parser: HttpRequestParser, target: bytes