KTOR-7104 Fix saving caches with different vary header

ktorio · Feb 13, 2025 · 2e0cee0 · 2e0cee0
1 parent bd652e4
commit 2e0cee0
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 5 deletions.
diff --git a/...ktor-client-core/common/src/io/ktor/client/plugins/cache/storage/UnlimitedCacheStorage.kt b/...ktor-client-core/common/src/io/ktor/client/plugins/cache/storage/UnlimitedCacheStorage.kt
@@ -23,7 +23,7 @@ internal class UnlimitedCacheStorage : HttpCacheStorage() {
     override fun find(url: Url, varyKeys: Map<String, String>): HttpCacheEntry? {
         val data = store.computeIfAbsent(url) { ConcurrentSet() }
         return data.find {
-            varyKeys.all { (key, value) -> it.varyKeys[key] == value }
+            varyKeys.all { (key, value) -> it.varyKeys[key] == value } && varyKeys.size == it.varyKeys.size
         }
     }
 
@@ -45,7 +45,7 @@ internal class UnlimitedStorage : CacheStorage {
     override suspend fun find(url: Url, varyKeys: Map<String, String>): CachedResponseData? {
         val data = store.computeIfAbsent(url) { ConcurrentSet() }
         return data.find {
-            varyKeys.all { (key, value) -> it.varyKeys[key] == value }
+            varyKeys.all { (key, value) -> it.varyKeys[key] == value } && varyKeys.size == it.varyKeys.size
         }
     }
 

diff --git a/...-client/ktor-client-core/jvm/src/io/ktor/client/plugins/cache/storage/FileCacheStorage.kt b/...-client/ktor-client-core/jvm/src/io/ktor/client/plugins/cache/storage/FileCacheStorage.kt
@@ -47,7 +47,7 @@ internal class CachingCacheStorage(
         }
         val data = store.getValue(url)
         return data.find {
-            varyKeys.all { (key, value) -> it.varyKeys[key] == value }
+            varyKeys.all { (key, value) -> it.varyKeys[key] == value } && varyKeys.size == it.varyKeys.size
         }
     }
 
@@ -83,7 +83,7 @@ private class FileCacheStorage(
     override suspend fun find(url: Url, varyKeys: Map<String, String>): CachedResponseData? {
         val data = readCache(key(url))
         return data.find {
-            varyKeys.all { (key, value) -> it.varyKeys[key] == value }
+            varyKeys.all { (key, value) -> it.varyKeys[key] == value } && varyKeys.size == it.varyKeys.size
         }
     }
 

diff --git a/ktor-client/ktor-client-core/jvm/test/CachingCacheStorageTest.kt b/ktor-client/ktor-client-core/jvm/test/CachingCacheStorageTest.kt
@@ -99,7 +99,7 @@ private class InMemoryCacheStorage : CacheStorage {
         findCalledCount++
         val cache = store.computeIfAbsent(url) { mutableSetOf() }
         return cache.find {
-            varyKeys.all { (key, value) -> it.varyKeys[key] == value }
+            varyKeys.all { (key, value) -> it.varyKeys[key] == value } && varyKeys.size == it.varyKeys.size
         }
     }
 

diff --git a/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/plugins/CacheTest.kt b/ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/plugins/CacheTest.kt
@@ -836,6 +836,28 @@ class CacheTest : ClientLoader() {
         }
     }
 
+    @Test
+    fun testDifferentVaryHeaders() = clientTests {
+        val storage = CacheStorage.Unlimited()
+        config {
+            install(HttpCache) {
+                publicStorage(storage)
+            }
+        }
+
+        test { client ->
+            client.get("$TEST_SERVER/cache/different-vary") {
+                header("200", "true")
+                header("Set-Vary", "X-Requested-With,Accept-Encoding")
+            }
+            assertFailsWith<InvalidCacheStateException> {
+                client.get("$TEST_SERVER/cache/different-vary") {
+                    header("Set-Vary", "X-Requested-With")
+                }
+            }
+        }
+    }
+
     /**
      * Does delay and ensures that the [GMTDate] measurements report at least
      * the specified number of [milliseconds].

diff --git a/ktor-test-server/src/main/kotlin/test/server/tests/Cache.kt b/ktor-test-server/src/main/kotlin/test/server/tests/Cache.kt
@@ -118,6 +118,15 @@ internal fun Application.cacheTestServer() {
                 call.response.header(HttpHeaders.CacheControl, "max-age: 120")
                 call.respond(HttpStatusCode.OK)
             }
+            get("/different-vary") {
+                if (call.request.headers.contains("200")) {
+                    call.response.header("Vary", "X-Requested-With,Accept-Encoding")
+                    call.respond(HttpStatusCode.OK)
+                } else {
+                    call.response.header("Vary", "X-Requested-With")
+                    call.respond(HttpStatusCode.NotModified)
+                }
+            }
         }
     }
 }