web: do not send content on HTTPError(204)

Change `write_error()` to not send body if status code in (204, 304) or (100 <= status code < 200) - refactored into `RequestHandler._should_not_send_content(status_code)` Change `get_arguments()` to raise `ValueError` if `strip` is not boolean - as opposed to `AssertionError`. Fixes tornadoweb#3360
pawciobiel · Feb 16, 2024 · f627e91 · f627e91
1 parent 65a9e48
commit f627e91
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 5 deletions.
diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py
@@ -1687,6 +1687,18 @@ def test_204_headers(self):
         self.assertNotIn("Transfer-Encoding", response.headers)
 
 
+class Header204viaHTTPErrorTest(SimpleHandlerTestCase):
+    class Handler(RequestHandler):
+        def get(self):
+            raise HTTPError(204)
+
+    def test_204_headers_via_httperror(self):
+        response = self.fetch("/")
+        self.assertEqual(response.code, 204)
+        self.assertNotIn("Content-Length", response.headers)
+        self.assertNotIn("Transfer-Encoding", response.headers)
+
+
 class Header304Test(SimpleHandlerTestCase):
     class Handler(RequestHandler):
         def get(self):

diff --git a/tornado/web.py b/tornado/web.py
@@ -466,7 +466,8 @@ def get_arguments(self, name: str, strip: bool = True) -> List[str]:
         # Make sure `get_arguments` isn't accidentally being called with a
         # positional argument that's assumed to be a default (like in
         # `get_argument`.)
-        assert isinstance(strip, bool)
+        if not isinstance(strip, bool):
+            raise ValueError("`strip` must be boolean")
 
         return self._get_arguments(name, self.request.arguments, strip)
 
@@ -1186,6 +1187,10 @@ def flush(self, include_footers: bool = False) -> "Future[None]":
                 future.set_result(None)
                 return future
 
+    def _should_not_send_content(self, status_code: int) -> bool:
+        """Check if we should not send body content for given `status_code`"""
+        return status_code in (204, 304) or (100 <= status_code < 200)
+
     def finish(self, chunk: Optional[Union[str, bytes, dict]] = None) -> "Future[None]":
         """Finishes this response, ending the HTTP request.
 
@@ -1219,10 +1224,9 @@ def finish(self, chunk: Optional[Union[str, bytes, dict]] = None) -> "Future[Non
                 if self.check_etag_header():
                     self._write_buffer = []
                     self.set_status(304)
-            if self._status_code in (204, 304) or (100 <= self._status_code < 200):
-                assert not self._write_buffer, (
-                    "Cannot send body with %s" % self._status_code
-                )
+            if self._should_not_send_content(self._status_code):
+                if self._write_buffer:
+                    raise RuntimeError(f"Cannot send body with status code HTTP{self._status_code}")
                 self._clear_representation_headers()
             elif "Content-Length" not in self._headers:
                 content_length = sum(len(part) for part in self._write_buffer)
@@ -1319,6 +1323,8 @@ def write_error(self, status_code: int, **kwargs: Any) -> None:
             for line in traceback.format_exception(*kwargs["exc_info"]):
                 self.write(line)
             self.finish()
+        elif self._should_not_send_content(status_code):
+            self.finish()
         else:
             self.finish(
                 "<html><title>%(code)d: %(message)s</title>"