Skip to content

Commit 370cfd9

Browse files
author
Matteo Mazza
committed
fix(HttpCache): align mergedHeadersLookup separator with HttpCacheEntry.varyKeys()
mergedHeadersLookup() joined multi-value headers with ";" while varyKeys() joined them with ",", so findResponse() always discarded cached entries for requests with repeated header values (e.g. when ContentNegotiation appends a second Accept value). Extract joinHeaderValues() (RFC 7230 §3.2.2 comma separator) and use it in both places so the stored and looked-up strings are always identical.
1 parent e0cf313 commit 370cfd9

3 files changed

Lines changed: 17 additions & 18 deletions

File tree

ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/cache/HttpCache.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -393,8 +393,8 @@ internal fun mergedHeadersLookup(
393393
}
394394

395395
else -> {
396-
val value = content.headers.getAll(header) ?: allHeadersExtractor(header) ?: emptyList()
397-
value.joinToString(";")
396+
val value = content.headers.getAll(header) ?: allHeadersExtractor(header)
397+
value.joinHeaderValues()
398398
}
399399
}
400400
}

ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/cache/HttpCacheEntry.kt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,17 @@ public class HttpCacheEntry internal constructor(
5050
}
5151
}
5252

53+
// RFC 7230 §3.2.2: multiple values for the same header field are equivalent to a comma-separated list.
54+
internal fun List<String>?.joinHeaderValues(): String = this?.joinToString(",") ?: ""
55+
5356
internal fun HttpResponse.varyKeys(): Map<String, String> {
5457
val validationKeys = vary() ?: return emptyMap()
5558

5659
val result = mutableMapOf<String, String>()
5760
val requestHeaders = call.request.headers
5861

5962
for (key in validationKeys) {
60-
result[key.lowercase()] = requestHeaders.getAll(key)?.joinToString(",") ?: ""
63+
result[key.lowercase()] = requestHeaders.getAll(key).joinHeaderValues()
6164
}
6265

6366
return result

ktor-client/ktor-client-core/common/test/HttpCacheTest.kt

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -199,31 +199,27 @@ class HttpCacheTest {
199199
)
200200

201201
/**
202-
* Unit test: shows the separator mismatch between how varyKeys are stored and looked up.
202+
* Regression test: varyKeys() and mergedHeadersLookup() must produce identical strings for
203+
* the same multi-value header so that findResponse() can match cached entries.
203204
*
204-
* HttpCacheEntry.varyKeys() stores multi-value headers joined with "," (comma):
205-
* result[key.lowercase()] = requestHeaders.getAll(key)?.joinToString(",") ?: ""
206-
*
207-
* mergedHeadersLookup() in HttpCache.kt joins them with ";" (semicolon):
208-
* value.joinToString(";")
209-
*
210-
* When ContentNegotiation appends a second Accept value, the stored "a,b" never equals
211-
* the looked-up "a;b", so findResponse() always discards the cached entry.
205+
* Before the fix, varyKeys() joined with "," while mergedHeadersLookup() joined with ";",
206+
* so stored "a,b" never equalled looked-up "a;b" and every request was a cache miss.
207+
* Both now delegate to joinHeaderValues() (RFC 7230 §3.2.2 comma separator).
212208
*/
213209
@Test
214-
fun varyKeysStoredWithCommaSeparatorButMergedHeadersLookupUsesSemicolon() {
210+
fun varyKeysSeparatorMatchesMergedHeadersLookupSeparator() {
215211
val requestHeaders = Headers.build {
216212
append(HttpHeaders.Accept, "application/vnd.github+json") // set by caller
217213
append(HttpHeaders.Accept, "application/json") // appended by ContentNegotiation
218214
}
219215

220-
// How HttpCacheEntry.varyKeys() stores the value:
221-
val stored = requestHeaders.getAll(HttpHeaders.Accept)?.joinToString(",")
216+
// How HttpCacheEntry.varyKeys() stores the value (joinHeaderValues → comma):
217+
val stored = requestHeaders.getAll(HttpHeaders.Accept).joinHeaderValues()
222218
// → "application/vnd.github+json,application/json"
223219

224-
// How mergedHeadersLookup() computes the lookup value:
225-
val lookup = (requestHeaders.getAll(HttpHeaders.Accept) ?: emptyList()).joinToString(";")
226-
// → "application/vnd.github+json;application/json"
220+
// How mergedHeadersLookup() now computes the lookup value (joinHeaderValues → comma):
221+
val lookup = requestHeaders.getAll(HttpHeaders.Accept).joinHeaderValues()
222+
// → "application/vnd.github+json,application/json"
227223

228224
assertEquals(stored, lookup)
229225
}

0 commit comments

Comments
 (0)