Skip to content

Commit

Permalink
Relax HTTP method validation in UrlDispatcher. (#1037)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
wolfhechel authored and asvetlov committed Aug 2, 2016
1 parent 8f0a5ea commit 34b14c5
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 10 deletions.
9 changes: 7 additions & 2 deletions aiohttp/web_urldispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion demos/chat/aiohttpdemo_chat/main.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down
1 change: 0 additions & 1 deletion demos/chat/aiohttpdemo_chat/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import string

import aiohttp_jinja2

from aiohttp import web

log = logging.getLogger(__name__)
Expand Down
2 changes: 1 addition & 1 deletion demos/polls/aiohttpdemo_polls/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion demos/polls/aiohttpdemo_polls/middlewares.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import aiohttp_jinja2

from aiohttp import web

async def handle_404(request, response):
Expand Down
1 change: 0 additions & 1 deletion demos/polls/aiohttpdemo_polls/views.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import aiohttp_jinja2

from aiohttp import web

from . import db
Expand Down
35 changes: 32 additions & 3 deletions tests/test_urldispatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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())
Expand Down

0 comments on commit 34b14c5

Please sign in to comment.