From 6a58c57f6d6317eb655784471ee493f87d6c9415 Mon Sep 17 00:00:00 2001 From: Pontus Karlsson Date: Tue, 2 Aug 2016 14:12:07 +0200 Subject: [PATCH] Relax HTTP method validation in UrlDispatcher. Previously the validation only compared against "safe HTTP methods", now we validate against the allowed character-set defined in RFC 2616 section 5.1.1. --- aiohttp/web_urldispatcher.py | 9 +++-- demos/chat/aiohttpdemo_chat/main.py | 2 +- demos/chat/aiohttpdemo_chat/views.py | 1 - demos/polls/aiohttpdemo_polls/main.py | 2 +- demos/polls/aiohttpdemo_polls/middlewares.py | 1 - demos/polls/aiohttpdemo_polls/views.py | 1 - tests/test_urldispatch.py | 35 ++++++++++++++++++-- 7 files changed, 41 insertions(+), 10 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 40319d5fa80..f8767ba1ac3 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -30,6 +30,9 @@ PY_35 = sys.version_info >= (3, 5) +HTTP_METHOD_RE = re.compile(r"^[0-9A-Za-z!#\$%&'\*\+\-\.\^_`\|~]+$") + + class AbstractResource(Sized, Iterable): def __init__(self, *, name=None): @@ -63,7 +66,6 @@ def _append_query(url, query): class AbstractRoute(abc.ABC): - METHODS = hdrs.METH_ALL | {hdrs.METH_ANY} def __init__(self, method, handler, *, expect_handler=None, @@ -76,7 +78,7 @@ def __init__(self, method, handler, *, 'Coroutine is expected, got {!r}'.format(expect_handler) method = method.upper() - if method not in self.METHODS: + if not self.validate_method(method): raise ValueError("{} is not allowed HTTP method".format(method)) assert callable(handler), handler @@ -103,6 +105,9 @@ def handler_wrapper(*args, **kwargs): self._expect_handler = expect_handler self._resource = resource + def validate_method(self, method): + return HTTP_METHOD_RE.match(method) + @property def method(self): return self._method diff --git a/demos/chat/aiohttpdemo_chat/main.py b/demos/chat/aiohttpdemo_chat/main.py index 933b3aa5d4d..64a86001612 100644 --- a/demos/chat/aiohttpdemo_chat/main.py +++ b/demos/chat/aiohttpdemo_chat/main.py @@ -1,9 +1,9 @@ import asyncio import logging -import aiohttp_jinja2 import jinja2 +import aiohttp_jinja2 from aiohttp import web from aiohttpdemo_chat.views import setup as setup_routes diff --git a/demos/chat/aiohttpdemo_chat/views.py b/demos/chat/aiohttpdemo_chat/views.py index 32cacce5c6b..fe941204456 100644 --- a/demos/chat/aiohttpdemo_chat/views.py +++ b/demos/chat/aiohttpdemo_chat/views.py @@ -4,7 +4,6 @@ import string import aiohttp_jinja2 - from aiohttp import web log = logging.getLogger(__name__) diff --git a/demos/polls/aiohttpdemo_polls/main.py b/demos/polls/aiohttpdemo_polls/main.py index 048523c704e..09444b952e6 100644 --- a/demos/polls/aiohttpdemo_polls/main.py +++ b/demos/polls/aiohttpdemo_polls/main.py @@ -2,9 +2,9 @@ import logging import pathlib -import aiohttp_jinja2 import jinja2 +import aiohttp_jinja2 from aiohttp import web from aiohttpdemo_polls.db import init_postgres from aiohttpdemo_polls.middlewares import setup_middlewares diff --git a/demos/polls/aiohttpdemo_polls/middlewares.py b/demos/polls/aiohttpdemo_polls/middlewares.py index b42c922daa2..9a77cdda7fc 100644 --- a/demos/polls/aiohttpdemo_polls/middlewares.py +++ b/demos/polls/aiohttpdemo_polls/middlewares.py @@ -1,5 +1,4 @@ import aiohttp_jinja2 - from aiohttp import web async def handle_404(request, response): diff --git a/demos/polls/aiohttpdemo_polls/views.py b/demos/polls/aiohttpdemo_polls/views.py index c4d99969838..06a468b0bae 100644 --- a/demos/polls/aiohttpdemo_polls/views.py +++ b/demos/polls/aiohttpdemo_polls/views.py @@ -1,5 +1,4 @@ import aiohttp_jinja2 - from aiohttp import web from . import db diff --git a/tests/test_urldispatch.py b/tests/test_urldispatch.py index 0f3d82af1db..7fcdfa91369 100644 --- a/tests/test_urldispatch.py +++ b/tests/test_urldispatch.py @@ -56,6 +56,24 @@ def test_register_route_checks(self): '/handler/to/path') self.router.register_route(route) + def test_register_uncommon_http_methods(self): + handler = self.make_handler() + + uncommon_http_methods = { + 'PROPFIND', + 'PROPPATCH', + 'COPY', + 'LOCK', + 'UNLOCK' + 'MOVE', + 'SUBSCRIBE', + 'UNSUBSCRIBE', + 'NOTIFY' + } + + for method in uncommon_http_methods: + PlainRoute(method, handler, 'url', '/handler/to/path') + def test_add_route_root(self): handler = self.make_handler() self.router.add_route('GET', '/', handler) @@ -585,9 +603,20 @@ def test_add_route_not_started_with_slash(self): self.router.add_route('GET', 'invalid_path', handler) def test_add_route_invalid_method(self): - with self.assertRaises(ValueError): - handler = self.make_handler() - self.router.add_route('INVALID_METHOD', '/path', handler) + + sample_bad_methods = { + 'BAD METHOD', + 'B@D_METHOD', + '[BAD_METHOD]', + '{BAD_METHOD}', + '(BAD_METHOD)', + 'B?D_METHOD', + } + + for bad_method in sample_bad_methods: + with self.assertRaises(ValueError): + handler = self.make_handler() + self.router.add_route(bad_method, '/path', handler) def fill_routes(self): route1 = self.router.add_route('GET', '/plain', self.make_handler())