Fuel: change in behaviour of POST between 1.14.0 and 1.15.0

[fnunezkanut]

It seems a change was introduced sometime in 1.15.0

This used to work

    fun getSessionInfo(tokenId: String): AmSession {
        logger.debug { "fetching info for session: $tokenId" }
        val (req, res, result) =
            http.request(
                method = Method.POST,
                path = "/json/sessions?_action=getSessionInfo&tokenId=$tokenId"
            )
                    .header("Accept-API-Version" to "resource=2.0")
                    .header("Content-Type" to "application/json")
                    .responseString()
        logger.debug { "\n${req.cUrlString()}\n--> $req\n$res" }
        result.fold({ d ->
            return Json().parse(d)
        }, { err ->
            throw err.exception
        })
    }

and it sent this request

$ curl -i -X POST -H "Accept-Encoding:compress;q=0.5, gzip;q=1.0" -H "Content-Type:application/json" -H "Accept:application/json" -H "Accept-API-Version:resource=2.0" https://authserver/json/sessions?_action=getSessionInfo&tokenId=d5zqRsYnIPTqXvZRZGvdEdOB9Gs.*AAJTSQACMDIAAlNLABxPWFZLMHRhMHBHeXl6YjMrQ1h5Wk5LdGJubFk9AAJTMQACMDE.*
--> "Body : (empty)"
"Headers : (4)"
Accept-Encoding : compress;q=0.5, gzip;q=1.0
Content-Type : application/json
Accept : application/json
Accept-API-Version : resource=2.0

Notice the content type is "application/json" as is expected due to it being requested in code example on line 10

however in newer versions the request is this, notice the content type header being appended for some reason, instead of overwritting

$ curl -i -X POST -H "Accept-Encoding:compress;q=0.5, gzip;q=1.0" -H "Content-Type:application/x-www-form-urlencoded; application/json" -H "Accept:application/json" -H "Accept-API-Version:resource=2.0" https://authserver/json/sessions?_action=getSessionInfo&tokenId=d5zqRsYnIPTqXvZRZGvdEdOB9Gs.*AAJTSQACMDIAAlNLABxPWFZLMHRhMHBHeXl6YjMrQ1h5Wk5LdGJubFk9AAJTMQACMDE.*
--> --> https://authserver/json/sessions?_action=getSessionInfo&tokenId=d5zqRsYnIPTqXvZRZGvdEdOB9Gs.*AAJTSQACMDIAAlNLABxPWFZLMHRhMHBHeXl6YjMrQ1h5Wk5LdGJubFk9AAJTMQACMDE.*
"Body : (empty)"
"Headers : (4)"
Accept-Encoding : compress;q=0.5, gzip;q=1.0
Content-Type : application/x-www-form-urlencoded; application/json
Accept : application/json
Accept-API-Version : resource=2.0

content type now for whatever reason is Content-Type : application/x-www-form-urlencoded; application/json

The server in this instance cant handle that, but the issue is that Fuel is doing something other than what the code tells it to do, it should send Content-Type : application/json as that is what was specified in code

[fnunezkanut]

Ok I looked over changes and this seems to be the the change which causing this issue in Request.kt line 109

    fun header(vararg pairs: Pair<String, Any>?): Request {
        pairs.filterNotNull().forEach { (key,value) ->
            if (!headers.containsKey(key)) {
                headers += Pair(key, value.toString())
            } else {
                headers[key] =  headers.getValue(key).let { "$it; $value" }
            }
        }
        return this
    }

headers[key] = headers.getValue(key).let { "$it; $value" }

so this appends the value instead of overwriting it

[fnunezkanut]

Seems change occurred after 1.14.0 not 1.13.0 as i originally theorized, editing title

[SleeplessByte]

@kittinunf @markGilchrist

Content-Type is not a header that can be appended to. Changing this into something like

fun header(vararg pairs: Pair<String, Any>?): Request {
    headers.delete(key)
    return appendHeader(pairs)
}

fun appendHeader(pairs: Pair<String, Any>?): Request {
    pairs.filterNotNull().forEach { (key,value) ->
        if (!headers.containsKey(key)) {
            headers += Pair(key, value.toString())
        } else {
            headers[key] =  headers.getValue(key).let { "$it; $value" }
        }
    }
    return this
}

allows you to keep the old behaviour whilst supporting the new behaviour.

[markGilchrist]

@SleeplessByte @fnunezkanut The reason this was added was to support multiple headers by the same key.

if you change your code from

.header("Accept-API-Version" to "resource=2.0")
                    .header("Content-Type" to "application/json")
                    .responseString()

to

.header(mapOf("Accept-API-Version" to "resource=2.0", "Content-Type" to "application/json"))

I believe that it will overwrite the values if that is what you require. If I have any of the above wrong then please let me know and I will investigate further

[SleeplessByte]

Yep,

I was there when you added it :p.

What I mean is: you can keep the overwrite behaviour in the public API
whilst also supporting multiple headers by the same key.

To me both seem acceptable. One is assuming header is always additive and
one is assuming header is always overwriting.

Cheers!

On Thu, 9 Aug 2018, 19:23 Mark Gilchrist, [e-mail] wrote:

@SleeplessByte https://github.com/SleeplessByte @fnunezkanut
https://github.com/fnunezkanut The reason this was added was to support
multiple headers by the same key.

if you change your code from

.header("Accept-API-Version" to "resource=2.0")
.header("Content-Type" to "application/json")
.responseString()

to

.header(mapOf("Accept-API-Version" to "resource=2.0", "Content-Type" to "application/json"))

I believe that it will overwrite the values if that is what you require.
If I have any of the above wrong then please let me know and I will
investigate further

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kittinunf/Fuel/issues/408#issuecomment-411834062, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB35WMhD_v8HHgqg2CUcv3ZqmNzQl9YNks5uPHAXgaJpZM4V1dcQ
.

[fnunezkanut]

Hah sure enough that (passing a map of pairs to header()) does the trick! Sorry for false alarm :)