Round-trip cleanly non-ASCII data in HTTP messages.

Use ISO-8859-1 consistently, except in places that must really be ASCII.

This leads to changes in undocumented behaviors.

When serializing -- this changes in this commit:

- Non-ASCII request paths are rejected instead of encoded with UTF-8.
  This isn't supposed to have any effect because websockets converts
  non-ASCII URIs to ASCII IRIs before they reach the HTTP layer.
- Other non-ASCII values (reason phrase, header values) are now encoded
  as ISO-8859-1 instead of UTF-8 -- which was never intended; there were
  comments saying "only contain ASCII"; it's an implementation accident.
  This has the benefit that it's now possible to send header values in
  any encoding, while previously only valid UTF-8 could be sent.

When parsing -- this changed between 16.0 and 16.1, in ff4869b:

- Non-ASCII request paths are rejected instead of decoded as ASCII with
  surrogate escapes.
- Other non-ASCII values (reason phrase, header values) are now decoded
  as ISO-8859-1 instead of ASCII with surrogate escapes.

Author: aaugustin

docs/project/changelog.rst

@@ -52,6 +52,14 @@ Backwards-incompatible changes
     Previously, the server closed the connection without returning an HTTP
     response. Now, ``process_request`` runs and can return an HTTP response.
 
+.. admonition:: Encoding and decoding non-ASCII headers in handshake requests
+    and responses changed.
+    :class: note
+
+    The previous behavior was undocumented, inconsistent, and didn't match the
+    HTTP specification. If you relied on the encoding being UTF-8 or ASCII with
+    surrogate escapes, depending on the context, you must switch to ISO-8859-1.
+
 .. admonition:: In the :mod:`threading` implementation, the ``socket`` argument
     is renamed to ``sock``.
     :class: note
@@ -76,6 +84,9 @@ New features
 Improvements
 ............
 
+* Supported non-ASCII headers consistently in handshake requests and responses,
+  using ISO-8859-1 encoding.
+
 * Replied with HTTP 405 Method Not Allowed when the handshake request doesn't
   use the GET method, instead of closing the connection.
 

src/websockets/datastructures.py

@@ -25,7 +25,7 @@ def __str__(self) -> str:
         return super().__str__()
 
 
-# Obsolete line folding for header values is not supported.
+# Same regex as http11._value_re, but for matching str rather than bytes.
 is_valid_header_value = re.compile(r"[\x09\x20-\x7e\x80-\xff]*").fullmatch
 
 
@@ -65,6 +65,26 @@ class Headers(MutableMapping[str, str]):
     - :meth:`get_all` returns a list of all values for a header;
     - :meth:`raw_items` returns an iterator of ``(name, values)`` pairs.
 
+    Header names and values are expected to contain only ASCII text. However,
+    non-ASCII values happen in practice, even though there is no standard for
+    transmitting non-ASCII data in HTTP headers. :class:`Headers` supports it
+    by treating it as ISO-8859-1 data. This is a safe and reversible encoding
+    to represent arbitrary data in a :class:`str`.
+
+    When reading headers from the network, if the actual encoding isn't
+    ISO-8859-1, you must re-encode and decode, e.g.::
+
+        value = headers[key].encode("iso-8859-1").decode("utf-8")
+
+    Conversely, when sending headers to the network, if you need to use a
+    different encoding, you can encode and decode, e.g.::
+
+        headers[key] = value.encode("utf-8").decode("iso-8859-1")
+
+    When assigning a value to a header, as a security hardening measure, the
+    value is checked for unsafe characters. The name isn't checked because it's
+    usually a constant in code, unlikely to be tainted by user input.
+
     """
 
     __slots__ = ["_dict", "_list"]
@@ -88,8 +108,9 @@ def copy(self) -> Headers:
         return copy
 
     def serialize(self) -> bytes:
-        # Since headers only contain ASCII characters, we can keep this simple.
-        return str(self).encode()
+        # parse_headers() supports non-ASCII header values. It decodes them as
+        # ISO-8859-1. Encode back in ISO-8859-1 in order to round-trip cleanly.
+        return str(self).encode("iso-8859-1")
 
     # Collection methods
 

src/websockets/http11.py

@@ -86,6 +86,10 @@ class Request:
     """
     WebSocket handshake request.
 
+    ``method`` and ``path`` must contain only ASCII characters. ``headers``
+    should contain only ASCII characters; however, non-ASCII header values are
+    tolerated and encoded as ISO-8859-1.
+
     Attributes:
         path: Request path, including optional query.
         headers: Request headers.
@@ -121,10 +125,10 @@ def parse(
 
         This is a generator-based coroutine.
 
-        The request method, path, and headers are expected to contain only ASCII
-        characters. The request path isn't URL-decoded or validated in any way.
-        Non-ASCII header values are decoded as ISO-8859-1, which has been
-        historically allowed, but is now deprecated.
+        The request method and path must contain only ASCII characters. The
+        request path isn't URL-decoded or validated in any way. Request headers
+        should contain only ASCII characters; however, non-ASCII header values
+        are tolerated and decoded with ISO-8859-1.
 
         :meth:`parse` doesn't read the request body because WebSocket handshake
         requests don't have one. If the request contains a body, it may be read
@@ -188,9 +192,10 @@ def serialize(self) -> bytes:
         Serialize a WebSocket handshake request.
 
         """
-        # Since the request line and headers only contain ASCII characters,
-        # we can keep this simple.
-        request = f"{self.method} {self.path} HTTP/1.1\r\n".encode()
+        # Methods are hardcoded and always ASCII. Non-ASCII paths are converted
+        # from URI to IRI and percent-encoded. Enforce ASCII as a safety net.
+        request_line = f"{self.method} {self.path} HTTP/1.1\r\n"
+        request = request_line.encode("ascii")
         request += self.headers.serialize()
         return request
 
@@ -200,6 +205,10 @@ class Response:
     """
     WebSocket handshake response.
 
+    ``reason_phrase`` and ``headers`` should contain only ASCII characters;
+    however, non-ASCII reason phrases and header values are tolerated and
+    encoded as ISO-8859-1.
+
     Attributes:
         status_code: Response code.
         reason_phrase: Response reason.
@@ -241,9 +250,9 @@ def parse(
 
         This is a generator-based coroutine.
 
-        The reason phrase and header values are expected to contain only ASCII
-        characters. Non-ASCII values are decoded as ISO-8859-1, which has been
-        historically allowed, but is now deprecated.
+        The reason phrase and headers should contain only ASCII characters;
+        however, non-ASCII reason phrases and header values are tolerated and
+        decoded as ISO-8859-1.
 
         Args:
             read_line: Generator-based coroutine that reads a LF-terminated
@@ -319,9 +328,9 @@ def serialize(self) -> bytes:
         Serialize a WebSocket handshake response.
 
         """
-        # Since the status line and headers only contain ASCII characters,
-        # we can keep this simple.
-        response = f"HTTP/1.1 {self.status_code} {self.reason_phrase}\r\n".encode()
+        # Encode the reason phrase as ISO-8859-1 to round-trip cleanly.
+        status_line = f"HTTP/1.1 {self.status_code} {self.reason_phrase}\r\n"
+        response = status_line.encode("iso-8859-1")
         response += self.headers.serialize()
         response += self.body
         return response
@@ -364,9 +373,8 @@ def parse_headers(
     """
     Parse HTTP headers.
 
-    Header values are expected to contain only ASCII characters. Non-ASCII
-    values are decoded as ISO-8859-1, which has been historically allowed,
-    but is now deprecated.
+    Headers should contain only ASCII characters; however, non-ASCII values are
+    tolerated and decoded as ISO-8859-1.
 
     Args:
         read_line: Generator-based coroutine that reads a LF-terminated line
@@ -404,8 +412,10 @@ def parse_headers(
 
         name = raw_name.decode("ascii")  # guaranteed to be ASCII at this point
         # Headers should be ASCII. Section 5.5 of RFC 9110 says: "Historically,
-        # HTTP allowed field content with text in the ISO-8859-1 charset."
-        # It's easy to reverse and cannot crash, making it a decent choice.
+        # HTTP allowed field content with text in the ISO-8859-1 charset." and
+        # "A recipient SHOULD treat other allowed octets in field content (i.e.,
+        # obs-text) as opaque data." ISO-8859-1 is an opaque representation of
+        # arbitrary binary data in a str object and it is easy to reverse.
         value = raw_value.decode("iso-8859-1")
 
         # Since we just validated raw_value, we don't need to revalidate it.

tests/test_datastructures.py

@@ -122,6 +122,14 @@ def test_setitem_case_insensitive(self):
         self.headers["upgrade"] = "websocket"
         self.assertEqual(self.headers["Upgrade"], "websocket")
 
+    def test_setitem_non_ascii_value(self):
+        self.headers["X-Non-ASCII"] = "café"
+        self.assertEqual(self.headers["X-Non-ASCII"], "café")
+
+    def test_setitem_non_latin1_value(self):
+        with self.assertRaises(InvalidHeaderValue):
+            self.headers["X-Invalid"] = "→"
+
     def test_setitem_invalid_value(self):
         with self.assertRaises(InvalidHeaderValue):
             self.headers["X-Invalid"] = "multi\r\nline"
@@ -181,6 +189,14 @@ def test_set_insecure_invalid_value(self):
         self.headers.set_insecure("X-Invalid", "multi\r\nline")
         self.assertEqual(self.headers["X-Invalid"], "multi\r\nline")  # oops
 
+    def test_serialize_non_ascii_value(self):
+        self.headers.clear()
+        self.headers.set_insecure("X-Non-ASCII", "café")
+        self.assertEqual(
+            self.headers.serialize(),
+            b"X-Non-ASCII: caf\xe9\r\n\r\n",
+        )
+
 
 class MultiValueHeadersTests(unittest.TestCase):
     def setUp(self):

tests/test_http11.py

@@ -431,6 +431,13 @@ def test_serialize_with_body(self):
             b"Hello world!\n",
         )
 
+    def test_serialize_iso_8859_1_reason(self):
+        response = Response(200, "ØK", Headers([("Content-Length", "0")]))
+        self.assertEqual(
+            response.serialize(),
+            b"HTTP/1.1 200 \xd8K\r\nContent-Length: 0\r\n\r\n",
+        )
+
 
 class HeadersTests(GeneratorTestCase):
     def setUp(self):