Repository: twisted/twisted

Issue:
Twisted doesn't validate HTTP versions
# Description

Twisted does no validation of received requests' HTTP versions.

# How to cause this behavior

1. Start a Twisted HTTP server that echos the received version, such as [this one](https://github.com/narfindustries/http-garden/blob/main/images/twisted/server.py).
2. Send Twisted an HTTP request with an invalid version, and extract the parsed version from the response:
```sh
(printf 'GET / This-is-totally-invalid!-It-even-contains-unprintable-bytes,-like-\x00-and\xff!!\r\nHost: a\r\n\r\n'; sleep 1) \
    | nc localhost 80 \
    | awk '{RS=""; FS="\r\n\r\n"} {print $2}' \
    | tail -n 1 \
    | jq '.["version"]' \
    | xargs echo \
    | base64 -d \
    | xxd
```
3. Observe that the version is unchanged, including the NUL and non-ASCII byte:
```
00000000: 5468 6973 2d69 732d 746f 7461 6c6c 792d  This-is-totally-
00000010: 696e 7661 6c69 6421 2d49 742d 6576 656e  invalid!-It-even
00000020: 2d63 6f6e 7461 696e 732d 756e 7072 696e  -contains-unprin
00000030: 7461 626c 652d 6279 7465 732c 2d6c 696b  table-bytes,-lik
00000040: 652d 002d 616e 64ff 2121                 e-.-and.!!
```

# The correct behavior
RFC 9112 specifies that HTTP versions have to match the following grammar rule:
```
  HTTP-version  = HTTP-name "/" DIGIT "." DIGIT
  HTTP-name     = %s"HTTP"
```
In other words, HTTP versions need to match the following regex: `HTTP/[0-9]\.[0-9]`.

**Testing environment**
 - Operating System and Version; paste the output of these commands:
   - `Linux e153e0bbe66c 6.9.7-arch1-1 #1 SMP PREEMPT_DYNAMIC Fri, 28 Jun 2024 04:32:50 +0000 x86_64 GNU/Linux`
 - Twisted version [e.g. 22.2.0]
   - `24.3.0.post0`

Discussion:

Patch:
diff --git a/src/twisted/web/http.py b/src/twisted/web/http.py
index b08ee9121c..a65fb37b09 100644
--- a/src/twisted/web/http.py
+++ b/src/twisted/web/http.py
@@ -236,6 +236,58 @@ weekdayname_lower = [name.lower() for name in weekdayname]
 monthname_lower = [name and name.lower() for name in monthname]
 
 
+def _parseRequestLine(line: bytes) -> tuple[bytes, bytes, bytes]:
+    """
+    Parse an HTTP request line, which looks like:
+
+        GET /foo/bar HTTP/1.1
+
+    This function attempts to validate the well-formedness of
+    the line. RFC 9112 section 3 provides this ABNF:
+
+        request-line   = method SP request-target SP HTTP-version
+
+    We allow any method that is a valid token:
+
+        method         = token
+        token          = 1*tchar
+        tchar          = "!" / "#" / "$" / "%" / "&" / "'" / "*"
+                        / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~"
+                        / DIGIT / ALPHA
+
+    We allow any non-empty request-target that contains only printable
+    ASCII characters (no whitespace).
+
+    The RFC defines HTTP-version like this:
+
+        HTTP-version  = HTTP-name "/" DIGIT "." DIGIT
+        HTTP-name     = %s"HTTP"
+
+    However, this function is more strict than the RFC: we only allow
+    HTTP versions of 1.0 and 1.1, as later versions of HTTP don't use
+    a request line.
+
+    @returns: C{(method, request, version)} three-tuple
+
+    @raises: L{ValueError} when malformed
+    """
+    method, request, version = line.split(b" ")
+
+    if not _istoken(method):
+        raise ValueError("Invalid method")
+
+    for c in request:
+        if c <= 32 or c > 176:
+            raise ValueError("Invalid request-target")
+    if request == b"":
+        raise ValueError("Empty request-target")
+
+    if version != b"HTTP/1.1" and version != b"HTTP/1.0":
+        raise ValueError("Invalid version")
+
+    return method, request, version
+
+
 def _parseContentType(line: bytes) -> bytes:
     """
     Parse the Content-Type header.
@@ -455,6 +507,20 @@ def toChunk(data):
     return (networkString(f"{len(data):x}"), b"\r\n", data, b"\r\n")
 
 
+def _istoken(b: bytes) -> bool:
+    """
+    Is the string a token per RFC 9110 section 5.6.2?
+    """
+    for c in b:
+        if c not in (
+            b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"  # ALPHA
+            b"0123456789"  # DIGIT
+            b"!#$%^'*+-.^_`|~"
+        ):
+            return False
+    return b != b""
+
+
 def _ishexdigits(b: bytes) -> bool:
     """
     Is the string case-insensitively hexidecimal?
@@ -2320,14 +2386,9 @@ class HTTPChannel(basic.LineReceiver, policies.TimeoutMixin):
 
             self.__first_line = 0
 
-            parts = line.split()
-            if len(parts) != 3:
-                self._respondToBadRequestAndDisconnect()
-                return
-            command, request, version = parts
             try:
-                command.decode("ascii")
-            except UnicodeDecodeError:
+                command, request, version = _parseRequestLine(line)
+            except ValueError:
                 self._respondToBadRequestAndDisconnect()
                 return
 
@@ -2428,7 +2489,8 @@ class HTTPChannel(basic.LineReceiver, policies.TimeoutMixin):
             self._respondToBadRequestAndDisconnect()
             return False
 
-        if not header or header[-1:].isspace():
+        # Header names must be tokens, per RFC 9110 section 5.1.
+        if not _istoken(header):
             self._respondToBadRequestAndDisconnect()
             return False
 
diff --git a/src/twisted/web/newsfragments/12233.bugfix b/src/twisted/web/newsfragments/12233.bugfix
new file mode 100644
index 0000000000..bfec79b031
--- /dev/null
+++ b/src/twisted/web/newsfragments/12233.bugfix
@@ -0,0 +1,1 @@
+The HTTP 1.0/1.1 server provided by twisted.web is now more picky about the first line of a request, improving compliance with RFC 9112.
diff --git a/src/twisted/web/newsfragments/12235.bugfix b/src/twisted/web/newsfragments/12235.bugfix
new file mode 100644
index 0000000000..45b83655c5
--- /dev/null
+++ b/src/twisted/web/newsfragments/12235.bugfix
@@ -0,0 +1,1 @@
+The HTTP 1.0/1.1 server provided by twisted.web now constains the characters set of HTTP header names, improving compliance with RFC 9110.


Test patch:
diff --git a/src/twisted/web/test/test_http.py b/src/twisted/web/test/test_http.py
index b88d7c474b..fa7f3dfa1b 100644
--- a/src/twisted/web/test/test_http.py
+++ b/src/twisted/web/test/test_http.py
@@ -1727,7 +1727,7 @@ class ParsingTests(unittest.TestCase):
         self.assertTrue(channel.transport.disconnecting)
         self.assertEqual(processed, [])
 
-    def test_invalidNonAsciiMethod(self):
+    def test_invalidMethodNonAscii(self):
         """
         When client sends invalid HTTP method containing
         non-ascii characters HTTP 400 'Bad Request' status will be returned.
@@ -1745,6 +1745,114 @@ class ParsingTests(unittest.TestCase):
         self.assertTrue(channel.transport.disconnecting)
         self.assertEqual(processed, [])
 
+    def test_invalidRequestLineExtraSpaces(self):
+        """
+        The three components of the request line must not be
+        separated by anything other than a single SP character,
+        or a 400 status results.
+        """
+        for requestLine in [
+            b"GET  / HTTP/1.0",
+            b"GET /  HTTP/1.0",
+            b"GET\t/ HTTP/1.0",
+            b"GET /\vHTTP/1.1",
+            b"GET / HTTP/1.1 ",
+            b" GET / HTTP/1.1",
+        ]:
+            self.assertRequestRejected(
+                [requestLine, b"Content-Length: 0", b"Host: foo.example", b"", b""]
+            )
+
+    def test_invalidMethodEmpty(self):
+        """
+        A request with an empty method field is rejected with a
+        400 status code.
+        """
+        self.assertRequestRejected(
+            [
+                b" /foo HTTP/1.1",
+                b"Content-Length: 0",
+                b"Host: foo.example",
+                b"",
+                b"",
+            ]
+        )
+
+    def test_invalidMethodNUL(self):
+        """
+        A request with a method that contains a NUL character
+        is rejected with a 400 status code.
+        """
+        self.assertRequestRejected(
+            [
+                b"GET\0 /foo HTTP/1.1",
+                b"Content-Length: 0",
+                b"Host: foo.example",
+                b"",
+                b"",
+            ]
+        )
+
+    def test_invalidVersion(self):
+        """
+        A request with an invalid HTTP version number is rejected
+        with a 400 status code.
+        """
+        self.assertRequestRejected(
+            [
+                b"HEAD /foo HTTP/1.2",
+                b"Content-Length: 0",
+                b"Host: foo.example",
+                b"",
+                b"",
+            ]
+        )
+
+    def test_invalidRequestTargetEmpty(self):
+        """
+        A request with an empty request-target (URI) is rejected with
+        a 400 status code.
+        """
+        self.assertRequestRejected(
+            [
+                b"POST  HTTP/1.1",
+                b"Content-Length: 0",
+                b"Host: foo.example",
+                b"",
+                b"",
+            ]
+        )
+
+    def test_invalidRequestTargetNUL(self):
+        """
+        A request with an empty request-target (URI) is rejected with
+        a 400 status code.
+        """
+        self.assertRequestRejected(
+            [
+                b"POST /foo\0 HTTP/1.1",
+                b"Content-Length: 0",
+                b"Host: foo.example",
+                b"",
+                b"",
+            ]
+        )
+
+    def test_invalidRequestTargetWhitespace(self):
+        """
+        A request with a request-target (URI) that contains whitespace
+        is rejected with a 400 status code.
+        """
+        self.assertRequestRejected(
+            [
+                b"POST /foo\t/bar HTTP/1.1",
+                b"Content-Length: 0",
+                b"Host: foo.example",
+                b"",
+                b"",
+            ]
+        )
+
     def test_basicAuth(self):
         """
         L{HTTPChannel} provides username and password information supplied in
@@ -1945,8 +2053,8 @@ class ParsingTests(unittest.TestCase):
     def test_invalidHeaderOnlyColon(self):
         """
         C{HTTPChannel} rejects a request with an empty header name (i.e.
-        nothing before the colon).  It produces a 400 (Bad Request) response is
-        generated and closes the connection.
+        nothing before the colon).  It produces a 400 (Bad Request) response
+        and closes the connection.
         """
         self.assertRequestRejected(
             [
@@ -1973,6 +2081,21 @@ class ParsingTests(unittest.TestCase):
             ]
         )
 
+    def test_invalidHeaderChars(self):
+        """
+        A request with a header that contains invalid characters
+        is rejected with a 400 status code.
+        """
+        for header in [
+            b"foo\x00bar: baz",  # NUL byte
+            b"foo\x1bbar: baz",  # ESC byte
+            b"Foo\vBar: baz",  # exotic whitespace
+            b"foo\xe2\x80\xbdbar: baz",  # non-ASCII bytes
+        ]:
+            self.assertRequestRejected(
+                [b"GET / HTTP/1.1", b"Host: foo.example", header, b"", b""]
+            )
+
     def test_headerLimitPerRequest(self):
         """
         C{HTTPChannel} enforces the limit of C{HTTPChannel.maxHeaders} per