Opened 13 years ago

Closed 12 years ago

Last modified 12 years 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: Aymeric Augustin
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 Aymeric Augustin 13 years ago.
17371.patch (15.8 KB ) - added by Aymeric Augustin 13 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 by Aymeric Augustin, 13 years ago

Component: UncategorizedTesting framework
Has patch: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/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.

by Aymeric Augustin, 13 years ago

Attachment: 17371-wip.patch added

comment:2 by Aymeric Augustin, 13 years ago

#15596 was closed as a duplicate.

Last edited 13 years ago by Aymeric Augustin (previous) (diff)

comment:3 by Aymeric Augustin, 13 years ago

Owner: changed from nobody to Aymeric Augustin

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 by Aymeric Augustin, 13 years ago

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

by Aymeric Augustin, 13 years ago

Attachment: 17371.patch added

comment:5 by Aymeric Augustin, 12 years ago

Triage Stage: AcceptedReady for checkin

comment:6 by Aymeric Augustin <aymeric.augustin@…>, 12 years ago

Resolution: fixed
Status: newclosed

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 by Aymeric Augustin, 12 years ago

#18048 was a duplicate.

comment:8 by Aymeric Augustin, 12 years ago

#19797 was a duplicate.

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