Skip to content

Commit

Permalink
KTOR-7483 Avoid caching requests with Authorization header (#4337)
Browse files Browse the repository at this point in the history
* KTOR-7483 Avoid caching requests with Authorization header
  • Loading branch information
e5l authored Sep 24, 2024
1 parent fa90875 commit d6c3a51
Show file tree
Hide file tree
Showing 6 changed files with 153 additions and 4 deletions.
4 changes: 3 additions & 1 deletion ktor-client/ktor-client-core/api/ktor-client-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ public final class io/ktor/client/plugins/api/TransformResponseBodyContext {

public final class io/ktor/client/plugins/cache/HttpCache {
public static final field Companion Lio/ktor/client/plugins/cache/HttpCache$Companion;
public synthetic fun <init> (Lio/ktor/client/plugins/cache/storage/HttpCacheStorage;Lio/ktor/client/plugins/cache/storage/HttpCacheStorage;Lio/ktor/client/plugins/cache/storage/CacheStorage;Lio/ktor/client/plugins/cache/storage/CacheStorage;ZZLkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (Lio/ktor/client/plugins/cache/storage/HttpCacheStorage;Lio/ktor/client/plugins/cache/storage/HttpCacheStorage;Lio/ktor/client/plugins/cache/storage/CacheStorage;Lio/ktor/client/plugins/cache/storage/CacheStorage;ZZZLkotlin/jvm/internal/DefaultConstructorMarker;)V
}

public final class io/ktor/client/plugins/cache/HttpCache$Companion : io/ktor/client/plugins/HttpClientPlugin {
Expand All @@ -612,11 +612,13 @@ public final class io/ktor/client/plugins/cache/HttpCache$Companion : io/ktor/cl

public final class io/ktor/client/plugins/cache/HttpCache$Config {
public fun <init> ()V
public final fun getCacheRequestWithAuth ()Z
public final fun getPrivateStorage ()Lio/ktor/client/plugins/cache/storage/HttpCacheStorage;
public final fun getPublicStorage ()Lio/ktor/client/plugins/cache/storage/HttpCacheStorage;
public final fun isShared ()Z
public final fun privateStorage (Lio/ktor/client/plugins/cache/storage/CacheStorage;)V
public final fun publicStorage (Lio/ktor/client/plugins/cache/storage/CacheStorage;)V
public final fun setCacheRequestWithAuth (Z)V
public final fun setPrivateStorage (Lio/ktor/client/plugins/cache/storage/HttpCacheStorage;)V
public final fun setPublicStorage (Lio/ktor/client/plugins/cache/storage/HttpCacheStorage;)V
public final fun setShared (Z)V
Expand Down
3 changes: 3 additions & 0 deletions ktor-client/ktor-client-core/api/ktor-client-core.klib.api
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,9 @@ final class io.ktor.client.plugins.cache/HttpCache { // io.ktor.client.plugins.c
final class Config { // io.ktor.client.plugins.cache/HttpCache.Config|null[0]
constructor <init>() // io.ktor.client.plugins.cache/HttpCache.Config.<init>|<init>(){}[0]

final var cacheRequestWithAuth // io.ktor.client.plugins.cache/HttpCache.Config.cacheRequestWithAuth|{}cacheRequestWithAuth[0]
final fun <get-cacheRequestWithAuth>(): kotlin/Boolean // io.ktor.client.plugins.cache/HttpCache.Config.cacheRequestWithAuth.<get-cacheRequestWithAuth>|<get-cacheRequestWithAuth>(){}[0]
final fun <set-cacheRequestWithAuth>(kotlin/Boolean) // io.ktor.client.plugins.cache/HttpCache.Config.cacheRequestWithAuth.<set-cacheRequestWithAuth>|<set-cacheRequestWithAuth>(kotlin.Boolean){}[0]
final var isShared // io.ktor.client.plugins.cache/HttpCache.Config.isShared|{}isShared[0]
final fun <get-isShared>(): kotlin/Boolean // io.ktor.client.plugins.cache/HttpCache.Config.isShared.<get-isShared>|<get-isShared>(){}[0]
final fun <set-isShared>(kotlin/Boolean) // io.ktor.client.plugins.cache/HttpCache.Config.isShared.<set-isShared>|<set-isShared>(kotlin.Boolean){}[0]
Expand Down
1 change: 1 addition & 0 deletions ktor-client/ktor-client-core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ kotlin.sourceSets {
dependencies {
api(project(":ktor-test-dispatcher"))
api(project(":ktor-client:ktor-client-mock"))
api(project(":ktor-server:ktor-server-test-host"))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ public class HttpCache private constructor(
private val publicStorageNew: CacheStorage,
private val privateStorageNew: CacheStorage,
private val useOldStorage: Boolean,
internal val isSharedClient: Boolean
internal val isSharedClient: Boolean,
internal val cacheRequestWithAuth: Boolean
) {
/**
* A configuration for the [HttpCache] plugin.
Expand All @@ -61,6 +62,17 @@ public class HttpCache private constructor(
internal var privateStorageNew: CacheStorage = CacheStorage.Unlimited()
internal var useOldStorage = false

/**
* Specifies if requests with Authorization header should be cached.
*
* According to specification, enabling this flag has security implications.
* See https://datatracker.ietf.org/doc/html/rfc2616#section-14.8,
* https://datatracker.ietf.org/doc/html/rfc7234#section-3,
* and https://datatracker.ietf.org/doc/html/rfc9111#section-3 for the details
*/
@Deprecated("Changing this flag has security implication", level = DeprecationLevel.WARNING)
public var cacheRequestWithAuth: Boolean = false

/**
* Specifies if the client where this plugin is installed is shared among multiple users.
* When set to true, all responses with `private` Cache-Control directive will not be cached.
Expand Down Expand Up @@ -138,7 +150,8 @@ public class HttpCache private constructor(
publicStorageNew = publicStorageNew,
privateStorageNew = privateStorageNew,
useOldStorage = useOldStorage,
isSharedClient = isShared
isSharedClient = isShared,
cacheRequestWithAuth = cacheRequestWithAuth
)
}
}
Expand All @@ -152,6 +165,10 @@ public class HttpCache private constructor(
if (content !is OutgoingContent.NoContent) return@intercept
if (context.method != HttpMethod.Get || !context.url.protocol.canStore()) return@intercept

if (!plugin.cacheRequestWithAuth && context.headers.contains(HttpHeaders.Authorization)) {
return@intercept
}

if (plugin.useOldStorage) {
interceptSendLegacy(plugin, content, scope)
return@intercept
Expand Down
127 changes: 127 additions & 0 deletions ktor-client/ktor-client-core/jvm/test/HttpCacheTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
/*
* Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
*/

import io.ktor.client.plugins.cache.*
import io.ktor.client.request.*
import io.ktor.client.statement.*
import io.ktor.http.*
import io.ktor.server.response.*
import io.ktor.server.routing.*
import io.ktor.server.testing.*
import kotlinx.coroutines.*
import kotlin.test.*
import kotlin.time.Duration.Companion.milliseconds

class HttpCacheTest {

@Test
fun `should not mix ETags when Authorization header is present`() = testApplication {
application {
routing {
get("/me") {
val user = call.request.headers["Authorization"]!!
if (user == "user-a") {
// Simulate slower network for one of the requests
delay(100.milliseconds)
}
val etag = "etag-of-$user"
if (call.request.headers["If-None-Match"] == etag) {
call.respond(HttpStatusCode.NotModified)
return@get
}
call.response.header("Cache-Control", "no-cache")
call.response.header("ETag", etag)
call.respondText(user)
}
}
}

val client = createClient {
install(HttpCache) {
isShared = true
}
}

assertEquals(
client.get("/me") {
headers["Authorization"] = "user-a"
}.bodyAsText(),
"user-a"
)
withContext(Dispatchers.Default) {
listOf(
launch {
val response = client.get("/me") {
headers["Authorization"] = "user-a"
}.bodyAsText()

assertEquals("user-a", response)
},
launch {
val response = client.get("/me") {
headers["Authorization"] = "user-b"
}.bodyAsText()

assertEquals("user-b", response)
}
).joinAll()
}
}

@Test
fun `should mix ETags when Authorization header is present and flag enabled`() = testApplication {
application {
routing {
get("/me") {
val user = call.request.headers["Authorization"]!!
if (user == "user-a") {
// Simulate slower network for one of the requests
delay(100.milliseconds)
}
val etag = "etag-of-$user"
if (call.request.headers["If-None-Match"] == etag) {
call.respond(HttpStatusCode.NotModified)
return@get
}
call.response.header("Cache-Control", "no-cache")
call.response.header("ETag", etag)
call.respondText(user)
}
}
}

val client = createClient {
install(HttpCache) {
isShared = true
@Suppress("DEPRECATION")
cacheRequestWithAuth = true
}
}

assertEquals(
client.get("/me") {
headers["Authorization"] = "user-a"
}.bodyAsText(),
"user-a"
)
withContext(Dispatchers.Default) {
listOf(
launch {
val response = client.get("/me") {
headers["Authorization"] = "user-a"
}.bodyAsText()

assertEquals("user-b", response)
},
launch {
val response = client.get("/me") {
headers["Authorization"] = "user-b"
}.bodyAsText()

assertEquals("user-b", response)
}
).joinAll()
}
}
}
1 change: 0 additions & 1 deletion ktor-io/common/test/ByteReadChannelOperationsTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -82,5 +82,4 @@ class ByteReadChannelOperationsTest {
assertContentEquals(expected.copyOfRange(0, 5), actual.copyOfRange(3, 8))
assertContentEquals(ByteArray(2) { 0 }, actual.copyOfRange(8, 10))
}

}

0 comments on commit d6c3a51

Please sign in to comment.