Opened 3 years ago

Closed 14 months ago

Last modified 3 months ago

#33230 closed Cleanup/optimization (wontfix)

Test client doesn't set explicitly provided Content-Type when data is empty

Reported by: Markus Holtermann Owned by: Anders Kaseorg
Component: Testing framework Version: 3.2
Severity: Normal Keywords:
Cc: Aymeric Augustin Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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:

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

Change History (8)

comment:1 by Carlton Gibson, 3 years ago

​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 πŸ€”)

comment:2 by Mariusz Felisiak, 3 years ago

Cc: Aymeric Augustin added
Component: Uncategorized β†’ Testing framework
Type: Uncategorized β†’ Cleanup/optimization

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.

comment:3 by Carlton Gibson, 3 years ago

Resolution: β†’ wontfix
Status: new β†’ closed

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… πŸ€”)

comment:4 by Anders Kaseorg, 14 months ago

Resolution: wontfix
Status: closed β†’ new

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 ​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 ​index into the map that declares the body’s schema and encoding.

comment:5 by Anders Kaseorg, 14 months ago

Has patch: set
Owner: changed from nobody to Anders Kaseorg
Status: new β†’ assigned

comment:6 by Natalia Bidart, 14 months ago

Resolution: β†’ wontfix
Status: assigned β†’ closed

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 ​Django Forum, where you'll reach a wider audience and likely get extra feedback.

Cheers, Natalia.

comment:7 by Lily Foote, 14 months ago

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.

comment:8 by Natalia Bidart, 3 months ago

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.

Note: See TracTickets for help on using tickets.
Back to Top