From 6687a05fc714775feb730a8a56bc2099a552b4cb Mon Sep 17 00:00:00 2001 From: Alexey Popravka Date: Tue, 20 Mar 2018 18:00:40 +0200 Subject: [PATCH 1/5] add 'actual_size' parameter to LineTooLong exception --- aiohttp/_http_parser.pyx | 11 ++++++----- aiohttp/http_exceptions.py | 5 +++-- aiohttp/http_parser.py | 10 ++++++---- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/aiohttp/_http_parser.pyx b/aiohttp/_http_parser.pyx index e3be670c346..1c47350a660 100644 --- a/aiohttp/_http_parser.pyx +++ b/aiohttp/_http_parser.pyx @@ -370,7 +370,7 @@ cdef int cb_on_url(cparser.http_parser* parser, try: if length > pyparser._max_line_size: raise LineTooLong( - 'Status line is too long', pyparser._max_line_size) + 'Status line is too long', pyparser._max_line_size, length) pyparser._buf.extend(at[:length]) except BaseException as ex: pyparser._last_error = ex @@ -386,7 +386,7 @@ cdef int cb_on_status(cparser.http_parser* parser, try: if length > pyparser._max_line_size: raise LineTooLong( - 'Status line is too long', pyparser._max_line_size) + 'Status line is too long', pyparser._max_line_size, length) pyparser._buf.extend(at[:length]) except BaseException as ex: pyparser._last_error = ex @@ -402,7 +402,7 @@ cdef int cb_on_header_field(cparser.http_parser* parser, pyparser._on_status_complete() if length > pyparser._max_field_size: raise LineTooLong( - 'Header name is too long', pyparser._max_field_size) + 'Header name is too long', pyparser._max_field_size, length) pyparser._on_header_field( at[:length].decode('utf-8', 'surrogateescape'), at[:length]) except BaseException as ex: @@ -419,10 +419,11 @@ cdef int cb_on_header_value(cparser.http_parser* parser, if pyparser._header_value is not None: if len(pyparser._header_value) + length > pyparser._max_field_size: raise LineTooLong( - 'Header value is too long', pyparser._max_field_size) + 'Header value is too long', pyparser._max_field_size, + len(pyparser._header_value) + length) elif length > pyparser._max_field_size: raise LineTooLong( - 'Header value is too long', pyparser._max_field_size) + 'Header value is too long', pyparser._max_field_size, length) pyparser._on_header_value( at[:length].decode('utf-8', 'surrogateescape'), at[:length]) except BaseException as ex: diff --git a/aiohttp/http_exceptions.py b/aiohttp/http_exceptions.py index b4f7a09c50a..a3d4c90e2aa 100644 --- a/aiohttp/http_exceptions.py +++ b/aiohttp/http_exceptions.py @@ -59,9 +59,10 @@ class ContentLengthError(PayloadEncodingError): class LineTooLong(BadHttpMessage): - def __init__(self, line, limit='Unknown'): + def __init__(self, line, limit='Unknown', actual_size='Unknown'): super().__init__( - "Got more than %s bytes when reading %s." % (limit, line)) + "Got more than %s bytes (%s) when reading %s." % ( + limit, actual_size, line)) class InvalidHeader(BadHttpMessage): diff --git a/aiohttp/http_parser.py b/aiohttp/http_parser.py index d31a62042e7..38d17460f9a 100644 --- a/aiohttp/http_parser.py +++ b/aiohttp/http_parser.py @@ -283,7 +283,8 @@ def parse_headers(self, lines): raise LineTooLong( 'request header field {}'.format( bname.decode("utf8", "xmlcharrefreplace")), - self.max_field_size) + self.max_field_size, + header_length) bvalue.append(line) # next line @@ -301,7 +302,8 @@ def parse_headers(self, lines): raise LineTooLong( 'request header field {}'.format( bname.decode("utf8", "xmlcharrefreplace")), - self.max_field_size) + self.max_field_size, + header_length) bvalue = bvalue.strip() name = bname.decode('utf-8', 'surrogateescape') @@ -351,7 +353,7 @@ class HttpRequestParserPy(HttpParser): def parse_message(self, lines): if len(lines[0]) > self.max_line_size: raise LineTooLong( - 'Status line is too long', self.max_line_size) + 'Status line is too long', self.max_line_size, len(lines[0])) # request line line = lines[0].decode('utf-8', 'surrogateescape') @@ -399,7 +401,7 @@ class HttpResponseParserPy(HttpParser): def parse_message(self, lines): if len(lines[0]) > self.max_line_size: raise LineTooLong( - 'Status line is too long', self.max_line_size) + 'Status line is too long', self.max_line_size, len(lines[0])) line = lines[0].decode('utf-8', 'surrogateescape') try: From de3952b0da74b582798705574beba2a9adb05600 Mon Sep 17 00:00:00 2001 From: Alexey Popravka Date: Tue, 20 Mar 2018 18:14:04 +0200 Subject: [PATCH 2/5] update (break) tests --- tests/test_http_parser.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/test_http_parser.py b/tests/test_http_parser.py index 57232ad63b6..51dd103a585 100644 --- a/tests/test_http_parser.py +++ b/tests/test_http_parser.py @@ -362,7 +362,8 @@ def test_max_header_field_size(parser): name = b'test' * 10 * 1024 text = (b'GET /test HTTP/1.1\r\n' + name + b':data\r\n\r\n') - with pytest.raises(http_exceptions.LineTooLong): + match = "400, message='Got more than 8190 bytes \(40960\) when reading" + with pytest.raises(http_exceptions.LineTooLong, match=match): parser.feed_data(text) @@ -371,7 +372,8 @@ def test_max_header_value_size(parser): text = (b'GET /test HTTP/1.1\r\n' b'data:' + name + b'\r\n\r\n') - with pytest.raises(http_exceptions.LineTooLong): + match = "400, message='Got more than 8190 bytes \(40960\) when reading" + with pytest.raises(http_exceptions.LineTooLong, match=match): parser.feed_data(text) @@ -380,7 +382,8 @@ def test_max_header_value_size_continuation(parser): text = (b'GET /test HTTP/1.1\r\n' b'data: test\r\n ' + name + b'\r\n\r\n') - with pytest.raises(http_exceptions.LineTooLong): + match = "400, message='Got more than 8190 bytes \(40965\) when reading" + with pytest.raises(http_exceptions.LineTooLong, match=match): parser.feed_data(text) @@ -453,7 +456,8 @@ def test_http_request_parser_bad_version(parser): def test_http_request_max_status_line(parser): - with pytest.raises(http_exceptions.LineTooLong): + match = "400, message='Got more than 8190 bytes \(40965\) when reading" + with pytest.raises(http_exceptions.LineTooLong, match=match): parser.feed_data( b'GET /path' + b'test' * 10 * 1024 + b' HTTP/1.1\r\n\r\n') @@ -475,7 +479,8 @@ def test_http_response_parser_utf8(response): def test_http_response_parser_bad_status_line_too_long(response): - with pytest.raises(http_exceptions.LineTooLong): + match = "400, message='Got more than 8190 bytes \(40962\) when reading" + with pytest.raises(http_exceptions.LineTooLong, match=match): response.feed_data( b'HTTP/1.1 200 Ok' + b'test' * 10 * 1024 + b'\r\n\r\n') From 3998b3ce99257405f8699efb99f1e680c2c992af Mon Sep 17 00:00:00 2001 From: Alexey Popravka Date: Tue, 3 Apr 2018 11:47:21 +0300 Subject: [PATCH 3/5] fix test fixtures in test_http_parser. HttpRequestParserPy and HttpRequestParserC have different signatures. --- tests/test_http_parser.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/test_http_parser.py b/tests/test_http_parser.py index 51dd103a585..37f5d02c0ab 100644 --- a/tests/test_http_parser.py +++ b/tests/test_http_parser.py @@ -38,7 +38,10 @@ def protocol(): @pytest.fixture(params=REQUEST_PARSERS) def parser(loop, protocol, request): """Parser implementations""" - return request.param(protocol, loop, 8190, 32768, 8190) + return request.param(protocol, loop, + max_line_size=8190, + max_headers=32768, + max_field_size=8190) @pytest.fixture(params=REQUEST_PARSERS) @@ -50,7 +53,10 @@ def request_cls(request): @pytest.fixture(params=RESPONSE_PARSERS) def response(loop, protocol, request): """Parser implementations""" - return request.param(protocol, loop, 8190, 32768, 8190) + return request.param(protocol, loop, + max_line_size=8190, + max_headers=32768, + max_field_size=8190) @pytest.fixture(params=RESPONSE_PARSERS) From e972be7a91e10805c7031619459ef8115c6c8f04 Mon Sep 17 00:00:00 2001 From: Alexey Popravka Date: Tue, 3 Apr 2018 11:49:52 +0300 Subject: [PATCH 4/5] Update HttpRequestParserPy/HttpResponseParserPy to count field lengths the same way the C-parser does. --- aiohttp/http_parser.py | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/aiohttp/http_parser.py b/aiohttp/http_parser.py index 38d17460f9a..89571960b3a 100644 --- a/aiohttp/http_parser.py +++ b/aiohttp/http_parser.py @@ -256,8 +256,6 @@ def parse_headers(self, lines): line_count = len(lines) while line: - header_length = len(line) - # Parse initial header name : value pair. try: bname, bvalue = line.split(b':', 1) @@ -265,8 +263,17 @@ def parse_headers(self, lines): raise InvalidHeader(line) from None bname = bname.strip(b' \t') + bvalue = bvalue.lstrip() if HDRRE.search(bname): raise InvalidHeader(bname) + if len(bname) > self.max_field_size: + raise LineTooLong( + "request header name {}".format( + bname.decode("utf8", "xmlcharrefreplace")), + self.max_field_size, + len(bname)) + + header_length = len(bvalue) # next line lines_idx += 1 @@ -351,10 +358,6 @@ class HttpRequestParserPy(HttpParser): """ def parse_message(self, lines): - if len(lines[0]) > self.max_line_size: - raise LineTooLong( - 'Status line is too long', self.max_line_size, len(lines[0])) - # request line line = lines[0].decode('utf-8', 'surrogateescape') try: @@ -362,6 +365,10 @@ def parse_message(self, lines): except ValueError: raise BadStatusLine(line) from None + if len(path) > self.max_line_size: + raise LineTooLong( + 'Status line is too long', self.max_line_size, len(path)) + # method method = method.upper() if not METHRE.match(method): @@ -399,20 +406,21 @@ class HttpResponseParserPy(HttpParser): Returns RawResponseMessage""" def parse_message(self, lines): - if len(lines[0]) > self.max_line_size: - raise LineTooLong( - 'Status line is too long', self.max_line_size, len(lines[0])) - line = lines[0].decode('utf-8', 'surrogateescape') try: version, status = line.split(None, 1) except ValueError: raise BadStatusLine(line) from None - else: - try: - status, reason = status.split(None, 1) - except ValueError: - reason = '' + + try: + status, reason = status.split(None, 1) + except ValueError: + reason = '' + + if len(reason) > self.max_line_size: + raise LineTooLong( + 'Status line is too long', self.max_line_size, + len(reason)) # version match = VERSRE.match(version) From a03d50b056d8362641effcffa0cdbcacf2a69e3b Mon Sep 17 00:00:00 2001 From: Alexey Popravka Date: Tue, 3 Apr 2018 12:26:50 +0300 Subject: [PATCH 5/5] Update parser tests with more cases; --- tests/test_http_parser.py | 102 ++++++++++++++++++++++++++++++++------ 1 file changed, 87 insertions(+), 15 deletions(-) diff --git a/tests/test_http_parser.py b/tests/test_http_parser.py index 37f5d02c0ab..c5f25871b68 100644 --- a/tests/test_http_parser.py +++ b/tests/test_http_parser.py @@ -364,35 +364,82 @@ def test_invalid_name(parser): parser.feed_data(text) -def test_max_header_field_size(parser): - name = b'test' * 10 * 1024 +@pytest.mark.parametrize('size', [40960, 8191]) +def test_max_header_field_size(parser, size): + name = b't' * size text = (b'GET /test HTTP/1.1\r\n' + name + b':data\r\n\r\n') - match = "400, message='Got more than 8190 bytes \(40960\) when reading" + match = ("400, message='Got more than 8190 bytes \({}\) when reading" + .format(size)) with pytest.raises(http_exceptions.LineTooLong, match=match): parser.feed_data(text) -def test_max_header_value_size(parser): - name = b'test' * 10 * 1024 +def test_max_header_field_size_under_limit(parser): + name = b't' * 8190 + text = (b'GET /test HTTP/1.1\r\n' + name + b':data\r\n\r\n') + + messages, upgrade, tail = parser.feed_data(text) + msg = messages[0][0] + assert msg == ( + 'GET', '/test', (1, 1), + CIMultiDict({name.decode(): 'data'}), + ((name, b'data'),), + False, None, False, False, URL('/test')) + + +@pytest.mark.parametrize('size', [40960, 8191]) +def test_max_header_value_size(parser, size): + name = b't' * size text = (b'GET /test HTTP/1.1\r\n' b'data:' + name + b'\r\n\r\n') - match = "400, message='Got more than 8190 bytes \(40960\) when reading" + match = ("400, message='Got more than 8190 bytes \({}\) when reading" + .format(size)) with pytest.raises(http_exceptions.LineTooLong, match=match): parser.feed_data(text) -def test_max_header_value_size_continuation(parser): - name = b'test' * 10 * 1024 +def test_max_header_value_size_under_limit(parser): + value = b'A' * 8190 + text = (b'GET /test HTTP/1.1\r\n' + b'data:' + value + b'\r\n\r\n') + + messages, upgrade, tail = parser.feed_data(text) + msg = messages[0][0] + assert msg == ( + 'GET', '/test', (1, 1), + CIMultiDict({'data': value.decode()}), + ((b'data', value),), + False, None, False, False, URL('/test')) + + +@pytest.mark.parametrize('size', [40965, 8191]) +def test_max_header_value_size_continuation(parser, size): + name = b'T' * (size - 5) text = (b'GET /test HTTP/1.1\r\n' b'data: test\r\n ' + name + b'\r\n\r\n') - match = "400, message='Got more than 8190 bytes \(40965\) when reading" + match = ("400, message='Got more than 8190 bytes \({}\) when reading" + .format(size)) with pytest.raises(http_exceptions.LineTooLong, match=match): parser.feed_data(text) +def test_max_header_value_size_continuation_under_limit(parser): + value = b'A' * 8185 + text = (b'GET /test HTTP/1.1\r\n' + b'data: test\r\n ' + value + b'\r\n\r\n') + + messages, upgrade, tail = parser.feed_data(text) + msg = messages[0][0] + assert msg == ( + 'GET', '/test', (1, 1), + CIMultiDict({'data': 'test ' + value.decode()}), + ((b'data', b'test ' + value),), + False, None, False, False, URL('/test')) + + def test_http_request_parser(parser): text = b'GET /path HTTP/1.1\r\n\r\n' messages, upgrade, tail = parser.feed_data(text) @@ -461,11 +508,23 @@ def test_http_request_parser_bad_version(parser): parser.feed_data(b'GET //get HT/11\r\n\r\n') -def test_http_request_max_status_line(parser): - match = "400, message='Got more than 8190 bytes \(40965\) when reading" +@pytest.mark.parametrize('size', [40965, 8191]) +def test_http_request_max_status_line(parser, size): + path = b't' * (size - 5) + match = ("400, message='Got more than 8190 bytes \({}\) when reading" + .format(size)) with pytest.raises(http_exceptions.LineTooLong, match=match): parser.feed_data( - b'GET /path' + b'test' * 10 * 1024 + b' HTTP/1.1\r\n\r\n') + b'GET /path' + path + b' HTTP/1.1\r\n\r\n') + + +def test_http_request_max_status_line_under_limit(parser): + path = b't' * (8190 - 5) + messages, upgraded, tail = parser.feed_data( + b'GET /path' + path + b' HTTP/1.1\r\n\r\n') + msg = messages[0][0] + assert msg == ('GET', '/path' + path.decode(), (1, 1), CIMultiDict(), (), + False, None, False, False, URL('/path' + path.decode())) def test_http_response_parser_utf8(response): @@ -484,11 +543,24 @@ def test_http_response_parser_utf8(response): assert not tail -def test_http_response_parser_bad_status_line_too_long(response): - match = "400, message='Got more than 8190 bytes \(40962\) when reading" +@pytest.mark.parametrize('size', [40962, 8191]) +def test_http_response_parser_bad_status_line_too_long(response, size): + reason = b't' * (size - 2) + match = ("400, message='Got more than 8190 bytes \({}\) when reading" + .format(size)) with pytest.raises(http_exceptions.LineTooLong, match=match): response.feed_data( - b'HTTP/1.1 200 Ok' + b'test' * 10 * 1024 + b'\r\n\r\n') + b'HTTP/1.1 200 Ok' + reason + b'\r\n\r\n') + + +def test_http_response_parser_status_line_under_limit(response): + reason = b'O' * 8190 + messages, upgraded, tail = response.feed_data( + b'HTTP/1.1 200 ' + reason + b'\r\n\r\n') + msg = messages[0][0] + assert msg.version == (1, 1) + assert msg.code == 200 + assert msg.reason == reason.decode() def test_http_response_parser_bad_version(response):