Skip to content

Commit

Permalink
do not use yarl.quote directly ; escaping should be consistent with t…
Browse files Browse the repository at this point in the history
…he one done in yarl.URL.__init__
  • Loading branch information
arthurdarcet committed Jan 23, 2017
1 parent 1d0750e commit 0732059
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 7 deletions.
17 changes: 10 additions & 7 deletions aiohttp/web_urldispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
from pathlib import Path
from types import MappingProxyType

from yarl import URL, quote, unquote
# do not use yarl.quote directly, use `URL(path).raw_path` instead of `quote(path)`
# Escaping of the URLs need to be consitent with the escaping done by yarl
from yarl import URL, unquote

from . import hdrs
from .abc import AbstractMatchInfo, AbstractRouter, AbstractView
Expand Down Expand Up @@ -373,7 +375,7 @@ def __init__(self, prefix, *, name=None):
assert not prefix or prefix.startswith('/'), prefix
assert prefix in ('', '/') or not prefix.endswith('/'), prefix
super().__init__(name=name)
self._prefix = quote(prefix, safe='/')
self._prefix = URL(prefix).raw_path

def add_prefix(self, prefix):
assert prefix.startswith('/')
Expand Down Expand Up @@ -421,7 +423,7 @@ def url_for(self, *, filename):
while filename.startswith('/'):
filename = filename[1:]
filename = '/' + filename
url = self._prefix + quote(filename, safe='/')
url = self._prefix + URL(filename).raw_path
return URL(url)

def get_info(self):
Expand Down Expand Up @@ -783,7 +785,8 @@ def add_resource(self, path, *, name=None):
if path and not path.startswith('/'):
raise ValueError("path should be started with / or be empty")
if not ('{' in path or '}' in path or self.ROUTE_RE.search(path)):
resource = PlainResource(quote(path, safe='/'), name=name)
url = URL(path)
resource = PlainResource(url.raw_path, name=name)
self.register_resource(resource)
return resource

Expand All @@ -805,9 +808,9 @@ def add_resource(self, path, *, name=None):
if '{' in part or '}' in part:
raise ValueError("Invalid path '{}'['{}']".format(path, part))

part = quote(part, safe='/')
formatter += part
pattern += re.escape(part)
path = URL(part).raw_path
formatter += path
pattern += re.escape(path)

try:
compiled = re.compile(pattern)
Expand Down
20 changes: 20 additions & 0 deletions tests/test_web_urldispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,26 @@ def test_access_non_existing_resource(tmp_dir_path, loop, test_client):
yield from r.release()


@pytest.mark.parametrize('registered_path,request_url', [
('/a:b', '/a:b'),
('/a@b', '/a@b'),
('/a:b', '/a%3Ab'),
])
@asyncio.coroutine
def test_url_escaping(loop, test_client, registered_path, request_url):
"""
Tests accessing a resource with
"""
app = web.Application(loop=loop)
handler = lambda req: web.Response()
app.router.add_get(registered_path, handler)
client = yield from test_client(app)

r = yield from client.get(request_url)
assert r.status == 200
yield from r.release()


@asyncio.coroutine
def test_unauthorized_folder_access(tmp_dir_path, loop, test_client):
"""
Expand Down

0 comments on commit 0732059

Please sign in to comment.