Merge branch 'master' into free-threading

Author: lysnikolaou

.github/zizmor.yml

@@ -0,0 +1,14 @@
+rules:
+  unpinned-uses:
+    config:
+      policies:
+        # Allow trusted repositories to use ref-pinning instead of hash-pinning.
+        #
+        # Defaults, from 
+        # https://github.com/woodruffw/zizmor/blob/7b4e76e94be2f4d7b455664ba5252b2b4458b91d/src/audit/unpinned_uses.rs#L172-L193
+        actions/*: ref-pin
+        github/*: ref-pin
+        dependabot/*: ref-pin
+        # Additional trusted repositories
+        pypa/*: ref-pin
+        astral-sh/setup-uv: ref-pin

docs/guide/templates.rst

@@ -118,8 +118,25 @@ Consequently, if you write random stuff inside of your template
 expressions, you will get random Python errors when you execute the
 template.
 
-All template output is escaped by default, using the
-`tornado.escape.xhtml_escape` function. This behavior can be changed
+Security
+~~~~~~~~
+
+Inserting untrusted content into a web page can lead to security vulnerabilities such as cross-site
+scripting (XSS). All data that is passed to a template should be *escaped* to prevent these
+vulnerabilities. The correct form of escaping is context-dependent; Tornado's templates are not
+aware of the syntax of HTML, JavaScript, etc, and so the template developer must sometimes
+explicitly apply the correct escaping function.
+
+The default escaping function is `tornado.escape.xhtml_escape`, which is appropriate for HTML body
+content (but not attribute values). In other cases, other functions should be used. In JavaScript,
+use the `.json_encode` function, e.g. ``<script>var x = {{ json_encode(some_object) }};</script>``.
+`.json_encode` can be used to escape strings, numbers, lists, and dicts. In this example, the
+JavaScript variable ``x`` will be the corresponding JavaScript type (string, number, array, or
+object), and not the JSON-encoded string representation. Note that it is unsafe to use
+`.json_encode` in the context of a JavaScript string literal (including template strings), only in
+the top-level syntactic context.
+
+The automatic escaping behavior can be disabled
 globally by passing ``autoescape=None`` to the `.Application` or
 `.tornado.template.Loader` constructors, for a template file with the
 ``{% autoescape None %}`` directive, or for a single expression by

tornado/httputil.py

@@ -81,6 +81,19 @@ class _ABNF:
     cannot be alphabetized as they are in the RFCs because of dependencies.
     """
 
+    # RFC 3986 (URI)
+    # The URI hostname ABNF is both complex (including detailed vaildation of IPv4 and IPv6
+    # literals) and not strict enough (a lot of punctuation is allowed by the ABNF even though
+    # it is not allowed by DNS). We simplify it by allowing square brackets and colons in any
+    # position, not only for their use in IPv6 literals.
+    uri_unreserved = re.compile(r"[A-Za-z0-9\-._~]")
+    uri_sub_delims = re.compile(r"[!$&'()*+,;=]")
+    uri_pct_encoded = re.compile(r"%[0-9A-Fa-f]{2}")
+    uri_host = re.compile(
+        rf"(?:[\[\]:]|{uri_unreserved.pattern}|{uri_sub_delims.pattern}|{uri_pct_encoded.pattern})*"
+    )
+    uri_port = re.compile(r"[0-9]*")
+
     # RFC 5234 (ABNF)
     VCHAR = re.compile(r"[\x21-\x7E]")
 
@@ -91,6 +104,7 @@ class _ABNF:
     token = re.compile(rf"{tchar.pattern}+")
     field_name = token
     method = token
+    host = re.compile(rf"(?:{uri_host.pattern})(?::{uri_port.pattern})?")
 
     # RFC 9112 (HTTP/1.1)
     HTTP_version = re.compile(r"HTTP/[0-9]\.[0-9]")
@@ -207,18 +221,39 @@ def get_all(self) -> Iterable[Tuple[str, str]]:
                 yield (name, value)
 
     def parse_line(self, line: str) -> None:
-        """Updates the dictionary with a single header line.
+        r"""Updates the dictionary with a single header line.
 
         >>> h = HTTPHeaders()
         >>> h.parse_line("Content-Type: text/html")
         >>> h.get('content-type')
         'text/html'
+        >>> h.parse_line("Content-Length: 42\r\n")
+        >>> h.get('content-type')
+        'text/html'
+
+        .. versionchanged:: 6.5
+            Now supports lines with or without the trailing CRLF, making it possible
+            to pass lines from AsyncHTTPClient's header_callback directly to this method.
+
+        .. deprecated:: 6.5
+           In Tornado 7.0, certain deprecated features of HTTP will become errors.
+           Specifically, line folding and the use of LF (with CR) as a line separator
+           will be removed.
         """
-        if line[0].isspace():
+        if m := re.search(r"\r?\n$", line):
+            # RFC 9112 section 2.2: a recipient MAY recognize a single LF as a line
+            # terminator and ignore any preceding CR.
+            # TODO(7.0): Remove this support for LF-only line endings.
+            line = line[: m.start()]
+        if not line:
+            # Empty line, or the final CRLF of a header block.
+            return
+        if line[0] in HTTP_WHITESPACE:
             # continuation of a multi-line header
+            # TODO(7.0): Remove support for line folding.
             if self._last_key is None:
                 raise HTTPInputError("first header line cannot start with whitespace")
-            new_part = " " + line.lstrip(HTTP_WHITESPACE)
+            new_part = " " + line.strip(HTTP_WHITESPACE)
             self._as_list[self._last_key][-1] += new_part
             self._dict[self._last_key] += new_part
         else:
@@ -243,13 +278,16 @@ def parse(cls, headers: str) -> "HTTPHeaders":
 
         """
         h = cls()
-        # RFC 7230 section 3.5: a recipient MAY recognize a single LF as a line
-        # terminator and ignore any preceding CR.
-        for line in headers.split("\n"):
-            if line.endswith("\r"):
-                line = line[:-1]
-            if line:
-                h.parse_line(line)
+
+        start = 0
+        while True:
+            lf = headers.find("\n", start)
+            if lf == -1:
+                h.parse_line(headers[start:])
+                break
+            line = headers[start : lf + 1]
+            start = lf + 1
+            h.parse_line(line)
         return h
 
     # MutableMapping abstract method implementations.
@@ -397,7 +435,7 @@ def __init__(
         version: str = "HTTP/1.0",
         headers: Optional[HTTPHeaders] = None,
         body: Optional[bytes] = None,
-        host: Optional[str] = None,
+        # host: Optional[str] = None,
         files: Optional[Dict[str, List["HTTPFile"]]] = None,
         connection: Optional["HTTPConnection"] = None,
         start_line: Optional["RequestStartLine"] = None,
@@ -416,7 +454,35 @@ def __init__(
         self.remote_ip = getattr(context, "remote_ip", None)
         self.protocol = getattr(context, "protocol", "http")
 
-        self.host = host or self.headers.get("Host") or "127.0.0.1"
+        try:
+            self.host = self.headers["Host"]
+        except KeyError:
+            if version == "HTTP/1.0":
+                # HTTP/1.0 does not require the Host header.
+                self.host = "127.0.0.1"
+            else:
+                raise HTTPInputError("Missing Host header")
+        if not _ABNF.host.fullmatch(self.host):
+            print(_ABNF.host.pattern)
+            raise HTTPInputError("Invalid Host header: %r" % self.host)
+        if "," in self.host:
+            # https://www.rfc-editor.org/rfc/rfc9112.html#name-request-target
+            # Server MUST respond with 400 Bad Request if multiple
+            # Host headers are present.
+            #
+            # We test for the presence of a comma instead of the number of
+            # headers received because a proxy may have converted
+            # multiple headers into a single comma-separated value
+            # (per RFC 9110 section 5.3).
+            #
+            # This is technically a departure from the RFC since the ABNF
+            # does not forbid commas in the host header. However, since
+            # commas are not allowed in DNS names, it is appropriate to
+            # disallow them. (The same argument could be made for other special
+            # characters, but commas are the most problematic since they could
+            # be used to exploit differences between proxies when multiple headers
+            # are supplied).
+            raise HTTPInputError("Multiple host headers not allowed: %r" % self.host)
         self.host_name = split_host_and_port(self.host.lower())[0]
         self.files = files or {}
         self.connection = connection

tornado/test/httpclient_test.py

@@ -482,6 +482,23 @@ def streaming_callback(chunk):
         self.assertRegex(first_line[0], "HTTP/[0-9]\\.[0-9] 200.*\r\n")
         self.assertEqual(chunks, [b"asdf", b"qwer"])
 
+    def test_header_callback_to_parse_line(self):
+        # Make a request with header_callback and feed the headers to HTTPHeaders.parse_line.
+        # (Instead of HTTPHeaders.parse which is used in normal cases). Ensure that the resulting
+        # headers are as expected, and in particular do not have trailing whitespace added
+        # due to the final CRLF line.
+        headers = HTTPHeaders()
+
+        def header_callback(line):
+            if line.startswith("HTTP/"):
+                # Ignore the first status line
+                return
+            headers.parse_line(line)
+
+        self.fetch("/hello", header_callback=header_callback)
+        for k, v in headers.get_all():
+            self.assertTrue(v == v.strip(), (k, v))
+
     @gen_test
     def test_configure_defaults(self):
         defaults = dict(user_agent="TestDefaultUserAgent", allow_ipv6=False)

tornado/test/httpserver_test.py

@@ -267,6 +267,7 @@ def test_100_continue(self):
             b"\r\n".join(
                 [
                     b"POST /hello HTTP/1.1",
+                    b"Host: 127.0.0.1",
                     b"Content-Length: 1024",
                     b"Expect: 100-continue",
                     b"Connection: close",
@@ -467,6 +468,7 @@ def test_chunked_request_body(self):
         self.stream.write(
             b"""\
 POST /echo HTTP/1.1
+Host: 127.0.0.1
 Transfer-Encoding: chunked
 Content-Type: application/x-www-form-urlencoded
 
@@ -491,6 +493,7 @@ def test_chunked_request_uppercase(self):
         self.stream.write(
             b"""\
 POST /echo HTTP/1.1
+Host: 127.0.0.1
 Transfer-Encoding: Chunked
 Content-Type: application/x-www-form-urlencoded
 
@@ -515,6 +518,7 @@ def test_chunked_request_body_invalid_size(self):
         self.stream.write(
             b"""\
 POST /echo HTTP/1.1
+Host: 127.0.0.1
 Transfer-Encoding: chunked
 
 1_a
@@ -537,6 +541,7 @@ def test_chunked_request_body_duplicate_header(self):
         self.stream.write(
             b"""\
 POST /echo HTTP/1.1
+Host: 127.0.0.1
 Transfer-Encoding: chunked
 Transfer-encoding: chunked
 
@@ -561,6 +566,7 @@ def test_chunked_request_body_unsupported_transfer_encoding(self):
         self.stream.write(
             b"""\
 POST /echo HTTP/1.1
+Host: 127.0.0.1
 Transfer-Encoding: gzip, chunked
 
 2
@@ -582,6 +588,7 @@ def test_chunked_request_body_transfer_encoding_and_content_length(self):
         self.stream.write(
             b"""\
 POST /echo HTTP/1.1
+Host: 127.0.0.1
 Transfer-Encoding: chunked
 Content-Length: 2
 
@@ -624,6 +631,7 @@ def test_invalid_content_length(self):
                             textwrap.dedent(
                                 f"""\
                             POST /echo HTTP/1.1
+                            Host: 127.0.0.1
                             Content-Length: {value}
                             Connection: close
 
@@ -659,7 +667,7 @@ def noop_context():
                 expect_log,
             ):
                 yield stream.connect(("127.0.0.1", self.get_http_port()))
-                stream.write(utf8(f"{method} /echo HTTP/1.1\r\n\r\n"))
+                stream.write(utf8(f"{method} /echo HTTP/1.1\r\nHost:127.0.0.1\r\n\r\n"))
                 resp = yield stream.read_until(b"\r\n\r\n")
                 self.assertTrue(
                     resp.startswith(b"HTTP/1.1 %d" % code),
@@ -968,16 +976,18 @@ def close(self):
     @gen_test
     def test_two_requests(self):
         yield self.connect()
-        self.stream.write(b"GET / HTTP/1.1\r\n\r\n")
+        self.stream.write(b"GET / HTTP/1.1\r\nHost:127.0.0.1\r\n\r\n")
         yield self.read_response()
-        self.stream.write(b"GET / HTTP/1.1\r\n\r\n")
+        self.stream.write(b"GET / HTTP/1.1\r\nHost:127.0.0.1\r\n\r\n")
         yield self.read_response()
         self.close()
 
     @gen_test
     def test_request_close(self):
         yield self.connect()
-        self.stream.write(b"GET / HTTP/1.1\r\nConnection: close\r\n\r\n")
+        self.stream.write(
+            b"GET / HTTP/1.1\r\nHost:127.0.0.1\r\nConnection: close\r\n\r\n"
+        )
         yield self.read_response()
         data = yield self.stream.read_until_close()
         self.assertTrue(not data)
@@ -1023,31 +1033,35 @@ def test_http10_keepalive_extra_crlf(self):
     @gen_test
     def test_pipelined_requests(self):
         yield self.connect()
-        self.stream.write(b"GET / HTTP/1.1\r\n\r\nGET / HTTP/1.1\r\n\r\n")
+        self.stream.write(
+            b"GET / HTTP/1.1\r\nHost:127.0.0.1\r\n\r\nGET / HTTP/1.1\r\nHost:127.0.0.1\r\n\r\n"
+        )
         yield self.read_response()
         yield self.read_response()
         self.close()
 
     @gen_test
     def test_pipelined_cancel(self):
         yield self.connect()
-        self.stream.write(b"GET / HTTP/1.1\r\n\r\nGET / HTTP/1.1\r\n\r\n")
+        self.stream.write(
+            b"GET / HTTP/1.1\r\nHost:127.0.0.1\r\n\r\nGET / HTTP/1.1\r\nHost:127.0.0.1\r\n\r\n"
+        )
         # only read once
         yield self.read_response()
         self.close()
 
     @gen_test
     def test_cancel_during_download(self):
         yield self.connect()
-        self.stream.write(b"GET /large HTTP/1.1\r\n\r\n")
+        self.stream.write(b"GET /large HTTP/1.1\r\nHost:127.0.0.1\r\n\r\n")
         yield self.read_headers()
         yield self.stream.read_bytes(1024)
         self.close()
 
     @gen_test
     def test_finish_while_closed(self):
         yield self.connect()
-        self.stream.write(b"GET /finish_on_close HTTP/1.1\r\n\r\n")
+        self.stream.write(b"GET /finish_on_close HTTP/1.1\r\nHost:127.0.0.1\r\n\r\n")
         yield self.read_headers()
         self.close()
         # Let the hanging coroutine clean up after itself
@@ -1075,10 +1089,10 @@ def test_keepalive_chunked(self):
     @gen_test
     def test_keepalive_chunked_head_no_body(self):
         yield self.connect()
-        self.stream.write(b"HEAD /chunked HTTP/1.1\r\n\r\n")
+        self.stream.write(b"HEAD /chunked HTTP/1.1\r\nHost:127.0.0.1\r\n\r\n")
         yield self.read_headers()
 
-        self.stream.write(b"HEAD /chunked HTTP/1.1\r\n\r\n")
+        self.stream.write(b"HEAD /chunked HTTP/1.1\r\nHost:127.0.0.1\r\n\r\n")
         yield self.read_headers()
         self.close()
 
@@ -1337,7 +1351,7 @@ def test_idle_after_use(self):
 
         # Use the connection twice to make sure keep-alives are working
         for i in range(2):
-            stream.write(b"GET / HTTP/1.1\r\n\r\n")
+            stream.write(b"GET / HTTP/1.1\r\nHost: 127.0.0.1\r\n\r\n")
             yield stream.read_until(b"\r\n\r\n")
             data = yield stream.read_bytes(11)
             self.assertEqual(data, b"Hello world")
@@ -1459,14 +1473,17 @@ def test_body_size_override_reset(self):
             # Use a raw stream so we can make sure it's all on one connection.
             stream.write(
                 b"PUT /streaming?expected_size=10240 HTTP/1.1\r\n"
+                b"Host: 127.0.0.1\r\n"
                 b"Content-Length: 10240\r\n\r\n"
             )
             stream.write(b"a" * 10240)
             start_line, headers, response = yield read_stream_body(stream)
             self.assertEqual(response, b"10240")
             # Without the ?expected_size parameter, we get the old default value
             stream.write(
-                b"PUT /streaming HTTP/1.1\r\n" b"Content-Length: 10240\r\n\r\n"
+                b"PUT /streaming HTTP/1.1\r\n"
+                b"Host: 127.0.0.1\r\n"
+                b"Content-Length: 10240\r\n\r\n"
             )
             with ExpectLog(gen_log, ".*Content-Length too long", level=logging.INFO):
                 data = yield stream.read_until_close()