Seems when we combine conditional headers for etag...

# ktor

[Anders]

Seems when we combine conditional headers for etag caching and f.ex the compression plugin, ktor wont automatically add correct Vary headers to cached responses (304) which makes some clients evict / fail caching because the backend response does not adhere to the RFC, so weve had to handle this manually. Shouldnt ktor keep track of its own plugins and add Vary headers where necessary? In addition, what is the correct way to handle InvalidCacheStateException, weve ended up with a verbose solution to delete that cache entry and remove the if none match header, because the httpcache plugin would simply set that header if we didnt predelete the cache entry. What exaclty does this exception mean - and why isnt it handle by the ktor client itself?

[Aleksei Tirman [JB]]

The InvalidCacheStateException is thrown when the client revalidates a stale entry with an If-None-Match request, the server responds 304 Not Modified, but the cached response can no longer be found.
Can you please share a code snippet to reproduce the problem with the Compression plugin?

[Anders]

InvalidCacheStateException

Shouldnt ktor itself automatically do a retry by evicting the cache? Now we need to do this silliness, unless you have a better idea:

private val publicCacheStorage = InvalidatingCacheStorage(CacheStorage.Unlimited())
private val privateCacheStorage = InvalidatingCacheStorage(CacheStorage.Unlimited())


install(HttpCache) {
    publicStorage(publicCacheStorage)
    privateStorage(privateCacheStorage)
}

modifyRequest { request ->
    val cacheRelated =
        cause is InvalidCacheStateException ||
                cause?.cause is InvalidCacheStateException ||
                cause?.isCacheIssue() == true
    if (cacheRelated) {
        val url = request.url.build()
        publicCacheStorage.markInvalid(url)
        privateCacheStorage.markInvalid(url)
        request.headers.remove(IfNoneMatch)
        request.headers.remove(IfModifiedSince)
    }
}

If we simply remove the header it is ironically readded by the HttpCache plugin, if we dont control the cache we cant remove the invalid cache (so we cant force remove if-none-match. Is there a better way? This seems unecessary.

For the compression stuff, just do this:

install(ConditionalHeaders) {
    version { call, outgoingContent ->
        when (outgoingContent) {
            is OutgoingContent.ByteArrayContent,
            is OutgoingContent.ReadChannelContent,
            -> {
                val bytes =
                    when (outgoingContent) {
                        is OutgoingContent.ByteArrayContent -> {
                            outgoingContent.bytes()
                        }

                        is OutgoingContent.ReadChannelContent -> {
                            outgoingContent.readFrom().readRemaining().readByteArray()
                        }
                    }

                val etag = generateWeakETag(bytes)
                listOf(EntityTagVersion(etag))
            }

            else -> {
                emptyList()
            }
        }
    }
}
install(Compression) {
    default()
    excludeContentType(EventStream)
    excludeContentType(Zip)
    minimumSize(1024)
}

The first request will have vary accept-encoding, the 304 will not, i presume because the body is null and the compression plugin is never triggered.

[Aleksei Tirman [JB]]

Ktor could reissue the request unconditionally as an alternative strategy to throwing the exception. Can you please describe the expected and the actual behavior for the case with the Compression plugin? I am trying to reproduce it with the following code:

val client = HttpClient(OkHttp) {
    install(HttpCache)
}

client.get("<http://localhost:8085>").let { response ->
    println(response.status)
    for ((name, values) in response.headers.entries()) {
        println("$name: $values")
    }
    println(response.bodyAsBytes().size)
}
client.get("<http://localhost:8085>").let { response ->
    println(response.status)
    for ((name, values) in response.headers.entries()) {
        println("$name: $values")
    }
    println(response.bodyAsBytes().size)
}
client.close()

[Anders]

When you do the initial request you should get Vary: Origin and Vary: Accept-Encoding , as far as I can tell you get none of them (but the RFC expects a Vary: Accept-Encoding) if the client requests that. So the logical solution is something like this:

fun Application.module() {
    install(DefaultHeaders) {
        header(HttpHeaders.Vary, HttpHeaders.Origin)
        header(HttpHeaders.Vary, HttpHeaders.AcceptEncoding)
    }
}

However when the response is sent it will only display HttpHeaders.Origin. I dont know where this is overriden but i presume it must be one of the content plugins. Both 200 and 304 omit this. This causes the RFC issue, we first noticed it on iOS which is quite rigid about it.

So what we did is this:

val VaryAcceptEncodingPlugin =
    createApplicationPlugin("VaryAcceptEncoding") {
        onCallRespond { call ->
            val acceptEncoding = "Accept-Encoding"
            val alreadySet =
                call.response.headers
                    .values(Vary)
                    .any { value ->
                        value.split(",").any {
                            it.trim().equals(acceptEncoding, ignoreCase = true)
                        }
                    }

            if (!alreadySet) {
                call.response.header(Vary, acceptEncoding)
            }
        }
    }

That triggers right before respond to force this behaviour. In this case we force ktor to return Accept-Encoding also for 304's, it seems to add it automatically for 200's. It actually seems to produce a duplicate:

vary
Origin, Accept-Encoding, Accept-Encoding

But that might be something with our application.

If you do none of these things. but only add the conditionalheaders plugin the first response (200) will correctly get the Vary: Origin, Accept-Encoding but the second (304) will only have Vary: Origin

[Anders]

"Ktor could reissue the request unconditionally as an alternative strategy to throwing the exception." - The exception is thrown internally from ktor, how do we configure it otherwise?

[Anders]

If i make an attempt to avoid that duplicate, it doesnt return it at all in the response. Bit confounded by that haha.

[Anders]

This ended up being a more proper, albeit more verbose solution:

import io.ktor.http.Headers
import io.ktor.http.HttpHeaders.Vary
import io.ktor.http.content.OutgoingContent
import io.ktor.server.application.call
import io.ktor.server.application.createApplicationPlugin
import io.ktor.server.response.ApplicationSendPipeline
import io.ktor.server.response.header
import io.ktor.utils.io.ByteReadChannel

val VaryAcceptEncodingPlugin =
    createApplicationPlugin("VaryAcceptEncoding") {
        application.sendPipeline.intercept(ApplicationSendPipeline.After) {
            val acceptEncoding = "Accept-Encoding"

            // Always add to response-level headers so the value survives when ConditionalHeaders
            // replaces the body with HttpStatusCodeContent (304), which discards content headers.
            // (ResponseBodyReadyForSend / ConditionalHeaders also runs at After, after us, and
            // content.headers are lost once the body is replaced.)
            if (!call.response.headers.values(Vary).hasAcceptEncoding()) {
                call.response.header(Vary, acceptEncoding)
            }

            // Compression at ContentEncoding adds Vary: Accept-Encoding to OutgoingContent.headers.
            // Strip it from there so the HTTP response doesn't carry it twice (once from above,
            // once from the compressed content).
            val content = subject as? OutgoingContent.ReadChannelContent ?: return@intercept

            if (content.headers.getAll(Vary).orEmpty().hasAcceptEncoding()) {
                proceedWith(ContentWithFilteredVary(content, acceptEncoding))
            }
        }
    }

private fun List<String>.hasAcceptEncoding(): Boolean =
    flatMap { it.split(",") }.any { it.trim().equals("Accept-Encoding", ignoreCase = true) }

private class ContentWithFilteredVary(
    private val delegate: ReadChannelContent,
    private val filteredToken: String,
) : OutgoingContent.ReadChannelContent() {
    override val contentType get() = delegate.contentType
    override val contentLength get() = delegate.contentLength
    override val status get() = delegate.status
    override val headers: Headers by lazy {
        Headers.build {
            delegate.headers.forEach { name, values ->
                when {
                    name.equals(Vary, ignoreCase = true) -> {
                        val filtered = values.flatMap { it.split(",") }
                            .map { it.trim() }
                            .filterNot { it.equals(filteredToken, ignoreCase = true) }
                        
                        if (filtered.isNotEmpty()) append(name, filtered.joinToString(", "))
                    }
                    
                    else -> {
                        appendAll(name, values)
                    }
                }
            }
        }
    }

    override fun readFrom(): ByteReadChannel = delegate.readFrom()
}

[Aleksei Tirman [JB]]

I have the following server configuration (based on the code you described and without the DefaultRequest plugin):

fun main() {
    embeddedServer(Netty, 8085) {
        install(ConditionalHeaders) {
            version { call, outgoingContent ->
                when (outgoingContent) {
                    is OutgoingContent.ByteArrayContent,
                    is OutgoingContent.ReadChannelContent,
                        -> {
                        val bytes =
                            when (outgoingContent) {
                                is OutgoingContent.ByteArrayContent -> {
                                    outgoingContent.bytes()
                                }

                                is OutgoingContent.ReadChannelContent -> {
                                    outgoingContent.readFrom().readRemaining().readByteArray()
                                }
                            }

                        val etag = generateWeakETag(bytes)
                        listOf(EntityTagVersion(etag))
                    }

                    else -> {
                        emptyList()
                    }
                }
            }
        }
        install(Compression) {
            default()
            excludeContentType(EventStream)
            excludeContentType(Zip)
            minimumSize(1024)
        }
        routing {
            get {
               call.respond(ByteArrayContent("a".repeat(10 * 1024).toByteArray(), ContentType.Text.Plain))
            }
        }
    }.start(wait = true)
}

fun generateWeakETag(bytes: ByteArray): String {
    val hash = MessageDigest.getInstance("SHA-1").digest(bytes)
    return "W/\"" + hash.joinToString("") { "%02x".format(it) } + "\""
}

The server responds 200 for both requests and the following headers:

connection: [keep-alive]
content-type: [text/plain]
etag: [W/"4916db897c2fdba6fdc226b7db136332b44e60d4"]
transfer-encoding: [chunked]
vary: [Accept-Encoding]

[Anders]

Do you pass in If-None-Match for the second request to trigger a 304? Thats when you miss the Accept-Encoding

[Aleksei Tirman [JB]]

Should I send the Etag from the first response as the value for the If-None-Match header?

[Anders]

Yes

[Aleksei Tirman [JB]]

The same responses for the following client code:

val client = HttpClient(OkHttp) {
    install(HttpCache)
}

val etag = client.get("<http://localhost:8085>").let { response ->
    println(response.status)
    for ((name, values) in response.headers.entries()) {
        println("$name: $values")
    }
    println(response.bodyAsBytes().size)
    response.headers[HttpHeaders.ETag]
}
client.get("<http://localhost:8085>") {
    etag?.let {
        header("If-None-Match", it)
        println("ETag set to $it")
    }
}.let { response ->
    println(response.status)
    for ((name, values) in response.headers.entries()) {
        println("$name: $values")
    }
    println(response.bodyAsBytes().size)
}

The output:

200 OK
connection: [keep-alive]
content-type: [text/plain]
etag: [W/"4916db897c2fdba6fdc226b7db136332b44e60d4"]
transfer-encoding: [chunked]
vary: [Accept-Encoding]
10240
ETag set to W/"4916db897c2fdba6fdc226b7db136332b44e60d4"


200 OK
connection: [keep-alive]
content-type: [text/plain]
etag: [W/"4916db897c2fdba6fdc226b7db136332b44e60d4"]
transfer-encoding: [chunked]
vary: [Accept-Encoding]
10240

[Anders]

Use application/json as content-type. Just pass {"foo":"bar"} or something

[Aleksei Tirman [JB]]

Shouldn't the response body be more than 1024 bytes because of the minimumSize(1024) in the Compression plugin configuration?

[Anders]

Hm, yes, that makes sense

[Aleksei Tirman [JB]]

Unfortunately, I cannot reproduce the problem when the server responds with a large JSON payload:

get {
   call.respond(ByteArrayContent(("\"" + "a".repeat(10 * 1024) + "\"").toByteArray(), ContentType.Application.Json))
}

[Anders]

hm 🤔