Opened 12 years ago

Closed 12 years ago

#18048 closed Bug (duplicate)

Test client implicitly assuming data to be querystring

Reported by: johan@… Owned by: Johan Sommerfeld
Component: Testing framework Version: 1.4
Severity: Normal Keywords: test client
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In some of the request methods (get, head, options, delete) the data input is assumed to be used as querystring input. This makes the client break when sending data such as a json string since urlencode assumes data to be a dict

This has been solved for some methods. In #11371 for the put request, where the solution was that urlencode was removed and querystring was only extracted from the path with urlparse.

The methods have a lot of redundant code, wehre as it is mostly the setting of content-length and such, this could be consolidated to one function wheras we could keep the possibility to send querystring as a dict into data with the content_type set to some dummy type such as "querystring". This aproach would be benefitial since we then allow the test client to send a body with all request, which is as far as I know not forbidden according to the rfc:2616 (I'm no expert in reading rfcs if I'm wrong pleas correct me).


Attachments (2)

test_patch.diff (2.1 KB ) - added by Johan Sommerfeld <johan@…> 12 years ago.
tests showing problem
patch_and_tests.diff (6.1 KB ) - added by Johan Sommerfeld <johan@…> 12 years ago.
patch of client, more and modified tests

Download all attachments as: .zip

Change History (6)

comment:1 by Johan Sommerfeld <johan@…>, 12 years ago

Type: UncategorizedBug

by Johan Sommerfeld <johan@…>, 12 years ago

Attachment: test_patch.diff added

tests showing problem

by Johan Sommerfeld <johan@…>, 12 years ago

Attachment: patch_and_tests.diff added

patch of client, more and modified tests

comment:2 by Johan Sommerfeld <johan@…>, 12 years ago

Has patch: set

upploaded patch and more tests. When I fixed the problem I saw one of my added tests was expecting a body on a head request.

the tests suite goes through.

I haven't written any document since the functionality hasn't changed. When I read the documentation I actually thought this would be supported. If anyone disagrees, pleas point me to where you would like me to add som documentation

comment:3 by Johan Sommerfeld, 12 years ago

Owner: changed from nobody to Johan Sommerfeld
Status: newassigned

comment:4 by Aymeric Augustin, 12 years ago

Resolution: duplicate
Status: assignedclosed

I committed a fix for #17371 after you opened this ticket, and I believe it addresses your concerns.

If it doesn't, could you open a new ticket specifically about the problems that still need fixing?

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