fuel: httpDelete did not send body payload

Fuel: v1.12.0

I tried to sent HTTP DELETE with JSON body. I checked HttpClient.kt that it skip body set up for GET, DELETE, HEAD

private fun setDoOutput(connection: HttpURLConnection, method: Method) = when (method) {
   Method.GET, Method.DELETE, Method.HEAD -> connection.doOutput = false
   Method.POST, Method.PUT, Method.PATCH -> connection.doOutput = true
}

Should we allow Method.DELETE? Because I have scenario that API need body payload for DELETE.

Same issue #245

About this issue

Original URL
State: open
Created 6 years ago
Comments: 32 (4 by maintainers)

Commits related to this issue

Allow body for delete requests (#306) — committed to strangeAeon/Fuel by deleted user 6 years ago
Allow body for delete requests (#306) — committed to strangeAeon/Fuel by deleted user 6 years ago

Most upvoted comments

I noticed curl response contained HTTP/2 200. I don’t know HttpUrlConnection able to support HTTP/2.

iNoles on Nov 6, 2018

@babedev @simonharrer @kittinunf @SleeplessByte @kmcgill88

Guys I am going to pick this up I have opened a WIP Pr (https://github.com/kittinunf/Fuel/pull/417)

I would really appreciate your feedback and reviews.

markGilchrist on Aug 22, 2018

Just as a datapoint, RequestBody with DELETE wasn’t working for me on 1.13.0, and after upgrading to 1.16.0 it is working for me. I’m not discounting @smallufo 's report, just letting you know that in my case it is working.

checketts on Nov 13, 2018

Alright. This might be an issue on our side. I will try some things out over here and see if I can make this work (and write a test so it doesn’t fail again/regression)

SleeplessByte on Oct 29, 2018

I try to remove the Accept-Encoding header ,

val (req: Request, res: Response, result: Result<String, FuelError>) = Fuel.delete(uri).also {
  it.headers.remove("Accept-Encoding")
}
  .header(mapOf("Content-Type" to "application/json"))
  .body(json)
  .responseString()

logger.info("curlString = {}" , req.cUrlString())

which generates this :

curl -i -X DELETE -d "{\"fields\": [\"persistent_menu\"]}" -H "Content-Type:application/json" https://graph.facebook.com/v3.2/me/messenger_profile?access_token=EAAGpxxx

The result is the same : failed in kotlin code , success in curl console.

If I remove Content-Type header (which leads empty header) , it won’t pass FB (both code and cURL) . So Content-Type header is mandatory here.

smallufo on Oct 28, 2018

Sorry , isn’t it what I have already done ?

val (req: Request, res: Response, result: Result<String, FuelError>) = Fuel.delete(uri)
  .header(mapOf("Content-Type" to "application/json"))
  .body(json)
  .responseString()

logger.info("curlString = {}" , req.cUrlString())
logger.info("req string = {}" , req.toString())

The result is as the screenshot I pasted.

smallufo on Oct 28, 2018

Wow , I don’t know Fuel has curlString function , great. This is my result :

curl -i -X DELETE -d "{\"fields\": [\"persistent_menu\"]}" -H "Accept-Encoding:compress;q=0.5, gzip;q=1.0" -H "Content-Type:application/json" https://graph.facebook.com/v3.2/me/messenger_profile?access_token=EAAGpXXX

intellij The result is the same.

BUT , very interesting . After I copy the curlString to console and execute , it is OK !!! curl

This is so weird.

smallufo on Oct 28, 2018

Before, the spec defined that the body SHOULD be ignored (and therefore a lot of clients did not allow for a DELETE request’s body, but this was updated with RFC7231:

https://tools.ietf.org/html/rfc7231#section-4.3.5

4.3.5.  DELETE
  
   [...]
   
   A payload within a DELETE request message has no defined semantics;
   sending a payload body on a DELETE request might cause some existing
   implementations to reject the request.

   Responses to the DELETE method are not cacheable.  If a DELETE
   request passes through a cache that has one or more stored responses
   for the effective request URI, those stored responses will be
   invalidated (see Section 4.4 of [RFC7234]).

Therefore, even though current users might not need this as a feature, it SHOULD be implemented.

SleeplessByte on Jul 25, 2018

Yeah, I think by the time I have designed this base on the RFC spec. It is not specified as permitted or not so that I didn’t follow through with the implementation for this. However, most of the modern HTTP libraries support this so we should support it too!

@babedev Would you mind creating a PR for this? I can help you out as well.

kittinunf on Feb 11, 2018