Fix raw path (aio-libs#13175)

Author: Dreamsorcerer

CHANGES/13175.bugfix.rst

@@ -0,0 +1,4 @@
+Fixed :attr:`~aiohttp.web.BaseRequest.raw_path` including the scheme and host
+for absolute-form request targets, and made the pure-Python parser reject
+authority-form targets (``host:port``) for methods other than ``CONNECT``
+-- by :user:`Dreamsorcerer`.

aiohttp/http_parser.py

@@ -696,8 +696,9 @@ def parse_message(self, lines: list[bytes]) -> RawRequestMessage:
             # absolute-form for proxy maybe,
             # https://datatracker.ietf.org/doc/html/rfc7230#section-5.3.2
             url = URL(path, encoded=True)
-            if url.scheme == "":
-                # not absolute-form
+            if not url.absolute:
+                # authority-form is only allowed with CONNECT
+                # https://www.rfc-editor.org/info/rfc9112/#section-3.2.3-1
                 raise InvalidURLError(
                     path.encode(errors="surrogateescape").decode("latin1")
                 )

aiohttp/web_request.py

@@ -465,7 +465,26 @@ def raw_path(self) -> str:
 
         E.g., ``/my%2Fpath%7Cwith%21some%25strange%24characters``
         """
-        return self._message.path
+        path = self._message.path
+
+        # An absolute-form target carries a "scheme://authority" that must not
+        # leak into the path. Strip it, keeping the remainder byte-for-byte,
+        # exactly as an origin-form target. Authority-form is used only by
+        # CONNECT and is left unchanged.
+        # https://www.rfc-editor.org/info/rfc9112/#section-3.2.2-9
+        # https://www.rfc-editor.org/info/rfc9112/#name-authority-form
+        if self._message.url.absolute and self._method != "CONNECT":
+            # absolute-form always contains "://" (guaranteed by the parser).
+            scheme_sep = path.find("://")
+            assert scheme_sep != -1
+            cursor = scheme_sep + 3
+            rel = len(path)
+            for delimiter in "/?#":
+                found = path.find(delimiter, cursor)
+                if found != -1:
+                    rel = min(rel, found)
+            return path[rel:]
+        return path
 
     @reify
     def query(self) -> MultiDictProxy[str]:

tests/test_http_parser.py

@@ -1133,6 +1133,12 @@ def test_url_absolute(parser: HttpRequestParser) -> None:
     assert msg.url == URL("https://www.google.com/path/to.html")
 
 
+def test_url_authority_form_only_connect(parser: HttpRequestParser) -> None:
+    # https://www.rfc-editor.org/info/rfc9112/#section-3.2.3-1
+    with pytest.raises(http_exceptions.InvalidURLError):
+        parser.feed_data(b"GET www.google.com:443 HTTP/1.1\r\nHost: a\r\n\r\n")
+
+
 def test_headers_old_websocket_key1(parser: HttpRequestParser) -> None:
     text = b"GET /test HTTP/1.1\r\nHost: a\r\nSEC-WEBSOCKET-KEY1: line\r\n\r\n"
 

tests/test_web_middleware.py

@@ -1,5 +1,6 @@
 import asyncio
 from collections.abc import Awaitable, Callable, Iterable
+from contextlib import suppress
 from typing import NoReturn
 
 import pytest
@@ -415,6 +416,32 @@ async def handle(request: web.Request) -> web.StreamResponse:
         assert resp.headers["Location"] == "/google.com"
         assert resp.url.query == URL("//google.com").query
 
+    async def test_open_redirect_absolute_form_target(
+        self, aiohttp_server: AiohttpServer
+    ) -> None:
+        async def handle(request: web.Request) -> web.Response:
+            assert False
+
+        app = web.Application(middlewares=[web.normalize_path_middleware()])
+        app.add_routes([web.get("/google.com/", handle)])
+        server = await aiohttp_server(app)
+
+        reader, writer = await asyncio.open_connection(server.host, server.port)
+        try:
+            writer.write(
+                b"GET http://google.com/google.com HTTP/1.1\r\n"
+                b"Host: localhost\r\nConnection: close\r\n\r\n"
+            )
+            await writer.drain()
+            head = (await reader.readuntil(b"\r\n\r\n")).decode("ascii")
+        finally:
+            writer.close()
+            with suppress(ConnectionResetError, BrokenPipeError):
+                await writer.wait_closed()
+
+        assert head.startswith("HTTP/1.1 308 ")
+        assert "\r\nLocation: /google.com/\r\n" in head
+
 
 async def test_normalize_path_skips_parser_error(
     aiohttp_server: AiohttpServer,

tests/test_web_request.py

@@ -220,6 +220,41 @@ def test_absolute_url() -> None:
     assert req.rel_url == URL.build(path="/path/to", query={"a": "1"})
 
 
+def test_absolute_form_raw_path() -> None:
+    # An absolute-form target (RFC 9112 3.2.2) must not leak the scheme/host
+    # into raw_path. The path, query and fragment are kept byte-for-byte, the
+    # same raw form an origin-form target yields.
+    req = make_mocked_request("GET", "https://example.com/path/to?a=1#frag")
+    assert req.raw_path == "/path/to?a=1#frag"
+    assert req.raw_path == make_mocked_request("GET", "/path/to?a=1#frag").raw_path
+
+
+def test_connect_authority_form_raw_path() -> None:
+    # Authority-form is only used by CONNECT (RFC 9112 3.2.3); its target is a
+    # bare host:port with no scheme prefix, so raw_path returns it unchanged.
+    message = RawRequestMessage(
+        "CONNECT",
+        "example.com:443",
+        HttpVersion(1, 1),
+        HeadersDictProxy(CIMultiDict()),
+        (),
+        False,
+        None,
+        False,
+        False,
+        URL.build(authority="example.com:443", encoded=True),
+    )
+    protocol = mock.Mock()
+    protocol.ssl_context = None
+    protocol.peername = None
+    protocol.sockname = ("127.0.0.1", 80)
+    req = web.BaseRequest(
+        message, mock.Mock(), protocol, mock.Mock(), mock.Mock(), mock.Mock()
+    )
+    assert req._message.url.absolute
+    assert req.raw_path == "example.com:443"
+
+
 def test_clone_absolute_scheme() -> None:
     req = make_mocked_request("GET", "https://example.com/path/to?a=1")
     assert req.scheme == "https"