ktorio / ktor

Framework for quickly creating connected applications in Kotlin with minimal effort
https://ktor.io
Apache License 2.0
12.86k stars 1.04k forks source link

Content-Type in a request headers #1127

Closed mister11 closed 1 year ago

mister11 commented 5 years ago

Ktor Version

1.1.4

Ktor Engine Used(client or server and name)

Apache

JVM Version, Operating System and Relevant Context

1.8, macOS Mojave and Linux Mint, IDEA 2019.1.2 CE

Feedback

I'm accessing and API that requires me to set Content-Type for all requests, but I'm getting io.ktor.http.UnsafeHeaderException: Header Content-Type is controlled by the engine and cannot be set explicitly

Current implementation:

val client = HttpClient(Apache) {
        install(JsonFeature) {
            serializer = GsonSerializer()
        }
        install(Logging) {
            level = LogLevel.HEADERS
        }
        defaultRequest {
            header("Content-Type", "application/vnd.api+json")
        }
    }
get("/test") {
            client.get<String>("url...") {
                headers {
                    // other headers
                }
            }
        }
mister11 commented 5 years ago

As an additional information, I can solve this issue by using OkHttp with a network interceptor, but I'm curious how to do the same thing using Apache client

e5l commented 5 years ago

Hi @mister11, thanks for the report. We introduced acceptContentTypes in JsonFeature since 1.2.0. It provides you possibility to set custom Accept and Content-Type headers automatically.

Could you check and report if it solves your problem?

mister11 commented 5 years ago

Not sure if I'm doing something wrong, but this is not working:

val client = HttpClient(Apache) {
        install(JsonFeature) {
            serializer = GsonSerializer()
            acceptContentTypes = acceptContentTypes + listOf(ContentType.parse("application/vnd.api+json; ext=bulk"))
        }
    }

This just sets Accept header and I need Content-Type header for requests.

To make it more clear, here is a working version of OkHttp interceptor implementation:

fun provideHeadersInterceptor() = Interceptor { chain ->
    val requestBuilder = chain.request().newBuilder()
        .addHeader("Content-Type", "application/vnd.api+json")

    chain.proceed(requestBuilder.build())
}
fun provideOkHttpClient(): HttpClient = HttpClient(OkHttp) {
    engine {
        addNetworkInterceptor(provideHeadersInterceptor())
    }
}
stosik commented 5 years ago

any solutions? I am in the same situation, but for me even interceptior does not seem to work as I would expected and I'm getting

Exception in thread "main" java.lang.ClassCastException: class shared.model.ProductUpdateDto cannot be cast to class io.ktor.client.call.HttpClientCall (shared.model.ProductUpdateDto and io.ktor.client.call.HttpClientCall are in unnamed module of loader 'app')
sannysoft commented 4 years ago

I can't use Ktor client just because I'm unable to set Content-Type. Even your examples doesn't work:

   val message = client.post<HelloWorld> {
      url("http://127.0.0.1:8080/")
      contentType(ContentType.Application.Json)
      body = HelloWorld(hello = "world")
   }
Andromedids commented 4 years ago

@sannysoft , the same doesn't work for me neither. But I found this post - and I can admit, this workaround works. But... it's a workaround :( (https://github.com/ktorio/ktor/issues/635)

val response = client.call(url) {
   method = HttpMethod.Post
   body = TextContent(json.writeValueAsString(userData), contentType = ContentType.Application.Json)
}.response
wfxr commented 4 years ago

Ali-OSS also encountered this problem. We need the ability to manually control any headers including Content-Type.

aajtodd commented 4 years ago

Why can't clients control Content-Type header? I'm not sure I understand why the engine "has" to.

It's also confusing since the public API of HttpMessageBuilder includes a setter

Agreed we need the ability to manually control this header for a number of reasons.

dbof10 commented 4 years ago

any solutions guys?

oleg-larshin commented 4 years ago

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.

loalexzzzz commented 4 years ago

any solutions guys?

Firerer commented 3 years ago

Ran into the same problem. Found out that I was use HttpMethod.Get and append("Content-Type", "application/json") at the same time. It is total fine for HttpMethod.Post request. Seems like this is the reason cause the problem. The error message is useless in this case.

kalgecin commented 3 years ago

I'm also facing this issue. The api I'm calling REQUIRES Content-Type header in a GET request (yes I know why?), otherwise it returns error. I'm currently using another http library, but would like to move my library to Multiplatform and Ktor seems to be a good option. Is content-type blocked in Ktor GET for a reason? Are there any plans to allow it?

minxylynx commented 2 years ago

bump on this. I'm facing the same issue, where the get requires the content-type be specified

adamwaite commented 2 years ago

Also blocked by this. Isn't this a normal thing to do? Why isn't it allowed...

sud007 commented 2 years ago

Oh dear, I am blocked on my PUT request on KMM Android Project! Any resolution guys?

typfel commented 2 years ago

I've solved this problem by registering my custom content types using the ContentNegotiation plugin in a fairly simple way at least for binary content. I think the ktor documentation should contain an example like this since this is a fairly common use case.

example: https://github.com/wireapp/kalium/pull/324

Kosert commented 2 years ago

On YouTrack this is marked as fixed, but I was still unable to set Content-Type header to GET request using ktor 2.0.3. Eventually I came up with this relatively simple workaround:

class EmptyContentWithContentType(
    override val contentType: ContentType
) : OutgoingContent.NoContent() {

    override val contentLength: Long = 0

    override fun toString(): String = "EmptyContent(contentType='$contentType')"
}

And then in request builder:

setBody(EmptyContentWithContentType(ContentType("application", "definitely.not.json")))
flaringapp commented 1 year ago

Still no solution?

sud007 commented 1 year ago

Yep no solution we need a content-type = application/x-www-form-urlencoded sadly, this has yet not worked for us!

e5l commented 1 year ago

@rsinukov could you check if we can do something?

flaringapp commented 1 year ago

Having conducted some investigation, I realized that ktor erases the Content-Type header in ContentNegotiation plugin, namely in io.ktor.client.plugins.contentnegotiation.ContentNegotiation.convertRequest():

...
request.headers.remove(HttpHeaders.ContentType)
...

Nevertheless, this header is appended to the request when ktor request is being converted to the engine request. E.g., in OkHttp it's done using io.ktor.client.engine.mergeHeaders() method used in file io.ktor.client.engine.okhttp.OkHttpEngine.kt, method convertToOkHttpRequest(). The most interesting part is inside io.ktor.client.engine.mergeHeaders() method. Indeed, it tries to resolve content type and length, and append Content-Type header:

...
val type = content.contentType?.toString()
    ?: content.headers[HttpHeaders.ContentType]
    ?: requestHeaders[HttpHeaders.ContentType]

val length = content.contentLength?.toString()
    ?: content.headers[HttpHeaders.ContentLength]
    ?: requestHeaders[HttpHeaders.ContentLength]

type?.let { block(HttpHeaders.ContentType, it) }
length?.let { block(HttpHeaders.ContentLength, it) }

In the end, I believe Content-Type header shouldn't be erased in the first place, but at least it'll be nice to provide explanation stated directly in content negotiation plugin documentation.

rsinukov commented 1 year ago

@sud007 Can you please elaborate on what doesn't work? This test passes:


    @Test
    fun testEmptyBodyWithContentTypeAndGet() = testSuspend {
        val client = HttpClient(MockEngine) {
            engine {
                addHandler { request ->
                    assertEquals("application/protobuf", request.headers[HttpHeaders.ContentType])
                    respond("OK")
                }
            }
        }

        client.get("/") {
            header(HttpHeaders.ContentType, ContentType.Application.ProtoBuf)
        }
    }
flaringapp commented 1 year ago

@rsinukov Please try adding content negotiation plugin

install(ContentNegotiation) {
    json()
}

And sending post request with any @Serializable object:

client.post("/") {
    header(HttpHeaders.ContentType, ContentType.Application.Json)
    setBody(
        SomeJsonObject("Hello", "Ktor")
    )
}

The test you've provided will fail (assert block should be updated to application/json):

expected:<application/json> but was:<null>
rsinukov commented 1 year ago

@flaringapp But isn't it only relevant for MockEngine, because it doesn't merge headers? In the real request Content-Type header will be present.

kalgecin commented 1 year ago

real question is what is the reason of removing the header if it was set by the user?

flaringapp commented 1 year ago

@rsinukov Yes, you are right. In the real request in will be present. I just don't want to say that the issue is explicitly with the MockEngine. Going back to content negotiation plugin which erases Content-Type header: it operates on HttpRequestPipeline.Transform phase, and the real request is being transformed and executed on HttpRequestPipeline.Send phase. Meaning, anywhere in between these two phases we won't be able to access the header no matter what engine we use.

flaringapp commented 1 year ago

Another question is why to completely delegate header appending to an engine. Is it feasible to keep the header by default, and let any engine override/remove it if necessary?

rsinukov commented 1 year ago

@flaringapp

Meaning, anywhere in between these two phases we won't be able to access the header no matter what engine we use.

You are able to access it through content.contentType, the same way as mergeHeaders does.

@kalgecin @flaringapp Can you elaborate on what problems it causes?

flaringapp commented 1 year ago

@rsinukov in my case, a test with MockEngine that verifies Content-Type header was failing. As you stated.

kalgecin commented 1 year ago

@rsinukov As several people have mentioned, some APIs for some reason require the header present, otherwise they return an error. And using such APIs with ktor is impossible because ktor removes the header. I'm still to understand the reason why ktor does not want to send the header through

rsinukov commented 1 year ago

@kalgecin Can you show me a reproducer where the client doesn't send header to a server, please? I fail to create one.

grassehh commented 6 months ago

@rsinukov it works but I am questioning a behavior. Why does the defaultTransformers remove the Content-Type header? This is relatively quite unintuitive as this can lead to issues like this one: https://github.com/zalando/logbook/issues/1627 where the Monitor plugin expects to have such header in the context.headers instead of OutgoingContent.

jafaircl commented 1 month ago

So there is no way to send the (extremely common) Content-Type header no matter how we attempt to append it without using a third party library to intercept requests? That seems... not great