Code

Opened 2 years ago

Closed 23 months ago

Last modified 14 months ago

#17371 closed Cleanup/optimization (fixed)

For DELETE requests TestClient encodes data as QUERY_STRING and forces an empty payload

Reported by: grzesiof@… Owned by: aaugustin
Component: Testing framework Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The Test Client's delete method is too limited.

The http 1.1 rfc (http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html) doesn't specify that a request body is not allowed for DELETE requests. In fact the method is more similar to POST's and PUT's then it is to GET's. However when making a DELETE request with the test client the data is encoded as the query_string and no request-body-payload is provided
(https://code.djangoproject.com/browser/django/trunk/django/test/client.py#L313).

I would personally make it behave the same way as POST's do, but I can imagine a lot of code may depend on the existing implementation.

Attachments (2)

17371-wip.patch (2.4 KB) - added by aaugustin 2 years ago.
17371.patch (15.8 KB) - added by aaugustin 2 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 2 years ago by aaugustin

  • Component changed from Uncategorized to Testing framework
  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization

Indeed, I don't think Django should do anything special for requests other than GET/HEAD and POST.

For GET and POST requests, Django provides decoded arguments in the request.GET and request.POST dictionaries, because that's what browsers use. It makes sense for the test client to provide the reverse encoding. And HEAD is well defined as being the same thing as GET.

For all other HTTP methods, including DELETE, Django won't process the request body, see #12635. So it would be more consistent if the test client didn't perform encoding — it currently does for PUT (like POST) and DELETE (like GET). See attached patch.

This patch probably breaks a bunch of tests, and I haven't decided what to do wrt. backwards compatibility yet. But I'm supporting the idea of making the test client more predictable.

See also #17360.

Changed 2 years ago by aaugustin

comment:2 Changed 2 years ago by aaugustin

#15596 was closed as a duplicate.

Last edited 2 years ago by aaugustin (previous) (diff)

comment:3 Changed 2 years ago by aaugustin

  • Owner changed from nobody to aaugustin

I'm quite sure we'll have to break backwards compatibility to obtain a consistent behavior of the TestClient on requests other than GET and POST. I intend to work on this after the 1.4 release.

comment:4 Changed 2 years ago by aaugustin

  • Patch needs improvement unset

Here's a patch that removes specific behavior from the test client for non-web methods, ie. methods other than GET, HEAD and POST.

In recent discussions, there was a clear consensus about not treating PUT requests like POST requests in Django. For consistency this design decision should be reflected in the test client. The same argument applies to OPTIONS and DELETE.

This is a backwards incompatible change. What do you think?

Related: #10571

Changed 2 years ago by aaugustin

comment:5 Changed 2 years ago by aaugustin

  • Triage Stage changed from Accepted to Ready for checkin

comment:6 Changed 23 months ago by Aymeric Augustin <aymeric.augustin@…>

  • Resolution set to fixed
  • Status changed from new to closed

In [e73838b6ddcc7b37c03f9eee04fa6e6a283fedb3]:

Fixed #17371 -- Made the test client more flexible

The OPTIONS, PUT and DELETE methods no longer apply arbitrary
data encoding (in the query string or in the request body).

comment:7 Changed 22 months ago by aaugustin

#18048 was a duplicate.

comment:8 Changed 14 months ago by aaugustin

#19797 was a duplicate.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.