Fix router matching pre-encoded URLs (#8898)

Co-authored-by: J. Nick Koston <nick@koston.org>

CHANGES/8898.bugfix.rst

@@ -0,0 +1 @@
+Fixed web router not matching pre-encoded URLs (requires yarl 1.9.6+) -- by :user:`Dreamsorcerer`.

aiohttp/web_urldispatcher.py

@@ -358,7 +358,7 @@ def register_route(self, route: "ResourceRoute") -> None:
     async def resolve(self, request: Request) -> _Resolve:
         allowed_methods: Set[str] = set()
 
-        match_dict = self._match(request.rel_url.raw_path)
+        match_dict = self._match(request.rel_url.path)
         if match_dict is None:
             return None, allowed_methods
 
@@ -623,7 +623,7 @@ def set_options_route(self, handler: Handler) -> None:
         )
 
     async def resolve(self, request: Request) -> _Resolve:
-        path = request.rel_url.raw_path
+        path = request.rel_url.path
         method = request.method
         allowed_methods = set(self._routes)
         if not path.startswith(self._prefix2) and path != self._prefix:
@@ -1012,7 +1012,7 @@ async def resolve(self, request: Request) -> UrlMappingMatchInfo:
         # candidates for a given url part because there are multiple resources
         # registered for the same canonical path, we resolve them in a linear
         # fashion to ensure registration order is respected.
-        url_part = request.rel_url.raw_path
+        url_part = request.rel_url.path
         while url_part:
             for candidate in resource_index.get(url_part, ()):
                 match_dict, allowed = await candidate.resolve(request)
@@ -1137,7 +1137,7 @@ def add_resource(self, path: str, *, name: Optional[str] = None) -> Resource:
             if resource.name == name and resource.raw_match(path):
                 return cast(Resource, resource)
         if not ("{" in path or "}" in path or ROUTE_RE.search(path)):
-            resource = PlainResource(_requote_path(path), name=name)
+            resource = PlainResource(path, name=name)
             self.register_resource(resource)
             return resource
         resource = DynamicResource(path, name=name)
@@ -1262,7 +1262,7 @@ def _quote_path(value: str) -> str:
 
 
 def _unquote_path(value: str) -> str:
-    return URL.build(path=value, encoded=True).path
+    return URL.build(path=value, encoded=True).path.replace("%2F", "/")
 
 
 def _requote_path(value: str) -> str:

requirements/base.txt

@@ -36,5 +36,5 @@ pycparser==2.22
     # via cffi
 uvloop==0.20.0 ; platform_system != "Windows" and implementation_name == "cpython"
     # via -r requirements/base.in
-yarl==1.9.4
+yarl==1.9.6
     # via -r requirements/runtime-deps.in

requirements/constraints.txt

@@ -286,7 +286,7 @@ webcolors==24.8.0
     # via blockdiag
 wheel==0.44.0
     # via pip-tools
-yarl==1.9.4
+yarl==1.9.6
     # via -r requirements/runtime-deps.in
 zipp==3.20.1
     # via

requirements/dev.txt

@@ -278,7 +278,7 @@ webcolors==24.8.0
     # via blockdiag
 wheel==0.44.0
     # via pip-tools
-yarl==1.9.4
+yarl==1.9.6
     # via -r requirements/runtime-deps.in
 zipp==3.20.1
     # via

requirements/runtime-deps.txt

@@ -30,5 +30,5 @@ pycares==4.4.0
     # via aiodns
 pycparser==2.22
     # via cffi
-yarl==1.9.4
+yarl==1.9.6
     # via -r requirements/runtime-deps.in

requirements/test.txt

@@ -134,5 +134,5 @@ uvloop==0.20.0 ; platform_system != "Windows" and implementation_name == "cpytho
     # via -r requirements/base.in
 wait-for-it==2.2.2
     # via -r requirements/test.in
-yarl==1.9.4
+yarl==1.9.6
     # via -r requirements/runtime-deps.in

tests/test_urldispatch.py

@@ -3,7 +3,7 @@
 import re
 from collections.abc import Container, Iterable, Mapping, MutableMapping, Sized
 from functools import partial
-from typing import Awaitable, Callable, List, NoReturn, Optional, Type
+from typing import Awaitable, Callable, Dict, List, NoReturn, Optional, Type
 from urllib.parse import unquote
 
 import pytest
@@ -486,7 +486,7 @@ def test_add_static_quoting(router: web.UrlDispatcher) -> None:
     )
     assert router["static"] is resource
     url = resource.url_for(filename="/1 2/файл%2F.txt")
-    assert url.path == "/пре /фикс/1 2/файл%2F.txt"
+    assert url.path == "/пре %2Fфикс/1 2/файл%2F.txt"
     assert str(url) == (
         "/%D0%BF%D1%80%D0%B5%20%2F%D1%84%D0%B8%D0%BA%D1%81"
         "/1%202/%D1%84%D0%B0%D0%B9%D0%BB%252F.txt"
@@ -562,19 +562,24 @@ def test_static_remove_trailing_slash(router: web.UrlDispatcher) -> None:
     assert "/prefix" == route._prefix
 
 
-async def test_add_route_with_re(router: web.UrlDispatcher) -> None:
+@pytest.mark.parametrize(
+    "pattern,url,expected",
+    (
+        (r"{to:\d+}", r"1234", {"to": "1234"}),
+        ("{name}.html", "test.html", {"name": "test"}),
+        (r"{fn:\w+ \d+}", "abc 123", {"fn": "abc 123"}),
+        (r"{fn:\w+\s\d+}", "abc 123", {"fn": "abc 123"}),
+    ),
+)
+async def test_add_route_with_re(
+    router: web.UrlDispatcher, pattern: str, url: str, expected: Dict[str, str]
+) -> None:
     handler = make_handler()
-    router.add_route("GET", r"/handler/{to:\d+}", handler)
-
-    req = make_mocked_request("GET", "/handler/1234")
+    router.add_route("GET", f"/handler/{pattern}", handler)
+    req = make_mocked_request("GET", f"/handler/{url}")
     info = await router.resolve(req)
     assert info is not None
-    assert {"to": "1234"} == info
-
-    router.add_route("GET", r"/handler/{name}.html", handler)
-    req = make_mocked_request("GET", "/handler/test.html")
-    info = await router.resolve(req)
-    assert {"name": "test"} == info
+    assert info == expected
 
 
 async def test_add_route_with_re_and_slashes(router: web.UrlDispatcher) -> None:
@@ -659,7 +664,7 @@ def test_route_dynamic_quoting(router: web.UrlDispatcher) -> None:
     route = router.add_route("GET", r"/пре %2Fфикс/{arg}", handler)
 
     url = route.url_for(arg="1 2/текст%2F")
-    assert url.path == "/пре /фикс/1 2/текст%2F"
+    assert url.path == "/пре %2Fфикс/1 2/текст%2F"
     assert str(url) == (
         "/%D0%BF%D1%80%D0%B5%20%2F%D1%84%D0%B8%D0%BA%D1%81"
         "/1%202/%D1%82%D0%B5%D0%BA%D1%81%D1%82%252F"

tests/test_web_urldispatcher.py

@@ -835,18 +835,15 @@ async def get_foobar(request: web.Request) -> web.Response:
         assert (await resp.text()) == "success!"
 
 
-@pytest.mark.xfail(
-    raises=AssertionError,
-    reason="Regression in v3.7: https://github.com/aio-libs/aiohttp/issues/5621",
-)
 @pytest.mark.parametrize(
     ("route_definition", "urlencoded_path", "expected_http_resp_status"),
     (
         ("/467,802,24834/hello", "/467%2C802%2C24834/hello", 200),
         ("/{user_ids:([0-9]+)(,([0-9]+))*}/hello", "/467%2C802%2C24834/hello", 200),
+        ("/467,802,24834/hello", "/467,802,24834/hello", 200),
+        ("/{user_ids:([0-9]+)(,([0-9]+))*}/hello", "/467,802,24834/hello", 200),
         ("/1%2C3/hello", "/1%2C3/hello", 404),
     ),
-    ids=("urldecoded_route", "urldecoded_route_with_regex", "urlencoded_route"),
 )
 async def test_decoded_url_match(
     aiohttp_client: AiohttpClient,
@@ -862,9 +859,8 @@ async def handler(request: web.Request) -> web.Response:
     app.router.add_get(route_definition, handler)
     client = await aiohttp_client(app)
 
-    r = await client.get(yarl.URL(urlencoded_path, encoded=True))
-    assert r.status == expected_http_resp_status
-    r.release()
+    async with client.get(yarl.URL(urlencoded_path, encoded=True)) as resp:
+        assert resp.status == expected_http_resp_status
 
 
 async def test_order_is_preserved(aiohttp_client: AiohttpClient) -> None: