]> git.ipfire.org Git - thirdparty/tornado.git/commitdiff
Make `uri` and `method` attributes on `HTTPServerRequest` mandatory 3542/head
authorArmen Michaeli <armen@armen.michaeli.name>
Sun, 9 Nov 2025 11:53:32 +0000 (12:53 +0100)
committerArmen Michaeli <armen@armen.michaeli.name>
Sun, 9 Nov 2025 12:07:39 +0000 (13:07 +0100)
This adds assertion checks during initialisation of `HTTPServerRequest` object, to ensure the corresponding attributes of the object being created, won't end up being `None`. To be clear, it is still permitted to provide `None` or omit specification for both `uri` and `method` parameters to the constructor if the `start_line` parameter specifies [good] values instead.

This is motivated by the idea that per HTTP, requests _always_ feature a method and a URI, and so the model implemented with `HTTPServerRequest` is amended accordingly. Beyond the seemingly idealistic motivation, this helps with _typed_ Tornado applications using e.g. `self.request.uri` as part of request handling -- i.e. in code with `self` being a `RequestHandler` -- to safely assume e.g. `uri` or `method` are `str` and not `str | None` / `Optional[str]` which would require ad-hoc assertions ala `assert self.request.uri is not None` (or `assert self.request.uri`) in _application code_, which IMO is a case of "surprising the user" -- as everyone would expect a HTTP request to have an URI and method be clearly defined as e.g. strings -- certainly excluding the `None` value.

Again, because semantics of `start_line` are preserved, the initialisation of the object _may_ omit parameters `uri` and/or `method` if `start_line` specifies valid values for these instead. In any case, it is the _attributes_ of the object being constructed, that end up being effectively validated with `assert` -- which make the type checker (tested with MyPy 1.18.2 here) assume `str` instead of `str | None`.

tornado/httputil.py
tornado/test/httputil_test.py

index d3dce21f5cb75620e979ccadb889f5e6fa89a853..a418b9fce5bf2d11f8e3cc229bb4ee4ea420ba98 100644 (file)
@@ -487,7 +487,9 @@ class HTTPServerRequest:
     ) -> None:
         if start_line is not None:
             method, uri, version = start_line
+        assert method
         self.method = method
+        assert uri
         self.uri = uri
         self.version = version
         self.headers = headers or HTTPHeaders()
index fdd5fcabf9efd9c99b558e965712d4a8d65ed15f..7a46b911569853adc3fdf6917107e0a37858ee49 100644 (file)
@@ -526,15 +526,15 @@ class HTTPServerRequestTest(unittest.TestCase):
         # All parameters are formally optional, but uri is required
         # (and has been for some time).  This test ensures that no
         # more required parameters slip in.
-        HTTPServerRequest(uri="/")
+        HTTPServerRequest(method="GET", uri="/")
 
     def test_body_is_a_byte_string(self):
-        request = HTTPServerRequest(uri="/")
+        request = HTTPServerRequest(method="GET", uri="/")
         self.assertIsInstance(request.body, bytes)
 
     def test_repr_does_not_contain_headers(self):
         request = HTTPServerRequest(
-            uri="/", headers=HTTPHeaders({"Canary": ["Coal Mine"]})
+            method="GET", uri="/", headers=HTTPHeaders({"Canary": ["Coal Mine"]})
         )
         self.assertNotIn("Canary", repr(request))