From 5466bf1c94d38e75bc053b0cfc163e2f948fe345 Mon Sep 17 00:00:00 2001 From: Senthil Kumaran Date: Sat, 18 Dec 2010 16:55:23 +0000 Subject: Fix Issue6791 - Limit the HTTP header readline with _MAXLENGTH. Patch by Antoine Pitrou --- Lib/http/client.py | 33 +++++++++++++++++++++++++++------ Lib/http/server.py | 8 ++++++-- Lib/test/test_httplib.py | 27 +++++++++++++++++++++++++++ Lib/test/test_httpservers.py | 7 +++++++ 4 files changed, 67 insertions(+), 8 deletions(-) diff --git a/Lib/http/client.py b/Lib/http/client.py index 7c97560..8ea75ce 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -203,6 +203,9 @@ responses = { # maximal amount of data to read at one time in _safe_read MAXAMOUNT = 1048576 +# maximal line length when calling readline(). +_MAXLINE = 65536 + class HTTPMessage(email.message.Message): # XXX The only usage of this method is in # http.server.CGIHTTPRequestHandler. Maybe move the code there so @@ -245,7 +248,9 @@ def parse_headers(fp, _class=HTTPMessage): """ headers = [] while True: - line = fp.readline() + line = fp.readline(_MAXLINE + 1) + if len(line) > _MAXLINE: + raise LineTooLong("header line") headers.append(line) if line in (b'\r\n', b'\n', b''): break @@ -299,7 +304,9 @@ class HTTPResponse(io.RawIOBase): self.will_close = _UNKNOWN # conn will close at end of response def _read_status(self): - line = str(self.fp.readline(), "iso-8859-1") + line = str(self.fp.readline(_MAXLINE + 1), "iso-8859-1") + if len(line) > _MAXLINE: + raise LineTooLong("status line") if self.debuglevel > 0: print("reply:", repr(line)) if not line: @@ -340,7 +347,10 @@ class HTTPResponse(io.RawIOBase): break # skip the header from the 100 response while True: - skip = self.fp.readline().strip() + skip = self.fp.readline(_MAXLINE + 1) + if len(skip) > _MAXLINE: + raise LineTooLong("header line") + skip = skip.strip() if not skip: break if self.debuglevel > 0: @@ -508,7 +518,9 @@ class HTTPResponse(io.RawIOBase): value = [] while True: if chunk_left is None: - line = self.fp.readline() + line = self.fp.readline(_MAXLINE + 1) + if len(line) > _MAXLINE: + raise LineTooLong("chunk size") i = line.find(b";") if i >= 0: line = line[:i] # strip chunk-extensions @@ -543,7 +555,9 @@ class HTTPResponse(io.RawIOBase): # read and discard trailer up to the CRLF terminator ### note: we shouldn't have any trailers! while True: - line = self.fp.readline() + line = self.fp.readline(_MAXLINE + 1) + if len(line) > _MAXLINE: + raise LineTooLong("trailer line") if not line: # a vanishingly small number of sites EOF without # sending the trailer @@ -692,7 +706,9 @@ class HTTPConnection: raise socket.error("Tunnel connection failed: %d %s" % (code, message.strip())) while True: - line = response.fp.readline() + line = response.fp.readline(_MAXLINE + 1) + if len(line) > _MAXLINE: + raise LineTooLong("header line") if line == b'\r\n': break @@ -1137,5 +1153,10 @@ class BadStatusLine(HTTPException): self.args = line, self.line = line +class LineTooLong(HTTPException): + def __init__(self, line_type): + HTTPException.__init__(self, "got more than %d bytes when reading %s" + % (_MAXLINE, line_type)) + # for backwards compatibility error = HTTPException diff --git a/Lib/http/server.py b/Lib/http/server.py index f1538f4..515572f 100644 --- a/Lib/http/server.py +++ b/Lib/http/server.py @@ -314,8 +314,12 @@ class BaseHTTPRequestHandler(socketserver.StreamRequestHandler): self.command, self.path, self.request_version = command, path, version # Examine the headers and look for a Connection directive. - self.headers = http.client.parse_headers(self.rfile, - _class=self.MessageClass) + try: + self.headers = http.client.parse_headers(self.rfile, + _class=self.MessageClass) + except http.client.LineTooLong: + self.send_error(400, "Line too long") + return False conntype = self.headers.get('Connection', "") if conntype.lower() == 'close': diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index 426995b..7dae65d 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -317,6 +317,33 @@ class BasicTest(TestCase): self.assertEqual("Basic realm=\"example\"", resp.getheader("www-authenticate")) + # Test lines overflowing the max line size (_MAXLINE in http.client) + + def test_overflowing_status_line(self): + body = "HTTP/1.1 200 Ok" + "k" * 65536 + "\r\n" + resp = client.HTTPResponse(FakeSocket(body)) + self.assertRaises((client.LineTooLong, client.BadStatusLine), resp.begin) + + def test_overflowing_header_line(self): + body = ( + 'HTTP/1.1 200 OK\r\n' + 'X-Foo: bar' + 'r' * 65536 + '\r\n\r\n' + ) + resp = client.HTTPResponse(FakeSocket(body)) + self.assertRaises(client.LineTooLong, resp.begin) + + def test_overflowing_chunked_line(self): + body = ( + 'HTTP/1.1 200 OK\r\n' + 'Transfer-Encoding: chunked\r\n\r\n' + + '0' * 65536 + 'a\r\n' + 'hello world\r\n' + '0\r\n' + ) + resp = client.HTTPResponse(FakeSocket(body)) + resp.begin() + self.assertRaises(client.LineTooLong, resp.read) + class OfflineTest(TestCase): def test_responses(self): self.assertEqual(client.responses[client.NOT_FOUND], "Not Found") diff --git a/Lib/test/test_httpservers.py b/Lib/test/test_httpservers.py index 85b5ec4..19d3d17 100644 --- a/Lib/test/test_httpservers.py +++ b/Lib/test/test_httpservers.py @@ -573,6 +573,13 @@ class BaseHTTPRequestHandlerTestCase(unittest.TestCase): self.assertEqual(result[0], b'HTTP/1.1 414 Request-URI Too Long\r\n') self.assertFalse(self.handler.get_called) + def test_header_length(self): + # Issue #6791: same for headers + result = self.send_typical_request( + b'GET / HTTP/1.1\r\nX-Foo: bar' + b'r' * 65537 + b'\r\n\r\n') + self.assertEqual(result[0], b'HTTP/1.1 400 Line too long\r\n') + self.assertFalse(self.handler.get_called) + class SimpleHTTPRequestHandlerTestCase(unittest.TestCase): """ Test url parsing """ def setUp(self): -- cgit v0.12