From 210368ca5ca8c4817437144598235df922ab0cfc Mon Sep 17 00:00:00 2001 From: Armen Michaeli Date: Sun, 9 Nov 2025 12:53:32 +0100 Subject: [PATCH] Make `uri` and `method` attributes on `HTTPServerRequest` mandatory 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 | 2 ++ tornado/test/httputil_test.py | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tornado/httputil.py b/tornado/httputil.py index d3dce21f..a418b9fc 100644 --- a/tornado/httputil.py +++ b/tornado/httputil.py @@ -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() diff --git a/tornado/test/httputil_test.py b/tornado/test/httputil_test.py index fdd5fcab..7a46b911 100644 --- a/tornado/test/httputil_test.py +++ b/tornado/test/httputil_test.py @@ -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)) -- 2.47.3