[Django] #33230: Test client doesn't set explicitly provided Content-Type when data is empty
Django
---------------------------------------------+------------------------
Reporter: Markus Holtermann | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 3.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------------+------------------------
When using Django's test client and setting an explicit Content-Type but
have an empty data, the provided content type isn't available in the
request in the view:
{{{#!python
from django.views.decorators.csrf import csrf_exempt
from django.http.response import HttpResponse
@csrf_exempt
def test_view(request):
return HttpResponse(repr(request.headers))
class MyTest(SimpleTestCase):
def test_content_type(self):
resp = self.client.post("/", data=b"", content_type="application
/octet-stream")
print(resp.content)
# b"{'Cookie': ''}"
}}}
Frankly, I'm not sure if that's the intended behavior. Can one set a
Content-Type for a request without payload? The Python Requests library
allows for it and sets the header.
The code in question that only conditionally sets the Content-Type is
https://github.com/django/django/blob/afeafd6036616bac8263d762c1610f22241c0187/django/test/client.py#L461-L466
--
Ticket URL: <https://code.djangoproject.com/ticket/33230>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Markus Holtermann | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 3.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
[https://www.rfc-editor.org/rfc/rfc7231#section-3.1.1.5 RFC 7231 3.1.1.5
"Content-Type"]:
> ...A sender that generates a message containing a payload body SHOULD
> generate a Content-Type header...
Can't see that it says explicitly (anything) for the empty case.
(So don't know what to infer 🤔)
--
Ticket URL: <https://code.djangoproject.com/ticket/33230#comment:1>
Django
Reporter: Markus Holtermann | Owner: nobody
Cleanup/optimization |
Component: Testing framework | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* cc: Aymeric Augustin (added)
* type: Uncategorized => Cleanup/optimization
* component: Uncategorized => Testing framework
Comment:
As far as I'm aware it was intentionally changed in
e73838b6ddcc7b37c03f9eee04fa6e6a283fedb3 (see #17371). It's also
documented for `options()`, `delete()`, and `put()`:
> ''When `data` is provided, it is used as the request body, and a
`Content-Type` header is set to `content_type`.''
--
Ticket URL: <https://code.djangoproject.com/ticket/33230#comment:2>
Django
Reporter: Markus Holtermann | Owner: nobody
Cleanup/optimization |
Component: Testing framework | Version: 3.2
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: new => closed
* resolution: => wontfix
Comment:
OK, let's say wontfix. (Happy if anyone wants to make the case we're
**wrong** here, but it doesn't look that way, I think… 🤔)
--
Ticket URL: <https://code.djangoproject.com/ticket/33230#comment:3>
Django
Reporter: Markus Holtermann | Owner: nobody
Cleanup/optimization |
Component: Testing framework | Version: 3.2
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Anders Kaseorg):
* status: closed => new
* resolution: wontfix =>
Comment:
Reopening with two arguments, one theoretical and one practical.
The test client currently omits both `Content-Length` and `Content-Type`
for an empty `PATCH`/`POST`/`PUT` body. But RFC 9110 is [https://www.rfc-
editor.org/rfc/rfc9110#section-8.6-5 explicitly clear] that `Content-
Length` is expected here: “A user agent SHOULD send Content-Length in a
request when the method defines a meaning for enclosed content and it is
not sending Transfer-Encoding. For example, a user agent normally sends
Content-Length in a POST request even when the value is 0 (indicating
empty content). A user agent SHOULD NOT send a Content-Length header field
when the request message does not contain content and the method semantics
do not anticipate such data.”
Although the expectation for `Content-Type` is potentially more open to
interpretation (“A sender that generates a message containing content
SHOULD generate a Content-Type header field…”), including it is the only
way to inform the server how to decode the empty body; for example, an
empty `application/x-www-form-urlencoded` body is valid while an empty
`application/json` body is not.
As a practical application, this is required for validating requests
against an OpenAPI document: the `Content-Type` is needed to
[https://spec.openapis.org/oas/v3.1.0#requestBodyObject index into the
map] that declares the body’s schema and encoding.
--
Ticket URL: <https://code.djangoproject.com/ticket/33230#comment:4>
Django
Reporter: Markus Holtermann | Owner: Anders
Type: | Kaseorg
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: 3.2
Severity: Normal | Resolution:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Anders Kaseorg):
* owner: nobody => Anders Kaseorg
* status: new => assigned
* has_patch: 0 => 1
Comment:
Submitted a patch at https://github.com/django/django/pull/17404.
--
Ticket URL: <https://code.djangoproject.com/ticket/33230#comment:5>
Django
Reporter: Markus Holtermann | Owner: Anders
Type: | Kaseorg
Component: Testing framework | Version: 3.2
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Natalia Bidart):
* status: assigned => closed
* resolution: => wontfix
Comment:
Hello Anders, thank you for providing additional details.
I've gone through all the links, and for a while, I found myself diving
into some non-trivial rabbit holes. However, it was a fun exercise to read
and investigate this issue. I believe we are in agreement that the HTTP
spec (RFC 9110) does not require a `Content-Type` header for empty bodies.
Having said that, I truly appreciate how request validation against an
OpenAPI spec benefits from always having a `Content-Type` header for map
indexing. However, if I were implementing a web service that does this, I
would make it robust enough to handle requests without this header,
considering that the HTTP spec does not enforce its presence. This could
involve checking the `Content-Length` first or using another strategy that
aligns with the specific business logic of the API endpoint.
The key point here is that, even if we were to change how the Django test
client behaves, a robust web service should still be able to handle cases
where the `Content-Type` is missing. After all, there might be other real
clients out there that do not send it for requests with empty bodies.
Following that rationale, I think that this is still a `wontfix`. If you
disagree, the recommended path forward is to first propose and discuss the
issue with the community and gain consensus to pursue the proposed change.
To do that, please start a new conversation on the
[https://forum.djangoproject.com/c/internals/5 Django Forum], where you'll
reach a wider audience and likely get extra feedback.
Cheers, Natalia.
--
Ticket URL: <https://code.djangoproject.com/ticket/33230#comment:6>
Django
Reporter: Markus Holtermann | Owner: Anders
Type: | Kaseorg
Cleanup/optimization | Status: closed
Component: Testing framework | Version: 3.2
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Lily Foote):
I'm sorry if I'm misunderstanding something here, but doesn't the wontfix
here leave it impossible to test the case where the content type is
explictly set?
Using the example above, both these cases behave the same:
{{{
class MyTest(SimpleTestCase):
def test_content_type(self):
resp = self.client.post("/", data=b"", content_type="application
/octet-stream")
print(resp.content)
# b"{'Cookie': ''}"
def test_no_content_type(self):
resp = self.client.post("/", data=b"")
print(resp.content)
# b"{'Cookie': ''}"
}}}
But it would be useful for the first case to set the content type so we
can test the case where it is present for empty content.
--
Ticket URL: <https://code.djangoproject.com/ticket/33230#comment:7>
Django
Reporter: Markus Holtermann | Owner: Anders
Type: | Kaseorg
Cleanup/optimization | Status: closed
Component: Testing framework | Version: 3.2
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Natalia Bidart):
Hey Lily and others!
I have an old action item to follow up on this. I'm not finding the time
to do so, would you or anyone involved in this ticket have the
availability to raise this in the forum?
I'd be happy to re-open once we get more eyes/thoughts on this.
--
Ticket URL: <https://code.djangoproject.com/ticket/33230#comment:8>