Skip to content

Commit

Permalink
[PR #9267/947b9c4 backport][3.10] Fix double unquoting in url dispatc…
Browse files Browse the repository at this point in the history
…her (#9269)

Co-authored-by: Sam Bull <[email protected]>
  • Loading branch information
bdraco and Dreamsorcerer authored Sep 23, 2024
1 parent 3eb7282 commit c5a3d81
Show file tree
Hide file tree
Showing 12 changed files with 52 additions and 23 deletions.
1 change: 1 addition & 0 deletions CHANGES/9267.breaking.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Increased minimum yarl version to 1.12.0 -- by :user:`bdraco`.
1 change: 1 addition & 0 deletions CHANGES/9267.bugfix.rst
24 changes: 12 additions & 12 deletions aiohttp/web_urldispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,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.path)
match_dict = self._match(request.rel_url.path_safe)
if match_dict is None:
return None, allowed_methods

Expand Down Expand Up @@ -425,8 +425,7 @@ def _match(self, path: str) -> Optional[Dict[str, str]]:
# string comparison is about 10 times faster than regexp matching
if self._path == path:
return {}
else:
return None
return None

def raw_match(self, path: str) -> bool:
return self._path == path
Expand Down Expand Up @@ -497,10 +496,9 @@ def _match(self, path: str) -> Optional[Dict[str, str]]:
match = self._pattern.fullmatch(path)
if match is None:
return None
else:
return {
key: _unquote_path(value) for key, value in match.groupdict().items()
}
return {
key: _unquote_path_safe(value) for key, value in match.groupdict().items()
}

def raw_match(self, path: str) -> bool:
return self._orig_path == path
Expand Down Expand Up @@ -645,7 +643,7 @@ def set_options_route(self, handler: Handler) -> None:
)

async def resolve(self, request: Request) -> _Resolve:
path = request.rel_url.path
path = request.rel_url.path_safe
method = request.method
allowed_methods = set(self._routes)
if not path.startswith(self._prefix2) and path != self._prefix:
Expand All @@ -654,7 +652,7 @@ async def resolve(self, request: Request) -> _Resolve:
if method not in allowed_methods:
return None, allowed_methods

match_dict = {"filename": _unquote_path(path[len(self._prefix) + 1 :])}
match_dict = {"filename": _unquote_path_safe(path[len(self._prefix) + 1 :])}
return (UrlMappingMatchInfo(match_dict, self._routes[method]), allowed_methods)

def __len__(self) -> int:
Expand Down Expand Up @@ -1035,7 +1033,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.path
url_part = request.rel_url.path_safe
while url_part:
for candidate in resource_index.get(url_part, ()):
match_dict, allowed = await candidate.resolve(request)
Expand Down Expand Up @@ -1286,8 +1284,10 @@ def _quote_path(value: str) -> str:
return URL.build(path=value, encoded=False).raw_path


def _unquote_path(value: str) -> str:
return URL.build(path=value, encoded=True).path.replace("%2F", "/")
def _unquote_path_safe(value: str) -> str:
if "%" not in value:
return value
return value.replace("%2F", "/").replace("%25", "%")


def _requote_path(value: str) -> str:
Expand Down
2 changes: 1 addition & 1 deletion requirements/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,5 @@ pycparser==2.21
# via cffi
uvloop==0.19.0 ; platform_system != "Windows" and implementation_name == "cpython"
# via -r requirements/base.in
yarl==1.11.0
yarl==1.12.0
# via -r requirements/runtime-deps.in
2 changes: 1 addition & 1 deletion requirements/constraints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ webcolors==1.11.1
# via blockdiag
wheel==0.37.0
# via pip-tools
yarl==1.11.0
yarl==1.12.0
# via -r requirements/runtime-deps.in
zipp==3.17.0
# via
Expand Down
2 changes: 1 addition & 1 deletion requirements/dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ webcolors==1.13
# via blockdiag
wheel==0.41.0
# via pip-tools
yarl==1.11.0
yarl==1.12.0
# via -r requirements/runtime-deps.in
zipp==3.17.0
# via
Expand Down
2 changes: 1 addition & 1 deletion requirements/runtime-deps.in
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ Brotli; platform_python_implementation == 'CPython'
brotlicffi; platform_python_implementation != 'CPython'
frozenlist >= 1.1.1
multidict >=4.5, < 7.0
yarl >= 1.0, < 2.0
yarl >= 1.12.0, < 2.0
2 changes: 1 addition & 1 deletion requirements/runtime-deps.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,5 @@ pycares==4.3.0
# via aiodns
pycparser==2.21
# via cffi
yarl==1.11.0
yarl==1.12.0
# via -r requirements/runtime-deps.in
2 changes: 1 addition & 1 deletion requirements/test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -125,5 +125,5 @@ uvloop==0.19.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.11.0
yarl==1.12.0
# via -r requirements/runtime-deps.in
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ install_requires =
attrs >= 17.3.0
frozenlist >= 1.1.1
multidict >=4.5, < 7.0
yarl >= 1.0, < 2.0
yarl >= 1.12.0, < 2.0

[options.exclude_package_data]
* =
Expand Down
17 changes: 14 additions & 3 deletions tests/test_urldispatch.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import pathlib
import re
from collections.abc import Container, Iterable, Mapping, MutableMapping, Sized
from urllib.parse import unquote
from urllib.parse import quote, unquote

import pytest
from re_assert import Matches
Expand Down Expand Up @@ -457,7 +457,7 @@ def test_add_static_quoting(router) -> None:
)
assert router["static"] is resource
url = resource.url_for(filename="/1 2/файл%2F.txt")
assert url.path == "/пре %2Fфикс/1 2/файл%2F.txt"
assert url.path == "/пре /фикс/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"
Expand Down Expand Up @@ -630,7 +630,7 @@ def test_route_dynamic_quoting(router) -> None:
route = router.add_route("GET", r"/пре %2Fфикс/{arg}", handler)

url = route.url_for(arg="1 2/текст%2F")
assert url.path == "/пре %2Fфикс/1 2/текст%2F"
assert url.path == "/пре /фикс/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"
Expand Down Expand Up @@ -742,6 +742,17 @@ async def test_dynamic_match_unquoted_path(router) -> None:
assert match_info == {"path": "path", "subpath": unquote(resource_id)}


async def test_dynamic_match_double_quoted_path(router: web.UrlDispatcher) -> None:
"""Verify that double-quoted path is unquoted only once."""
handler = make_handler()
router.add_route("GET", "/{path}/{subpath}", handler)
resource_id = quote("my/path|with!some%strange$characters", safe="")
double_quoted_resource_id = quote(resource_id, safe="")
req = make_mocked_request("GET", f"/path/{double_quoted_resource_id}")
match_info = await router.resolve(req)
assert match_info == {"path": "path", "subpath": resource_id}


def test_add_route_not_started_with_slash(router) -> None:
with pytest.raises(ValueError):
handler = make_handler()
Expand Down
18 changes: 17 additions & 1 deletion tests/test_web_urldispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import socket
import sys
from stat import S_IFIFO, S_IMODE
from typing import Any, Generator, Optional
from typing import Any, Generator, NoReturn, Optional

import pytest
import yarl
Expand Down Expand Up @@ -885,6 +885,22 @@ async def handler(request: web.Request) -> web.Response:
assert resp.status == expected_http_resp_status


async def test_decoded_raw_match_regex(aiohttp_client: AiohttpClient) -> None:
"""Verify that raw_match only matches decoded url."""
app = web.Application()

async def handler(request: web.Request) -> NoReturn:
assert False

app.router.add_get("/467%2C802%2C24834%2C24952%2C25362%2C40574/hello", handler)
client = await aiohttp_client(app)

async with client.get(
yarl.URL("/467%2C802%2C24834%2C24952%2C25362%2C40574/hello", encoded=True)
) as resp:
assert resp.status == 404 # should only match decoded url


async def test_order_is_preserved(aiohttp_client: AiohttpClient) -> None:
"""Test route order is preserved.
Expand Down

0 comments on commit c5a3d81

Please sign in to comment.