Replace MIME parsing with custom HTTP parsing.

Given that websockets makes straightforward use of HTTP, that websocket
implementations can be expected not to exhibit legacy behaviors, and
that RFC 7230 deprecates this behavior, parsing HTTP is doable.

Thanks https://github.com/njsmith/h11 for providing some inspiration,
especially for translating the RFC to regular expressions and figuring
out some edge cases.

I expect the new implementation to be faster, since it has a much
tighter focus than the stdlib's general purpose MIME parser, and
possibly more secure, since it was written from the beginning with
security as a primary goal (with the caveat that it's new code,
which means it's more likely to have security issues).

Fix #19.

Author: aaugustin

docs/changelog.rst

@@ -14,6 +14,8 @@ Changelog
 
 * Made read and write buffer sizes configurable.
 
+* Rewrote HTTP handling for simplicity and performance.
+
 3.3
 ...
 

websockets/client.py

@@ -5,7 +5,7 @@
 
 import asyncio
 import collections.abc
-import email.message
+import http.client
 
 from .exceptions import InvalidHandshake, InvalidMessage
 from .handshake import build_request, check_response
@@ -35,9 +35,8 @@ def write_http_request(self, path, headers):
 
         """
         self.path = path
-        self.request_headers = email.message.Message()
-        for name, value in headers:
-            self.request_headers[name] = value
+        self.request_headers = http.client.HTTPMessage()
+        self.request_headers._headers = headers     # HACK
         self.raw_request_headers = headers
 
         # Since the path and headers only contain ASCII characters,
@@ -63,10 +62,11 @@ def read_http_response(self):
         except ValueError as exc:
             raise InvalidMessage("Malformed HTTP message") from exc
 
-        self.response_headers = headers
-        self.raw_response_headers = list(headers.raw_items())
+        self.response_headers = http.client.HTTPMessage()
+        self.response_headers._headers = headers    # HACK
+        self.raw_response_headers = headers
 
-        return status_code, headers
+        return status_code, self.response_headers
 
     def process_subprotocol(self, get_header, subprotocols=None):
         """

websockets/http.py

@@ -8,8 +8,7 @@
 """
 
 import asyncio
-import email.parser
-import io
+import re
 import sys
 
 from .version import version as websockets_version
@@ -26,6 +25,26 @@
 ))
 
 
+# See https://tools.ietf.org/html/rfc7230#appendix-B.
+
+# Regex for validating header names.
+
+_token_re = re.compile(rb'^[-!#$%&\'*+.^_`|~0-9a-zA-Z]+$')
+
+# Regex for validating header values.
+
+# We don't attempt to support obsolete line folding.
+
+# Include HTAB (\x09), SP (\x20), VCHAR (\x21-\x7e), obs-text (\x80-\xff).
+
+# The ABNF is complicated because it attempts to express that optional
+# whitespace is ignored. We strip whitespace and don't revalidate that.
+
+# See also https://www.rfc-editor.org/errata_search.php?rfc=7230&eid=4189
+
+_value_re = re.compile(rb'^[\x09\x20-\x7e\x80-\xff]*$')
+
+
 @asyncio.coroutine
 def read_request(stream):
     """
@@ -34,20 +53,38 @@ def read_request(stream):
     ``stream`` is an :class:`~asyncio.StreamReader`.
 
     Return ``(path, headers)`` where ``path`` is a :class:`str` and
-    ``headers`` is a :class:`~email.message.Message`. ``path`` isn't
-    URL-decoded.
+    ``headers`` is a list of ``(name, value)`` tuples.
+
+    ``path`` isn't URL-decoded or validated in any way.
+
+    Non-ASCII characters are represented with surrogate escapes.
 
     Raise an exception if the request isn't well formatted.
 
     The request is assumed not to contain a body.
 
     """
-    request_line, headers = yield from read_message(stream)
-    method, path, version = request_line[:-2].decode().split(None, 2)
-    if method != 'GET':
-        raise ValueError("Unsupported method")
-    if version != 'HTTP/1.1':
-        raise ValueError("Unsupported HTTP version")
+    # https://tools.ietf.org/html/rfc7230#section-3.1.1
+
+    # Parsing is simple because fixed values are expected for method and
+    # version and because path isn't checked. Since WebSocket software tends
+    # to implement HTTP/1.1 strictly, there's little need for lenient parsing.
+
+    # Given the implementation of read_line(), request_line ends with CRLF.
+    request_line = yield from read_line(stream)
+
+    # This may raise "ValueError: not enough values to unpack"
+    method, path, version = request_line[:-2].split(b' ', 2)
+
+    if method != b'GET':
+        raise ValueError("Unsupported HTTP method: %r" % method)
+    if version != b'HTTP/1.1':
+        raise ValueError("Unsupported HTTP version: %r" % version)
+
+    path = path.decode('ascii', 'surrogateescape')
+
+    headers = yield from read_headers(stream)
+
     return path, headers
 
 
@@ -59,45 +96,82 @@ def read_response(stream):
     ``stream`` is an :class:`~asyncio.StreamReader`.
 
     Return ``(status, headers)`` where ``status`` is a :class:`int` and
-    ``headers`` is a :class:`~email.message.Message`.
+    ``headers`` is a list of ``(name, value)`` tuples.
+
+    Non-ASCII characters are represented with surrogate escapes.
 
     Raise an exception if the request isn't well formatted.
 
     The response is assumed not to contain a body.
 
     """
-    status_line, headers = yield from read_message(stream)
-    version, status, reason = status_line[:-2].decode().split(" ", 2)
-    if version != 'HTTP/1.1':
-        raise ValueError("Unsupported HTTP version")
-    return int(status), headers
+    # https://tools.ietf.org/html/rfc7230#section-3.1.2
+
+    # As in read_request, parsing is simple because a fixed value is expected
+    # for version, status is a 3-digit number, and reason can be ignored.
+
+    # Given the implementation of read_line(), status_line ends with CRLF.
+    status_line = yield from read_line(stream)
+
+    # This may raise "ValueError: not enough values to unpack"
+    version, status, reason = status_line[:-2].split(b' ', 2)
+
+    if version != b'HTTP/1.1':
+        raise ValueError("Unsupported HTTP version: %r" % version)
+    # This may raise "ValueError: invalid literal for int() with base 10"
+    status = int(status)
+    if not 100 <= status < 1000:
+        raise ValueError("Unsupported HTTP status code: %d" % status)
+    if not _value_re.match(reason):
+        raise ValueError("Invalid HTTP reason phrase: %r" % reason)
+
+    headers = yield from read_headers(stream)
+
+    return status, headers
 
 
 @asyncio.coroutine
-def read_message(stream):
+def read_headers(stream):
     """
     Read an HTTP message from ``stream``.
 
     ``stream`` is an :class:`~asyncio.StreamReader`.
 
     Return ``(start_line, headers)`` where ``start_line`` is :class:`bytes`
-    and ``headers`` is a :class:`~email.message.Message`.
+    and ``headers`` is a list of ``(name, value)`` tuples.
+
+    Non-ASCII characters are represented with surrogate escapes.
 
     The message is assumed not to contain a body.
 
     """
-    start_line = yield from read_line(stream)
-    header_lines = io.BytesIO()
-    for num in range(MAX_HEADERS):
-        header_line = yield from read_line(stream)
-        header_lines.write(header_line)
-        if header_line == b'\r\n':
+    # https://tools.ietf.org/html/rfc7230#section-3.2
+
+    # We don't attempt to support obsolete line folding.
+
+    headers = []
+    for _ in range(MAX_HEADERS):
+        line = yield from read_line(stream)
+        if line == b'\r\n':
             break
+
+        # This may raise "ValueError: not enough values to unpack"
+        name, value = line[:-2].split(b':', 1)
+        if not _token_re.match(name):
+            raise ValueError("Invalid HTTP header name: %r" % name)
+        value = value.strip(b' \t')
+        if not _value_re.match(value):
+            raise ValueError("Invalid HTTP header value: %r" % value)
+
+        headers.append((
+            name.decode('ascii'),   # guaranteed to be ASCII at this point
+            value.decode('ascii', 'surrogateescape'),
+        ))
+
     else:
-        raise ValueError("Too many headers")
-    header_lines.seek(0)
-    headers = email.parser.BytesHeaderParser().parse(header_lines)
-    return start_line, headers
+        raise ValueError("Too many HTTP headers")
+
+    return headers
 
 
 @asyncio.coroutine
@@ -108,9 +182,12 @@ def read_line(stream):
     ``stream`` is an :class:`~asyncio.StreamReader`.
 
     """
+    # Security: this is bounded by the StreamReader's limit (default = 32kB).
     line = yield from stream.readline()
+    # Security: this guarantees header values are small (hardcoded = 4kB)
     if len(line) > MAX_LINE:
         raise ValueError("Line too long")
+    # Not mandatory but safe - https://tools.ietf.org/html/rfc7230#section-3.5
     if not line.endswith(b'\r\n'):
         raise ValueError("Line without CRLF")
     return line

websockets/protocol.py

@@ -92,11 +92,13 @@ class WebSocketCommonProtocol(asyncio.StreamReaderProtocol):
     processed, the request path is available in the :attr:`path` attribute,
     and the request and response HTTP headers are available:
 
-    * as a MIME :class:`~email.message.Message` in the :attr:`request_headers`
+    * as a :class:`~http.client.HTTPMessage` in the :attr:`request_headers`
       and :attr:`response_headers` attributes
     * as an iterable of (name, value) pairs in the :attr:`raw_request_headers`
       and :attr:`raw_response_headers` attributes
 
+    These attributes must be treated as immutable.
+
     If a subprotocol was negotiated, it's available in the :attr:`subprotocol`
     attribute.
 

websockets/server.py

@@ -6,8 +6,7 @@
 
 import asyncio
 import collections.abc
-import email.message
-import http
+import http.client
 import logging
 
 from .compatibility import asyncio_ensure_future
@@ -155,20 +154,20 @@ def read_http_request(self):
             raise InvalidMessage("Malformed HTTP message") from exc
 
         self.path = path
-        self.request_headers = headers
-        self.raw_request_headers = list(headers.raw_items())
+        self.request_headers = http.client.HTTPMessage()
+        self.request_headers._headers = headers     # HACK
+        self.raw_request_headers = headers
 
-        return path, headers
+        return path, self.request_headers
 
     @asyncio.coroutine
     def write_http_response(self, status, headers):
         """
         Write status line and headers to the HTTP response.
 
         """
-        self.response_headers = email.message.Message()
-        for name, value in headers:
-            self.response_headers[name] = value
+        self.response_headers = http.client.HTTPMessage()
+        self.response_headers._headers = headers    # HACK
         self.raw_response_headers = headers
 
         # Since the status line and headers only contain ASCII characters,

websockets/test_http.py

@@ -2,7 +2,7 @@
 import unittest
 
 from .http import *
-from .http import read_message  # private API
+from .http import read_headers  # private API
 
 
 class HTTPTests(unittest.TestCase):
@@ -32,7 +32,7 @@ def test_read_request(self):
         )
         path, hdrs = self.loop.run_until_complete(read_request(self.stream))
         self.assertEqual(path, '/chat')
-        self.assertEqual(hdrs['Upgrade'], 'websocket')
+        self.assertEqual(dict(hdrs)['Upgrade'], 'websocket')
 
     def test_read_response(self):
         # Example from the protocol overview in RFC 6455
@@ -46,32 +46,54 @@ def test_read_response(self):
         )
         status, hdrs = self.loop.run_until_complete(read_response(self.stream))
         self.assertEqual(status, 101)
-        self.assertEqual(hdrs['Upgrade'], 'websocket')
+        self.assertEqual(dict(hdrs)['Upgrade'], 'websocket')
 
-    def test_method(self):
+    def test_request_method(self):
         self.stream.feed_data(b'OPTIONS * HTTP/1.1\r\n\r\n')
         with self.assertRaises(ValueError):
             self.loop.run_until_complete(read_request(self.stream))
 
-    def test_version(self):
+    def test_request_version(self):
         self.stream.feed_data(b'GET /chat HTTP/1.0\r\n\r\n')
         with self.assertRaises(ValueError):
             self.loop.run_until_complete(read_request(self.stream))
+
+    def test_response_version(self):
         self.stream.feed_data(b'HTTP/1.0 400 Bad Request\r\n\r\n')
         with self.assertRaises(ValueError):
             self.loop.run_until_complete(read_response(self.stream))
 
+    def test_response_status(self):
+        self.stream.feed_data(b'HTTP/1.1 007 My name is Bond\r\n\r\n')
+        with self.assertRaises(ValueError):
+            self.loop.run_until_complete(read_response(self.stream))
+
+    def test_response_reason(self):
+        self.stream.feed_data(b'HTTP/1.1 200 \x7f\r\n\r\n')
+        with self.assertRaises(ValueError):
+            self.loop.run_until_complete(read_response(self.stream))
+
+    def test_header_name(self):
+        self.stream.feed_data(b'foo bar: baz qux\r\n\r\n')
+        with self.assertRaises(ValueError):
+            self.loop.run_until_complete(read_headers(self.stream))
+
+    def test_header_value(self):
+        self.stream.feed_data(b'foo: \x00\x00\x0f\r\n\r\n')
+        with self.assertRaises(ValueError):
+            self.loop.run_until_complete(read_headers(self.stream))
+
     def test_headers_limit(self):
         self.stream.feed_data(b'foo: bar\r\n' * 500 + b'\r\n')
         with self.assertRaises(ValueError):
-            self.loop.run_until_complete(read_message(self.stream))
+            self.loop.run_until_complete(read_headers(self.stream))
 
     def test_line_limit(self):
         self.stream.feed_data(b'a' * 5000 + b'\r\n\r\n')
         with self.assertRaises(ValueError):
-            self.loop.run_until_complete(read_message(self.stream))
+            self.loop.run_until_complete(read_headers(self.stream))
 
     def test_line_ending(self):
-        self.stream.feed_data(b'GET / HTTP/1.1\n\n')
+        self.stream.feed_data(b'foo: bar\n\n')
         with self.assertRaises(ValueError):
-            self.loop.run_until_complete(read_message(self.stream))
+            self.loop.run_until_complete(read_headers(self.stream))