Hi, the below test is throwing `java.lang.IllegalA...

# ktor

[Chris C]

Hi, the below test is throwing java.lang.IllegalArgumentException: List has more than one element because there are two values in the Accept header. Removing jackson fixes it, but I was hoping to use the same client for json responses too. Is this possible? I’m using 2.0.0-beta-1.

class AcceptTest {
    private val readmeContentType = ContentType.parse("application/vnd.github.v3.html")

    @Test
    fun acceptTest() {
        val client = HttpClient() {
            install(ContentNegotiation) {
                jackson {  }
            }
        }
        val readme: String = runBlocking {
            client.get("<https://api.github.com/repos/ktorio/ktor/readme>") {
                header(HttpHeaders.Accept, readmeContentType)
            }.body()
        }
        println(readme)
    }
}

[Rustam Siniukov]

Weird, but github replies with 2 values in Content-Type header, when client sends 2 values in Accept : Content-Type: application/json,application/vnd.github.v3.html; charset=utf-8

[Chris C]

Yes, you’re right, the exception is happening because Github replies with 2 content-type headers which I hadn’t realised. However, my problem is really why is ktor sending 2 accept headers? It seems the application/json accept header is being added after my header is added, so I can’t even remove it.

[Rustam Siniukov]

It’s allowed to send 2 accept headers. For example, a client can be able to understand json and plain text, so it sends Accept: application/json, text/plain . Then server can choose which format to reply

[Chris C]

Sure, but I don't want to send 2 accept headers because I only want to accept the html one and there doesn't appear to be a way to control that

[Chris C]

I think if an accept header is set in the request it should take precedence over the list of content types which have a converter registered. In ContentNegotiation.Plugin#install there’s this bit of code which is replacing the accept headers:

val registrations = plugin.registrations
registrations.forEach { context.accept(it.contentTypeToSend) }

If that would be changed to this then I think that would give the request accept header precedence wouldn’t it?

val registrations = plugin.registrations
if (!context.headers.contains(HttpHeaders.Accept)) {
    registrations.forEach { context.accept(it.contentTypeToSend) }
}

What do you think @Rustam Siniukov?

[Rustam Siniukov]

I can imagine that there are cases where users may need both headers. Also, there can be multiple plugins installed, and each of them may add its own value and it’s a valid usage. Breaking this because of github weird reply is not good.
You can fix it on your side with interceptor, that will modify your headers and override Accept header when needed. Something like

private val OverrideAcceptHeaderKey = AttributeKey<ContentType>("OverrideAcceptHeaderKey")
    public var HttpRequestBuilder.overrideAcceptHeader: ContentType?
        get() = attributes.getOrNull(OverrideAcceptHeaderKey)
        set(value) {
            if (value != null) attributes.put(OverrideAcceptHeaderKey, value)
            else attributes.remove(OverrideAcceptHeaderKey)
        }

    @Test
    fun acceptTest() = runBlocking {
        val client = HttpClient(CIO) {
            install(ContentNegotiation) {
                json()
            }
        }
        val overrideAcceptHeaderPhase = PipelinePhase("overrideAcceptHeaderPhase")
        client.requestPipeline.insertPhaseBefore(HttpRequestPipeline.Send, overrideAcceptHeaderPhase)
        client.requestPipeline.intercept(overrideAcceptHeaderPhase) {
            val acceptHeaderValue = context.overrideAcceptHeader ?: return@intercept
            context.headers[HttpHeaders.Accept] = acceptHeaderValue
        }

        val response = client.get("<https://api.github.com/repos/ktorio/ktor/readme>") {
            overrideAcceptHeader = readmeContentType
        }
    }

[Rustam Siniukov]

We can design something better in the future, but for now there is no clear use case. Stoping ContentNegotiation from adding headers when they are already present in the request is not a solution.

[Chris C]

Thanks for that, that works well as a workaround. However, I do think the way this works should be reconsidered for a few reasons:
• It’s very unexpected as the user of ktor that setting the accept header doesn’t actually result in that being the accept header sent. This is certainly not behaviour I could predict from the API
• I think it’s usually wrong to override the intentions of the user, especially when there’s no way to change that without writing a plugin
• There are plenty of other reasons a user might want to manipulate the accept header besides this Github oddity. For example, what if the client has both json and xml installed? If the server supports both, then the user won’t have a way to control which content type they receive
I take your point about not breaking the way it currently works 🙂

[Rustam Siniukov]

I agree with you that there is a problem that users can’t set headers and make sure that they are not modified by plugins. But I want to understand the bigger problem here. There are more headers that are manipulated by the plugins than just Accept header and there are more plugins that manipulate headers than just ContentNegotiation . Good solution should not be ad-hoc for just this one specific case.
Btw, point 3 can be solved with q parameter on ContentType

[Chris C]

Yes, I totally agree. Thanks for your time, I appreciate it