Merge pull request #3552 from bdarnell/http-reason

bdarnell · web-flow · commit ca54b0d61f2a · 2025-12-10T15:54:32.000-05:00
web: Harden against invalid HTTP reason phrases
diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py
@@ -1769,7 +1769,7 @@ class StatusReasonTest(SimpleHandlerTestCase):
     class Handler(RequestHandler):
         def get(self):
             reason = self.request.arguments.get("reason", [])
-            self.set_status(
+            raise HTTPError(
                 int(self.get_argument("code")),
                 reason=to_unicode(reason[0]) if reason else None,
             )
@@ -1792,6 +1792,19 @@ def test_status(self):
         self.assertEqual(response.code, 682)
         self.assertEqual(response.reason, "Unknown")
 
+    def test_header_injection(self):
+        response = self.fetch("/?code=200&reason=OK%0D%0AX-Injection:injected")
+        self.assertEqual(response.code, 200)
+        self.assertEqual(response.reason, "Unknown")
+        self.assertNotIn("X-Injection", response.headers)
+
+    def test_reason_xss(self):
+        response = self.fetch("/?code=400&reason=<script>alert(1)</script>")
+        self.assertEqual(response.code, 400)
+        self.assertEqual(response.reason, "Unknown")
+        self.assertNotIn(b"script", response.body)
+        self.assertIn(b"Unknown", response.body)
+
 
 class DateHeaderTest(SimpleHandlerTestCase):
     class Handler(RequestHandler):
diff --git a/tornado/web.py b/tornado/web.py
@@ -359,8 +359,10 @@ def set_status(self, status_code: int, reason: Optional[str] = None) -> None:
 
         :arg int status_code: Response status code.
         :arg str reason: Human-readable reason phrase describing the status
-            code. If ``None``, it will be filled in from
-            `http.client.responses` or "Unknown".
+            code (for example, the "Not Found" in ``HTTP/1.1 404 Not Found``).
+            Normally determined automatically from `http.client.responses`; this
+            argument should only be used if you need to use a non-standard
+            status code.
 
         .. versionchanged:: 5.0
 
@@ -369,6 +371,14 @@ def set_status(self, status_code: int, reason: Optional[str] = None) -> None:
         """
         self._status_code = status_code
         if reason is not None:
+            if "<" in reason or not httputil._ABNF.reason_phrase.fullmatch(reason):
+                # Logically this would be better as an exception, but this method
+                # is called on error-handling paths that would need some refactoring
+                # to tolerate internal errors cleanly.
+                #
+                # The check for "<" is a defense-in-depth against XSS attacks (we also
+                # escape the reason when rendering error pages).
+                reason = "Unknown"
             self._reason = escape.native_str(reason)
         else:
             self._reason = httputil.responses.get(status_code, "Unknown")
@@ -1345,7 +1355,8 @@ def send_error(self, status_code: int = 500, **kwargs: Any) -> None:
                 reason = exception.reason
         self.set_status(status_code, reason=reason)
         try:
-            self.write_error(status_code, **kwargs)
+            if status_code != 304:
+                self.write_error(status_code, **kwargs)
         except Exception:
             app_log.error("Uncaught exception in write_error", exc_info=True)
         if not self._finished:
@@ -1373,7 +1384,7 @@ def write_error(self, status_code: int, **kwargs: Any) -> None:
             self.finish(
                 "<html><title>%(code)d: %(message)s</title>"
                 "<body>%(code)d: %(message)s</body></html>"
-                % {"code": status_code, "message": self._reason}
+                % {"code": status_code, "message": escape.xhtml_escape(self._reason)}
             )
 
     @property
@@ -2520,9 +2531,11 @@ class HTTPError(Exception):
         mode).  May contain ``%s``-style placeholders, which will be filled
         in with remaining positional parameters.
     :arg str reason: Keyword-only argument.  The HTTP "reason" phrase
-        to pass in the status line along with ``status_code``.  Normally
+        to pass in the status line along with ``status_code`` (for example,
+        the "Not Found" in ``HTTP/1.1 404 Not Found``).  Normally
         determined automatically from ``status_code``, but can be used
-        to use a non-standard numeric code.
+        to use a non-standard numeric code. This is not a general-purpose
+        error message.
     """
 
     def __init__(
