Skip to content

Commit

Permalink
Make METHRE RFC-7230 compliant
Browse files Browse the repository at this point in the history
Definition of method is:
```
    method = token
    tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." /
            "^" / "_" / "`" / "|" / "~" / DIGIT / ALPHA
    token = 1*tchar
```

So he had two issues:
1. Not all the characters were allowed.
2. Actually, we did allowed too much characters since `$-_` parsed as:
"all characters in range between code 36 ($) and code 95 (_)" instead of
"characters with codes 36, 45 and 95". So we did match methods like
`[GET]` which are malformed according the spec.
  • Loading branch information
kxepal committed Sep 2, 2018
1 parent 8aced36 commit 7ac7e4d
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGES/3235.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Make method match regexp RFC-7230 compliant
10 changes: 9 additions & 1 deletion aiohttp/http_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,15 @@
'RawRequestMessage', 'RawResponseMessage')

ASCIISET = set(string.printable)
METHRE = re.compile('[A-Z0-9$-_.]+')

# See https://tools.ietf.org/html/rfc7230#section-3.1.1
# and https://tools.ietf.org/html/rfc7230#appendix-B
#
# method = token
# tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." /
# "^" / "_" / "`" / "|" / "~" / DIGIT / ALPHA
# token = 1*tchar
METHRE = re.compile("[!#$%&'*+\-.^_`|~0-9A-Za-z]+")
VERSRE = re.compile(r'HTTP/(\d+).(\d+)')
HDRRE = re.compile(rb'[\x00-\x1F\x7F()<>@,;:\[\]={} \t\\\\\"]')

Expand Down
2 changes: 1 addition & 1 deletion tests/test_http_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ def test_http_request_parser_two_slashes(parser):

def test_http_request_parser_bad_method(parser):
with pytest.raises(http_exceptions.BadStatusLine):
parser.feed_data(b'!12%()+=~$ /get HTTP/1.1\r\n\r\n')
parser.feed_data(b'=":<G>(e),[T];?" /get HTTP/1.1\r\n\r\n')


def test_http_request_parser_bad_version(parser):
Expand Down
2 changes: 1 addition & 1 deletion tests/test_web_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ async def test_simple(srv, loop, buf):

async def test_bad_method(srv, loop, buf):
srv.data_received(
b'!@#$ / HTTP/1.0\r\n'
b':BAD; / HTTP/1.0\r\n'
b'Host: example.com\r\n\r\n')

await asyncio.sleep(0, loop=loop)
Expand Down

3 comments on commit 7ac7e4d

@webknjaz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/he had/it had/

@kxepal
Copy link
Member Author

@kxepal kxepal commented on 7ac7e4d Sep 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we actually (;

@kxepal
Copy link
Member Author

@kxepal kxepal commented on 7ac7e4d Sep 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks.

Please sign in to comment.