Improve Jetty connection configuration by exposing a builder on Jetty…

…'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>
ktorio · Mar 4, 2025 · e0742be · e0742be
1 parent 1242eb2
commit e0742be
Showing 16 changed files with 214 additions and 17 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/ktor-client/ktor-client-core/js/src/io/ktor/client/engine/js/JsClientEngine.kt b/ktor-client/ktor-client-core/js/src/io/ktor/client/engine/js/JsClientEngine.kt
@@ -156,7 +156,11 @@ private fun org.w3c.fetch.Headers.mapToKtor(method: HttpMethod, attributes: Attr
         append(key, value)
     }
 
-    dropCompressionHeaders(method, attributes)
+    dropCompressionHeaders(
+        method,
+        attributes,
+        alwaysRemove = PlatformUtils.IS_BROWSER
+    )
 }
 
 /**

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/...ient/ktor-client-tests/common/test/io/ktor/client/tests/ContentEncodingIntegrationTest.kt b/...ient/ktor-client-tests/common/test/io/ktor/client/tests/ContentEncodingIntegrationTest.kt
@@ -13,20 +13,27 @@ import io.ktor.utils.io.*
 import kotlinx.io.readString
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertNull
 
 private const val TEST_URL = "$TEST_SERVER/compression"
 
 class ContentEncodingIntegrationTest : ClientLoader() {
 
-    // GZipEncoder is implemented only on JVM.
+    // GZipEncoder is implemented only on JVM; implicitly decoded for browser
     @Test
     fun testGzipWithContentLengthWithoutPlugin() = clientTests(only("jvm:*", "web:Js")) {
         test { client ->
             val response = client.get("$TEST_URL/gzip-with-content-length")
-            val content = if (response.headers[HttpHeaders.ContentEncoding] == "gzip") {
-                GZipEncoder.decode(response.bodyAsChannel()).readRemaining().readString()
-            } else {
-                response.bodyAsText()
+            val content = when (response.headers[HttpHeaders.ContentEncoding]) {
+                "gzip" -> GZipEncoder.decode(response.bodyAsChannel()).readRemaining().readString()
+                null -> {
+                    // Content-Length should be removed for browser
+                    if (PlatformUtils.IS_BROWSER) {
+                        assertNull(response.headers[HttpHeaders.ContentLength])
+                    }
+                    response.bodyAsText()
+                }
+                else -> error("Unexpected content encoding: ${response.headers[HttpHeaders.ContentEncoding]}")
             }
 
             assertEquals("Hello, world", content)

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-server/ktor-server-jetty-jakarta/api/ktor-server-jetty-jakarta.api b/ktor-server/ktor-server-jetty-jakarta/api/ktor-server-jetty-jakarta.api
@@ -41,8 +41,10 @@ public class io/ktor/server/jetty/jakarta/JettyApplicationEngineBase : io/ktor/s
 public final class io/ktor/server/jetty/jakarta/JettyApplicationEngineBase$Configuration : io/ktor/server/engine/BaseApplicationEngine$Configuration {
 	public fun <init> ()V
 	public final fun getConfigureServer ()Lkotlin/jvm/functions/Function1;
+	public final fun getHttpConfiguration ()Lkotlin/jvm/functions/Function1;
 	public final fun getIdleTimeout-UwyO8pc ()J
 	public final fun setConfigureServer (Lkotlin/jvm/functions/Function1;)V
+	public final fun setHttpConfiguration (Lkotlin/jvm/functions/Function1;)V
 	public final fun setIdleTimeout-LRDsOJo (J)V
 }
 

diff --git a/...r-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyApplicationEngineBase.kt b/...r-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyApplicationEngineBase.kt
@@ -43,6 +43,12 @@ public open class JettyApplicationEngineBase(
          */
         public var configureServer: Server.() -> Unit = {}
 
+        /**
+         * Property function that will be called during Jetty server initialization with the http configuration instance
+         * that is passed to the managed connectors as a receiver.
+         */
+        public var httpConfiguration: HttpConfiguration.() -> Unit = {}
+
         /**
          * The duration of time that a connection can be idle before the connector takes action to close the connection.
          *
@@ -57,8 +63,8 @@ public open class JettyApplicationEngineBase(
      * Jetty server instance being configuring and starting
      */
     protected val server: Server = Server().apply {
-        configuration.configureServer(this)
         initializeServer(configuration)
+        configuration.configureServer(this)
     }
 
     override fun start(wait: Boolean): JettyApplicationEngineBase {

diff --git a/...erver/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/ServerInitializer.kt b/...erver/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/ServerInitializer.kt
@@ -23,7 +23,8 @@ internal fun Server.initializeServer(
             if (ktorConnector.type == ConnectorType.HTTPS) {
                 addCustomizer(SecureRequestCustomizer())
             }
-        }
+
+        }.apply(configuration.httpConfiguration)
 
         var alpnAvailable = false
         var alpnConnectionFactory: ALPNServerConnectionFactory?

diff --git a/.../ktor-server-jetty-jakarta/jvm/test/io/ktor/tests/server/jetty/jakarta/JettyEngineTest.kt b/.../ktor-server-jetty-jakarta/jvm/test/io/ktor/tests/server/jetty/jakarta/JettyEngineTest.kt
@@ -43,14 +43,14 @@ class JettyHttpServerJvmTest : HttpServerJvmTestSuite<JettyApplicationEngine, Je
     @Test
     fun testServletAttributes() = runTest {
         createAndStartServer {
-            get("/tomcat/attributes") {
+            get("/jetty/attributes") {
                 call.respondText(
                     call.request.servletRequestAttributes["ktor.test.attribute"]?.toString() ?: "Not found"
                 )
             }
         }
 
-        withUrl("/tomcat/attributes", {}) {
+        withUrl("/jetty/attributes", {}) {
             assertEquals("135", call.response.bodyAsText())
         }
     }

diff --git a/...r-jetty-jakarta/jvm/test/io/ktor/tests/server/jetty/jakarta/JettyHttpConfigurationTest.kt b/...r-jetty-jakarta/jvm/test/io/ktor/tests/server/jetty/jakarta/JettyHttpConfigurationTest.kt
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
+ */
+
+package io.ktor.tests.server.jetty.jakarta
+
+import io.ktor.client.*
+import io.ktor.client.request.*
+import io.ktor.http.*
+import io.ktor.server.engine.*
+import io.ktor.server.jetty.jakarta.*
+import io.ktor.server.response.*
+import io.ktor.server.routing.*
+import kotlinx.coroutines.test.*
+import org.junit.jupiter.params.*
+import org.junit.jupiter.params.provider.*
+import java.net.*
+import kotlin.random.*
+import kotlin.test.*
+
+class JettyHttpConfigurationTest {
+
+    private fun findFreePort() = ServerSocket(0).use { it.localPort }
+
+    companion object {
+
+        @JvmStatic
+        fun testCases() = listOf(
+            Arguments.of(8, 6, HttpStatusCode.RequestHeaderFieldTooLarge),
+            Arguments.of(16, 6, HttpStatusCode.NoContent),
+            Arguments.of(16, 8, HttpStatusCode.RequestHeaderFieldTooLarge)
+        )
+    }
+
+    @ParameterizedTest
+    @MethodSource("testCases")
+    fun `test HttpConfiguration with request header size example`(
+        requestHeaderSizeConfigKB: Int,
+        headerSizeKB: Int,
+        expectedStatusCode: HttpStatusCode
+    ) = runTest {
+        val serverPort = findFreePort()
+
+        embeddedServer(Jetty, configure = {
+            httpConfiguration = {
+                requestHeaderSize = requestHeaderSizeConfigKB * 1024
+            }
+            connector { port = serverPort }
+        }) {
+            routing {
+                get("/") {
+                    call.respond(HttpStatusCode.NoContent)
+                }
+            }
+        }.start(wait = false)
+
+        val createHeaderValue = { List(headerSizeKB * 1024) { Random.nextInt(33, 127).toChar() }.joinToString("") }
+
+        val response = HttpClient().use { client ->
+            client.get {
+                url("http://127.0.0.1:$serverPort/")
+                header("X-Custom-Large-Header-1", createHeaderValue())
+                header("X-Custom-Large-Header-2", createHeaderValue())
+            }
+        }
+
+        assertEquals(expectedStatusCode, response.status)
+    }
+}
diff --git a/ktor-server/ktor-server-jetty/api/ktor-server-jetty.api b/ktor-server/ktor-server-jetty/api/ktor-server-jetty.api
@@ -41,8 +41,10 @@ public class io/ktor/server/jetty/JettyApplicationEngineBase : io/ktor/server/en
 public final class io/ktor/server/jetty/JettyApplicationEngineBase$Configuration : io/ktor/server/engine/BaseApplicationEngine$Configuration {
 	public fun <init> ()V
 	public final fun getConfigureServer ()Lkotlin/jvm/functions/Function1;
+	public final fun getHttpConfiguration ()Lkotlin/jvm/functions/Function1;
 	public final fun getIdleTimeout-UwyO8pc ()J
 	public final fun setConfigureServer (Lkotlin/jvm/functions/Function1;)V
+	public final fun setHttpConfiguration (Lkotlin/jvm/functions/Function1;)V
 	public final fun setIdleTimeout-LRDsOJo (J)V
 }
 

diff --git a/ktor-server/ktor-server-jetty/jvm/src/io/ktor/server/jetty/JettyApplicationEngineBase.kt b/ktor-server/ktor-server-jetty/jvm/src/io/ktor/server/jetty/JettyApplicationEngineBase.kt
@@ -43,6 +43,12 @@ public open class JettyApplicationEngineBase(
          */
         public var configureServer: Server.() -> Unit = {}
 
+        /**
+         * Property function that will be called during Jetty server initialization with the http configuration instance
+         * that is passed to the managed connectors as a receiver.
+         */
+        public var httpConfiguration: HttpConfiguration.() -> Unit = {}
+
         /**
          * The duration of time that a connection can be idle before the connector takes action to close the connection.
          *

diff --git a/ktor-server/ktor-server-jetty/jvm/src/io/ktor/server/jetty/ServerInitializer.kt b/ktor-server/ktor-server-jetty/jvm/src/io/ktor/server/jetty/ServerInitializer.kt
@@ -23,7 +23,7 @@ internal fun Server.initializeServer(
             if (ktorConnector.type == ConnectorType.HTTPS) {
                 addCustomizer(SecureRequestCustomizer())
             }
-        }
+        }.apply(configuration.httpConfiguration)
 
         var alpnAvailable = false
         var alpnConnectionFactory: ALPNServerConnectionFactory?

diff --git a/...erver/ktor-server-jetty/jvm/test/io/ktor/tests/server/jetty/JettyHttpConfigurationTest.kt b/...erver/ktor-server-jetty/jvm/test/io/ktor/tests/server/jetty/JettyHttpConfigurationTest.kt
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
+ */
+
+package io.ktor.tests.server.jetty
+
+import io.ktor.client.*
+import io.ktor.client.request.*
+import io.ktor.http.*
+import io.ktor.server.engine.*
+import io.ktor.server.jetty.*
+import io.ktor.server.response.*
+import io.ktor.server.routing.*
+import kotlinx.coroutines.test.*
+import org.junit.jupiter.params.*
+import org.junit.jupiter.params.provider.*
+import java.net.*
+import kotlin.random.*
+import kotlin.test.*
+
+class JettyHttpConfigurationTest {
+
+    private fun findFreePort() = ServerSocket(0).use { it.localPort }
+
+    companion object {
+
+        @JvmStatic
+        fun testCases() = listOf(
+            Arguments.of(8, 6, HttpStatusCode.RequestHeaderFieldTooLarge),
+            Arguments.of(16, 6, HttpStatusCode.NoContent),
+            Arguments.of(16, 8, HttpStatusCode.RequestHeaderFieldTooLarge)
+        )
+    }
+
+    @ParameterizedTest
+    @MethodSource("testCases")
+    fun `test HttpConfiguration with request header size example`(
+        requestHeaderSizeConfigKB: Int,
+        headerSizeKB: Int,
+        expectedStatusCode: HttpStatusCode
+    ) = runTest {
+        val serverPort = findFreePort()
+
+        embeddedServer(Jetty, configure = {
+            httpConfiguration = {
+                requestHeaderSize = requestHeaderSizeConfigKB * 1024
+            }
+            connector { port = serverPort }
+        }) {
+            routing {
+                get("/") {
+                    call.respond(HttpStatusCode.NoContent)
+                }
+            }
+        }.start(wait = false)
+
+        val createHeaderValue = { List(headerSizeKB * 1024) { Random.nextInt(33, 127).toChar() }.joinToString("") }
+
+        val response = HttpClient().use { client ->
+            client.get {
+                url("http://127.0.0.1:$serverPort/")
+                header("X-Custom-Large-Header-1", createHeaderValue())
+                header("X-Custom-Large-Header-2", createHeaderValue())
+            }
+        }
+
+        assertEquals(expectedStatusCode, response.status)
+    }
+}
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
@@ -59,7 +59,7 @@ internal fun Application.cacheTestServer() {
             get("/etag-304") {
                 if (call.request.header("If-None-Match") == "My-ETAG") {
                     call.response.header("Etag", "My-ETAG")
-                    call.response.header("Vary", "Origin")
+                    call.response.header("Vary", "Origin, Accept-Encoding")
                     call.respond(HttpStatusCode.NotModified)
                     return@get
                 }
@@ -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)
+                }
+            }
         }
     }
 }