Skip to content

Commit 210368c

Browse files
committed
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`.
1 parent d30ef74 commit 210368c

File tree

2 files changed

+5
-3
lines changed

2 files changed

+5
-3
lines changed

tornado/httputil.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,9 @@ def __init__(
487487
) -> None:
488488
if start_line is not None:
489489
method, uri, version = start_line
490+
assert method
490491
self.method = method
492+
assert uri
491493
self.uri = uri
492494
self.version = version
493495
self.headers = headers or HTTPHeaders()

tornado/test/httputil_test.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -526,15 +526,15 @@ def test_default_constructor(self):
526526
# All parameters are formally optional, but uri is required
527527
# (and has been for some time). This test ensures that no
528528
# more required parameters slip in.
529-
HTTPServerRequest(uri="/")
529+
HTTPServerRequest(method="GET", uri="/")
530530

531531
def test_body_is_a_byte_string(self):
532-
request = HTTPServerRequest(uri="/")
532+
request = HTTPServerRequest(method="GET", uri="/")
533533
self.assertIsInstance(request.body, bytes)
534534

535535
def test_repr_does_not_contain_headers(self):
536536
request = HTTPServerRequest(
537-
uri="/", headers=HTTPHeaders({"Canary": ["Coal Mine"]})
537+
method="GET", uri="/", headers=HTTPHeaders({"Canary": ["Coal Mine"]})
538538
)
539539
self.assertNotIn("Canary", repr(request))
540540

0 commit comments

Comments
 (0)