Skip to content

KTOR-7104 Fix saving caches with different vary header #4673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

marychatte
Copy link
Member

Subsystem
Client, Cache

Motivation
KTOR-7104
In the issue it's said:

Cache isn't updated when Vary header for 304 response matches but is not equal to Vary for 200 response (however, it's answering with cached response)

But actually, it shouldn't update the cache as well as it shouldn't respond with cached data because in documentation we can find the next:

The same Vary header value should be used on all responses for a given URL, including 304 Not Modified responses and the "default" response.

@marychatte marychatte self-assigned this Feb 13, 2025
@marychatte marychatte enabled auto-merge (squash) February 14, 2025 12:42
…pdated-when-Vary-header-for-304-response-matches-but-not-equal-to-Vary-for-200-response
@marychatte marychatte merged commit 7808fca into main Feb 14, 2025
13 of 17 checks passed
@marychatte marychatte deleted the marychatte/KTOR-7104-HttpCache-Cache-isn't-updated-when-Vary-header-for-304-response-matches-but-not-equal-to-Vary-for-200-response branch February 14, 2025 13:32
anton-erofeev pushed a commit to anton-erofeev/ktor that referenced this pull request Feb 23, 2025
e5l added a commit that referenced this pull request Mar 4, 2025
…'s HttpConfiguration object (#4387)

* KTOR-7934 Apply fix for Js target

* Update version to 3.1.1-SNAPSHOT (#4674)

* KTOR-7104 Fix saving caches with different vary header (#4673)

* Improve Jetty connection configuration by exposing a builder on the HttpConfiguration object

---------

Co-authored-by: Bruce Hamilton <bruce.hamilton@jetbrains.com>
Co-authored-by: Osip Fatkullin <osip.fatkullin@jetbrains.com>
Co-authored-by: Mariia Skripchenko <61115099+marychatte@users.noreply.github.com>
Co-authored-by: Leonid Stashevsky <e5l@users.noreply.github.com>
@StefMa
Copy link
Contributor

StefMa commented Mar 18, 2025

I'm not sure if this is the same problem as I face, but I have the feeling this didn't fixed the issue.
Here is a reproducer with 3.1.1 (where it should be fixed accroding to the release notes):

class VaryTest {

    private fun server() {
        embeddedServer(Netty, port = 4445) {
            routing {
                get("/cache") {
                    if (call.request.headers.contains("200")) {
                        call.response.header("Vary", "X-Requested-With,Accept-Encoding")
                        call.respondText { "200 OK response" }
                    } else {
                        call.response.header("Vary", "X-Requested-With")
                        call.respond(HttpStatusCode.NotModified, null)
                    }
                }
            }
        }.start(wait = false)
    }

    @Test
    fun name() = runTest {
        launch { server() }
        delay(1000) // Fake delay to start the server

        val storage = CacheStorage.Unlimited()
        val client = httpClient().config {
            install(HttpCache) {
                publicStorage(storage)
            }
        }

        client.get("http://localhost:4445/cache") {
            header("200", "")
            header("Accept-Encoding", "gzip,deflate")
        }
        println("First was successfully")
        client.get("http://localhost:4445/cache") {
            header("Accept-Encoding", "gzip,deflate")
        }
        println("We don't get here. Its crashing")

        val cache = storage.findAll(Url("http://localhost:4444/cache"))
        assert(cache.size == 1) { "Expected 1 but was ${cache.size}" }
    }
}

This result in:

First was successfully

The entry for url: http://localhost:4445/cache was removed from cache
io.ktor.client.plugins.cache.InvalidCacheStateException: The entry for url: http://localhost:4445/cache was removed from cache
	at io.ktor.client.plugins.cache.HttpCache$Companion$install$2.invokeSuspend(HttpCache.kt:240)
	at io.ktor.client.plugins.cache.HttpCache$Companion$install$2.invoke(HttpCache.kt)
	at io.ktor.client.plugins.cache.HttpCache$Companion$install$2.invoke(HttpCache.kt)
	at io.ktor.util.pipeline.DebugPipelineContext.proceedLoop(DebugPipelineContext.kt:79)
	at io.ktor.util.pipeline.DebugPipelineContext.proceed(DebugPipelineContext.kt:57)
	at io.ktor.util.pipeline.DebugPipelineContext.proceedWith(DebugPipelineContext.kt:42)
	at io.ktor.client.plugins.DoubleReceivePluginKt$SaveBodyPlugin$2$1.invokeSuspend(DoubleReceivePlugin.kt:78)
	at io.ktor.client.plugins.DoubleReceivePluginKt$SaveBodyPlugin$2$1.invoke(DoubleReceivePlugin.kt)
	at io.ktor.client.plugins.DoubleReceivePluginKt$SaveBodyPlugin$2$1.invoke(DoubleReceivePlugin.kt)
	at io.ktor.util.pipeline.DebugPipelineContext.proceedLoop(DebugPipelineContext.kt:79)
	at io.ktor.util.pipeline.DebugPipelineContext.proceed(DebugPipelineContext.kt:57)
	at io.ktor.util.pipeline.DebugPipelineContext.execute$ktor_utils(DebugPipelineContext.kt:63)
	at io.ktor.util.pipeline.Pipeline.execute(Pipeline.kt:92)
	at io.ktor.client.HttpClient$2.invokeSuspend(HttpClient.kt:1367)
	at io.ktor.client.HttpClient$2.invoke(HttpClient.kt)
	at io.ktor.client.HttpClient$2.invoke(HttpClient.kt)
	at io.ktor.util.pipeline.DebugPipelineContext.proceedLoop(DebugPipelineContext.kt:79)
	at io.ktor.util.pipeline.DebugPipelineContext.proceed(DebugPipelineContext.kt:57)
	at io.ktor.util.pipeline.DebugPipelineContext.proceedWith(DebugPipelineContext.kt:42)
	at io.ktor.client.engine.HttpClientEngine$install$1.invokeSuspend(HttpClientEngine.kt:166)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:100)
	at kotlinx.coroutines.test.TestDispatcher.processEvent$kotlinx_coroutines_test(TestDispatcher.kt:24)
	at kotlinx.coroutines.test.TestCoroutineScheduler.tryRunNextTaskUnless$kotlinx_coroutines_test(TestCoroutineScheduler.kt:99)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt$runTest$2$1$workRunner$1.invokeSuspend(TestBuilders.kt:326)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:100)
	at kotlinx.coroutines.EventLoopImplBase.processNextEvent(EventLoop.common.kt:263)
	at kotlinx.coroutines.BlockingCoroutine.joinBlocking(Builders.kt:95)
	at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking(Builders.kt:69)
	at kotlinx.coroutines.BuildersKt.runBlocking(Unknown Source)
	at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking$default(Builders.kt:47)
	at kotlinx.coroutines.BuildersKt.runBlocking$default(Unknown Source)
	at kotlinx.coroutines.test.TestBuildersJvmKt.createTestResult(TestBuildersJvm.kt:10)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTest-8Mi8wO0(TestBuilders.kt:309)
	at kotlinx.coroutines.test.TestBuildersKt.runTest-8Mi8wO0(Unknown Source)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTest-8Mi8wO0(TestBuilders.kt:167)
	at kotlinx.coroutines.test.TestBuildersKt.runTest-8Mi8wO0(Unknown Source)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTest-8Mi8wO0$default(TestBuilders.kt:159)
	at kotlinx.coroutines.test.TestBuildersKt.runTest-8Mi8wO0$default(Unknown Source)
	at VaryTest.name(VaryTest.kt:38)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.runTestClass(JUnitTestClassExecutor.java:112)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:58)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:40)
	at org.gradle.api.internal.tasks.testing.junit.AbstractJUnitTestClassProcessor.processTestClass(AbstractJUnitTestClassProcessor.java:54)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:53)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:92)
	at jdk.proxy1/jdk.proxy1.$Proxy4.processTestClass(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker$2.run(TestWorker.java:183)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:132)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:103)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:63)
	at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:121)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:71)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)

Dependencies:

+--- io.ktor:ktor-client-core:3.1.1
|    \--- io.ktor:ktor-client-core-jvm:3.1.1
|         +--- io.ktor:ktor-http:3.1.1
|         |    \--- io.ktor:ktor-http-jvm:3.1.1
|         |         +--- io.ktor:ktor-utils:3.1.1
|         |         |    \--- io.ktor:ktor-utils-jvm:3.1.1
|         |         |         +--- io.ktor:ktor-io:3.1.1
|         |         |         |    \--- io.ktor:ktor-io-jvm:3.1.1
|         +--- io.ktor:ktor-http-cio:3.1.1
|         |    \--- io.ktor:ktor-http-cio-jvm:3.1.1
|         |         +--- io.ktor:ktor-network:3.1.1
|         |         |    \--- io.ktor:ktor-network-jvm:3.1.1
|         |         |         +--- io.ktor:ktor-utils:3.1.1 (*)
|         |         +--- io.ktor:ktor-http:3.1.1 (*)
|         |         +--- io.ktor:ktor-io:3.1.1 (*)
|         +--- io.ktor:ktor-events:3.1.1
|         |    \--- io.ktor:ktor-events-jvm:3.1.1
|         |         +--- io.ktor:ktor-http:3.1.1 (*)
|         |         +--- io.ktor:ktor-utils:3.1.1 (*)
|         +--- io.ktor:ktor-websocket-serialization:3.1.1
|         |    \--- io.ktor:ktor-websocket-serialization-jvm:3.1.1
|         |         +--- io.ktor:ktor-http:3.1.1 (*)
|         |         +--- io.ktor:ktor-serialization:3.1.1
|         |         |    \--- io.ktor:ktor-serialization-jvm:3.1.1
|         |         |         +--- io.ktor:ktor-http:3.1.1 (*)
|         |         |         +--- io.ktor:ktor-websockets:3.1.1
|         |         |         |    \--- io.ktor:ktor-websockets-jvm:3.1.1
|         |         |         |         +--- io.ktor:ktor-http:3.1.1 (*)
|         +--- io.ktor:ktor-sse:3.1.1
|         |    \--- io.ktor:ktor-sse-jvm:3.1.1
|         |         +--- io.ktor:ktor-http:3.1.1 (*)
+--- io.ktor:ktor-serialization-kotlinx-json:3.1.1
|    \--- io.ktor:ktor-serialization-kotlinx-json-jvm:3.1.1
|         +--- io.ktor:ktor-http:3.1.1 (*)
|         +--- io.ktor:ktor-serialization-kotlinx:3.1.1
|         |    \--- io.ktor:ktor-serialization-kotlinx-jvm:3.1.1
|         |         +--- io.ktor:ktor-http:3.1.1 (*)
|         |         +--- io.ktor:ktor-serialization:3.1.1 (*)
+--- io.ktor:ktor-client-content-negotiation:3.1.1
|    \--- io.ktor:ktor-client-content-negotiation-jvm:3.1.1
|         +--- io.ktor:ktor-client-core:3.1.1 (*)
|         +--- io.ktor:ktor-serialization:3.1.1 (*)
+--- io.ktor:ktor-client-mock:3.1.1
|    \--- io.ktor:ktor-client-mock-jvm:3.1.1
|         +--- io.ktor:ktor-http:3.1.1 (*)
|         +--- io.ktor:ktor-client-core:3.1.1 (*)
+--- io.ktor:ktor-server-core:3.1.1
|    \--- io.ktor:ktor-server-core-jvm:3.1.1
|         +--- io.ktor:ktor-utils:3.1.1 (*)
|         +--- io.ktor:ktor-http:3.1.1 (*)
|         +--- io.ktor:ktor-serialization:3.1.1 (*)
|         +--- io.ktor:ktor-events:3.1.1 (*)
|         +--- io.ktor:ktor-http-cio:3.1.1 (*)
|         +--- io.ktor:ktor-websockets:3.1.1 (*)
+--- io.ktor:ktor-client-okhttp:3.1.1
|    \--- io.ktor:ktor-client-okhttp-jvm:3.1.1
|         +--- io.ktor:ktor-client-core:3.1.1 (*)
\--- io.ktor:ktor-server-netty:3.1.1
     \--- io.ktor:ktor-server-netty-jvm:3.1.1
          +--- io.ktor:ktor-server-core:3.1.1 (*)

@StefMa
Copy link
Contributor

StefMa commented Mar 18, 2025

nevermind, it works as expected.
The Vary header is not allowed to change 🙃
This is what the docs say.

Sorry for spamming 🙃

e5l added a commit that referenced this pull request Apr 17, 2025
…'s HttpConfiguration object (#4387)

* KTOR-7934 Apply fix for Js target

* Update version to 3.1.1-SNAPSHOT (#4674)

* KTOR-7104 Fix saving caches with different vary header (#4673)

* Improve Jetty connection configuration by exposing a builder on the HttpConfiguration object

---------

Co-authored-by: Bruce Hamilton <bruce.hamilton@jetbrains.com>
Co-authored-by: Osip Fatkullin <osip.fatkullin@jetbrains.com>
Co-authored-by: Mariia Skripchenko <61115099+marychatte@users.noreply.github.com>
Co-authored-by: Leonid Stashevsky <e5l@users.noreply.github.com>
osipxd added a commit that referenced this pull request May 7, 2025
…'s HttpConfiguration object (#4387)

* KTOR-7934 Apply fix for Js target

* Update version to 3.1.1-SNAPSHOT (#4674)

* KTOR-7104 Fix saving caches with different vary header (#4673)

* Improve Jetty connection configuration by exposing a builder on the HttpConfiguration object

---------

Co-authored-by: Bruce Hamilton <bruce.hamilton@jetbrains.com>
Co-authored-by: Osip Fatkullin <osip.fatkullin@jetbrains.com>
Co-authored-by: Mariia Skripchenko <61115099+marychatte@users.noreply.github.com>
Co-authored-by: Leonid Stashevsky <e5l@users.noreply.github.com>
bjhham added a commit that referenced this pull request May 7, 2025
…'s HttpConfiguration object (#4387)

* KTOR-7934 Apply fix for Js target

* Update version to 3.1.1-SNAPSHOT (#4674)

* KTOR-7104 Fix saving caches with different vary header (#4673)

* Improve Jetty connection configuration by exposing a builder on the HttpConfiguration object

---------

Co-authored-by: Bruce Hamilton <bruce.hamilton@jetbrains.com>
Co-authored-by: Osip Fatkullin <osip.fatkullin@jetbrains.com>
Co-authored-by: Mariia Skripchenko <61115099+marychatte@users.noreply.github.com>
Co-authored-by: Leonid Stashevsky <e5l@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants