DELETE does not allow POST-style data

[fluxsauce]

One of our API calls using method DELETE requires JSON in a POST field. We had been doing it with straight cURL. When transitioning to Requests, I tried the delete method but that didn't allow data to be passed. Then, I attempted to just use the request method.

$response = Requests::request($url, $headers, $data, Requests::DELETE, $options);

URL:

https://REMOVED/terminus.php?site=UUID&path=code-branch

headers:

Array (
    [Content-Type] => application/json
    [Content-Length] => 40
)

data:

{"data":{"refspec":"refs\/heads\/prog"}}

method: DELETE

options:

Array (
    [timeout] => 30
    [cookies] => Array (
            [0] => REMOVED
        )
    [verifyname] => FALSE
    [verify] => FALSE
)

That caused:

http_build_query(): Parameter 1 expected to be Array or Object. Incorrect value given cURL.php:327

https://github.com/rmccue/Requests/blob/master/library/Requests/Transport/cURL.php#L327

I believe the problem is at:

https://github.com/rmccue/Requests/blob/022400b23f697792fa340c93d250257293c6ec37/library/Requests/Transport/cURL.php#L224

This assumes that DELETE won't be POSTing. Additionally:

https://github.com/rmccue/Requests/blob/master/library/Requests/Transport/cURL.php#L241

cURL isn't setting options for CURLOPT_POST or CURLOPT_POSTFIELDS.

I'll admit that this is a bit edge-case. However, it doesn't seem to be against RFC. http://stackoverflow.com/questions/299628/is-an-entity-body-allowed-for-an-http-delete-request gets into that in greater detail.

I have no problem setting an additional option or calling the request method in a different way.

Let me know if you have any questions or if you need more context! Thanks in advance.

[rmccue]

I checked the source after reading the ticket and thought this might be the case. Drats.

So, the issue here isn't really whether it's a bug, but rather how to accommodate this in a backwards compatible way with the API. Technically, GET and HEAD can also have entity bodies (only TRACE disallows it), but it's convenient to pass in an array of data and have that squashed into a query string. This is also inconsistent with POST, PATCH and PUT, since you can't add query parameters there.

I'd also hate to add in a parameter like actually_use_this_as_data, but I'm stuck here with backwards compatibility.

Unless you've got a better idea of how to handle this, here's what I'm thinking:

• Requests 1.7: Introduce a use_data_as_query option, default to in_array($options['type'], array(Requests::HEAD, Requests::GET, Requests::DELETE)). This solves the issue for you now, but is pretty ugly.
• Requests 2.0: Change the data parameter to always contain the entity body, and reconsider the convenience of data-as-query-string. Most likely add a query option that can do this across all request methods.

Comments welcome, as always. :)

[fluxsauce]

Thanks for looking into this!

• Requests 1.7 - that would work. data_as_query might be more concise.
• Requests 2.0 - rather than data, what about body? That's more specific, and then query would make more sense as well.

[shrivedog]

Currently encountering this behavior/bug in my API testing.

Any decisions on the implementation strategy, or assignment of the coding work?

Personally I rather enjoy the convenience of automatic parsing of the data into a query string, and would opt for a boolean option to indicate when this should be done. Default for the option is a toss up.

[kadimi]

Solution on #347

You need to use Requests::request to send GET, DELETE, or OPTIONS requests with bodies, rather than using the convenience methods.