Repository: encode/starlette

Issue:
Add EmptyResponse class
HTTP responses with status codes 1xx, 204, 205, and 304 are special cases.  1xx, 204, 304 responses must not include a message body.  The empty message body for 205 responses can be handled in a few ways, but the only one that would make sense for Starlette would be to add a Content-Length header with value 0.

Currently it is not at all obvious to users how to implement these responses.  Starlette can make it explicit with the addition of an EmptyResponse class that does the right thing for each of the status codes cited above.

The "right thing" differs by status code. The key difference is 

* 1xx, 204: must not have a Content-Length header
* 205: should have Content-Length: 0
* 304: may have a Content-Length header if the value equals the length of the corresponding 200 response that could have been sent.

Thus, an EmptyResponse object should ensure no Content-Length header in the first case, ensure Content-Length: 0 in the second case, and pass along a Content-Length header if supplied in the third case; here it is the caller's responsibility to get the value right.

In all cases, an EmptyResponse object would send an 'http.response.body' event with body: b'', more_body: False so that the ASGI server knows to begin sending the response.

The addition of an EmptyResponse class should resolve at least one open issue (https://github.com/encode/starlette/issues/1099).

Discussion:
TBBle: Looking at this change and prompted by https://github.com/encode/starlette/pull/1395#issuecomment-1006582075, what happens if you still pass a body in, i.e. `Response(status_code=204, content="hi")` in the test?

My skimming of the relevant [ASGI 3 spec](https://asgi.readthedocs.io/en/latest/specs/www.html#response-start-send-event) suggests the ASGI server may add `Transfer-Encoding: chunked`, which then becomes RFC-invalid again, as the presence of either of `Content-Length` or `Transfer-Encoding` signals a body, which is disallowed for 204 and related responses.

Perhaps the implicit expectation is that the ASGI server will use "may" here and recognise response codes for which `Transfer-Encoding` should not be added (and also implicitly drop the body data, perhaps)? Although that seems like a gap in the spec that it's not explicit, and a gap in behaviour that the same thing is not done for `Content-Length`, as the same HTTP spec restrictions apply.

I apologise if I'm asking a question with an obvious/known answer already. I'm new to the ASGI ecosystem, and may be misunderstanding the division of responsibility or otherwise.
tomchristie: > what happens if you still pass a body in, i.e. Response(status_code=204, content="hi") in the test?

Probably you've just got yourself an RFC-invalid response.

We *could* potentially guard against those kinds of cases, and I'd be happy for us to discuss that as a follow-up issue if anyone's sufficiently motivated to do so.

The most important thing for *this* pull request tho, is to make sure we're doing the right thing for the sensible cases.


Patch:
diff --git a/starlette/responses.py b/starlette/responses.py
index da765cf..26d7305 100644
--- a/starlette/responses.py
+++ b/starlette/responses.py
@@ -71,7 +71,11 @@ class Response:
             populate_content_type = b"content-type" not in keys
 
         body = getattr(self, "body", None)
-        if body is not None and populate_content_length:
+        if (
+            body is not None
+            and populate_content_length
+            and not (self.status_code < 200 or self.status_code in (204, 304))
+        ):
             content_length = str(len(body))
             raw_headers.append((b"content-length", content_length.encode("latin-1")))
 


Test patch:
diff --git a/tests/test_responses.py b/tests/test_responses.py
index 150fe47..e2337bd 100644
--- a/tests/test_responses.py
+++ b/tests/test_responses.py
@@ -333,6 +333,13 @@ def test_empty_response(test_client_factory):
     assert response.headers["content-length"] == "0"
 
 
+def test_empty_204_response(test_client_factory):
+    app = Response(status_code=204)
+    client: TestClient = test_client_factory(app)
+    response = client.get("/")
+    assert "content-length" not in response.headers
+
+
 def test_non_empty_response(test_client_factory):
     app = Response(content="hi")
     client: TestClient = test_client_factory(app)