diff --git a/CHANGES/8898.bugfix.rst b/CHANGES/8898.bugfix.rst new file mode 100644 index 00000000000..0de6646c8cb --- /dev/null +++ b/CHANGES/8898.bugfix.rst @@ -0,0 +1 @@ +Fixed web router not matching pre-encoded URLs (requires yarl 1.9.6+) -- by :user:`Dreamsorcerer`. diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 65a62e099a5..05419e4544b 100644 --- a/aiohttp/web_urldispatcher.py +++ b/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: diff --git a/requirements/base.txt b/requirements/base.txt index 93bbd68ef40..5f88ac15fa4 100644 --- a/requirements/base.txt +++ b/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 diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 8736e3ed696..27148853268 100644 --- a/requirements/constraints.txt +++ b/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 diff --git a/requirements/dev.txt b/requirements/dev.txt index 53a3dc2b1dd..b3b76a19e23 100644 --- a/requirements/dev.txt +++ b/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 diff --git a/requirements/runtime-deps.txt b/requirements/runtime-deps.txt index 82a745383a0..67b0e522cdd 100644 --- a/requirements/runtime-deps.txt +++ b/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 diff --git a/requirements/test.txt b/requirements/test.txt index 6e54f0e9665..b5dead0f3dc 100644 --- a/requirements/test.txt +++ b/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 diff --git a/tests/test_urldispatch.py b/tests/test_urldispatch.py index 16348c0029c..705734c0933 100644 --- a/tests/test_urldispatch.py +++ b/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" diff --git a/tests/test_web_urldispatcher.py b/tests/test_web_urldispatcher.py index 70b69c30338..20ed160e39e 100644 --- a/tests/test_web_urldispatcher.py +++ b/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: