From c09faf7fc7dd90e6be757a4edc870e0785b76479 Mon Sep 17 00:00:00 2001 From: gibkim <86421130+gibkim@users.noreply.github.com> Date: Wed, 7 Oct 2026 11:59:42 +0900 Subject: [PATCH 1/5] fix: resolve SSE endpoint references per RFC 3986 in SseClientTransport A query-only endpoint such as ?sessionId=... was resolved against the parent directory of the SSE URL, so a client connecting to a server mounted with mcp("/sse") posted to /?sessionId=... and got 404. Resolve relative, absolute-path, query-only and network-path references against the SSE request URL per RFC 3986 section 5.2, and run the existing origin check on every resolved endpoint. Fixes #1080 --- .../sdk/integration/sse/SseSubpathTest.kt | 56 ++++++++++++ .../kotlin/sdk/client/SseClientTransport.kt | 87 ++++++++++++------- .../sdk/client/sse/SseClientTransportTest.kt | 60 +++++++++++++ 3 files changed, 174 insertions(+), 29 deletions(-) create mode 100644 integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/sse/SseSubpathTest.kt diff --git a/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/sse/SseSubpathTest.kt b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/sse/SseSubpathTest.kt new file mode 100644 index 000000000..b780eec8a --- /dev/null +++ b/integration-test/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/sse/SseSubpathTest.kt @@ -0,0 +1,56 @@ +package io.modelcontextprotocol.kotlin.sdk.integration.sse + +import io.kotest.matchers.shouldBe +import io.ktor.client.HttpClient +import io.ktor.client.plugins.sse.SSE +import io.ktor.server.application.install +import io.ktor.server.engine.embeddedServer +import io.ktor.server.routing.routing +import io.modelcontextprotocol.kotlin.sdk.client.Client +import io.modelcontextprotocol.kotlin.sdk.client.SseClientTransport +import io.modelcontextprotocol.kotlin.sdk.server.Server +import io.modelcontextprotocol.kotlin.sdk.server.ServerOptions +import io.modelcontextprotocol.kotlin.sdk.server.mcp +import io.modelcontextprotocol.kotlin.sdk.types.Implementation +import io.modelcontextprotocol.kotlin.sdk.types.ServerCapabilities +import io.modelcontextprotocol.kotlin.test.utils.actualPort +import io.modelcontextprotocol.kotlin.test.utils.runIntegrationTest +import kotlin.test.Test +import kotlin.time.Duration.Companion.seconds +import io.ktor.client.engine.cio.CIO as ClientCIO +import io.ktor.server.cio.CIO as ServerCIO +import io.ktor.server.sse.SSE as ServerSSE + +private const val HOST = "127.0.0.1" +private const val SSE_PATH = "/api/mcp/sse" + +class SseSubpathTest { + + @Test + fun `client connects to a server mounted on a subpath`() = runIntegrationTest(timeout = 20.seconds) { + val server = embeddedServer(ServerCIO, host = HOST, port = 0) { + install(ServerSSE) + routing { + mcp(SSE_PATH) { + Server( + serverInfo = Implementation("test-server", "1.0.0"), + options = ServerOptions(ServerCapabilities()), + ) + } + } + }.startSuspend(wait = false) + val httpClient = HttpClient(ClientCIO) { install(SSE) } + val client = Client(Implementation("test-client", "1.0.0")) + try { + // No requestBuilder: the client must follow the query-only endpoint the server announces. + client.connect(SseClientTransport(httpClient, "http://$HOST:${server.actualPort()}$SSE_PATH")) + + client.serverVersion?.name shouldBe "test-server" + client.ping() + } finally { + client.close() + httpClient.close() + server.stopSuspend(gracePeriodMillis = 0, timeoutMillis = 1_000) + } + } +} diff --git a/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt b/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt index 6b5f9ab89..018a2a3f2 100644 --- a/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt +++ b/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt @@ -60,17 +60,6 @@ public class SseClientTransport( session.call.request.url.protocolWithAuthority } - private val baseUrl: String by lazy { - session.call.request.url.let { url -> - val path = url.encodedPath - when { - path.isEmpty() -> origin - path.endsWith("/") -> origin + path.removeSuffix("/") - else -> origin + path.take(path.lastIndexOf("/")) - } - } - } - override suspend fun initialize() { session = urlString?.let { client.sseSession( @@ -156,27 +145,25 @@ public class SseClientTransport( /** * Resolves and completes [endpoint] based on [eventData]. - * Uses full URLs as-is, but rejects those whose origin differs from the SSE connection origin, - * treats absolute paths as origin-relative, and relative paths as relative to [baseUrl]. + * Resolves the reference against the SSE request URL as described in RFC 3986, section 5.2, + * and rejects the result if its origin differs from the SSE connection origin. */ private fun handleEndpoint(eventData: String) { try { - val endpointUrl = if (eventData.startsWith("http://") || eventData.startsWith("https://")) { - val url = Url(eventData) - val connectionUrl = session.call.request.url - if (!url.hasSameOrigin(connectionUrl)) { - val error = IllegalArgumentException( - "Endpoint origin ${url.safeOrigin} does not match connection origin ${connectionUrl.safeOrigin}", - ) - _onError(error) - endpoint.completeExceptionally(error) - return - } - eventData - } else if (eventData.startsWith("/")) { - origin + eventData - } else { - "$baseUrl/$eventData" + val connectionUrl = session.call.request.url + val endpointUrl = when { + eventData.startsWith("http://") || eventData.startsWith("https://") -> eventData + eventData.startsWith("//") -> "${connectionUrl.protocol.name}:$eventData" + else -> origin + connectionUrl.resolvePathAndQuery(eventData) + } + val url = Url(endpointUrl) + if (!url.hasSameOrigin(connectionUrl)) { + val error = IllegalArgumentException( + "Endpoint origin ${url.safeOrigin} does not match connection origin ${connectionUrl.safeOrigin}", + ) + _onError(error) + endpoint.completeExceptionally(error) + return } endpoint.complete(endpointUrl) logger.debug { "Client connected to endpoint: $endpointUrl" } @@ -224,3 +211,45 @@ private fun Url.hasSameOrigin(other: Url): Boolean = /** Scheme, host and non-default port, without user info, so it is safe to put into error messages. */ private val Url.safeOrigin: String get() = "${protocol.name}://$hostWithPortIfSpecified" + +/** + * Resolves a reference without scheme and authority against this URL (RFC 3986, section 5.2.2) + * and returns the resulting path and query. The fragment is dropped. + */ +private fun Url.resolvePathAndQuery(reference: String): String { + val withoutFragment = reference.substringBefore('#') + val referencePath = withoutFragment.substringBefore('?') + val referenceQuery = if ('?' in withoutFragment) withoutFragment.substringAfter('?') else null + val path = when { + referencePath.isEmpty() -> encodedPath + + referencePath.startsWith("/") -> removeDotSegments(referencePath) + + else -> removeDotSegments( + encodedPath.substringBeforeLast('/', missingDelimiterValue = "") + "/" + referencePath, + ) + } + val baseQuery = encodedQuery.takeIf { it.isNotEmpty() } + val query = if (referencePath.isEmpty()) referenceQuery ?: baseQuery else referenceQuery + return if (query == null) path else "$path?$query" +} + +/** Removes `.` and `..` segments from an absolute [path] (RFC 3986, section 5.2.4). */ +private fun removeDotSegments(path: String): String { + val output = ArrayDeque() + val segments = path.removePrefix("/").split('/') + segments.forEachIndexed { index, segment -> + val isLast = index == segments.lastIndex + when (segment) { + "." -> if (isLast) output.addLast("") + + ".." -> { + output.removeLastOrNull() + if (isLast) output.addLast("") + } + + else -> output.addLast(segment) + } + } + return output.joinToString("/", prefix = "/") +} diff --git a/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt b/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt index 01eb1f7df..7128a3862 100644 --- a/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt +++ b/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt @@ -39,6 +39,66 @@ class SseClientTransportTest { post.url.toString() shouldBe "http://example.com/api/mcp/post?sessionId=xyz" } + @Test + fun `query-only endpoint keeps the sse path`() = runTest { + val post = sendThroughEndpoint(sseUrl = SSE_URL, endpointEvent = "?sessionId=abc") + + post.url.toString() shouldBe "http://example.com/api/mcp/sse?sessionId=abc" + } + + @Test + fun `query-only endpoint replaces the sse query`() = runTest { + val post = sendThroughEndpoint(sseUrl = "$SSE_URL?token=t", endpointEvent = "?sessionId=abc") + + post.url.toString() shouldBe "http://example.com/api/mcp/sse?sessionId=abc" + } + + @Test + fun `relative path endpoint drops the sse query`() = runTest { + val post = sendThroughEndpoint(sseUrl = "$SSE_URL?token=t", endpointEvent = "post?sessionId=xyz") + + post.url.toString() shouldBe "http://example.com/api/mcp/post?sessionId=xyz" + } + + @Test + fun `relative path endpoint resolves against a trailing-slash sse url`() = runTest { + val post = sendThroughEndpoint(sseUrl = "http://example.com/api/mcp/", endpointEvent = "post?sessionId=xyz") + + post.url.toString() shouldBe "http://example.com/api/mcp/post?sessionId=xyz" + } + + @Test + fun `dot-segment endpoint resolves against the sse directory`() = runTest { + val post = sendThroughEndpoint(sseUrl = SSE_URL, endpointEvent = "../messages?sessionId=abc") + + post.url.toString() shouldBe "http://example.com/api/messages?sessionId=abc" + } + + @Test + fun `dot segments cannot climb above the root`() = runTest { + val post = sendThroughEndpoint(sseUrl = SSE_URL, endpointEvent = "../../../../messages?sessionId=abc") + + post.url.toString() shouldBe "http://example.com/messages?sessionId=abc" + } + + @Test + fun `network-path endpoint with a different host is rejected`() = runTest { + val exception = startWithRejectedEndpoint( + sseUrl = SSE_URL, + endpointEvent = "//evil.example.com/messages?sessionId=abc", + ) + + exception.message shouldBe + "Endpoint origin http://evil.example.com does not match connection origin http://example.com" + } + + @Test + fun `network-path endpoint with the same host is accepted`() = runTest { + val post = sendThroughEndpoint(sseUrl = SSE_URL, endpointEvent = "//example.com/messages?sessionId=abc") + + post.url.toString() shouldBe "http://example.com/messages?sessionId=abc" + } + @Test fun `full url endpoint with a different host is rejected without exposing credentials`() = runTest { val exception = startWithRejectedEndpoint( From 0906a5b1f2909ce4cd9f58d57816897a0e739461 Mon Sep 17 00:00:00 2001 From: gibkim <86421130+gibkim@users.noreply.github.com> Date: Wed, 7 Oct 2026 12:33:51 +0900 Subject: [PATCH 2/5] fix: treat any scheme-prefixed endpoint as an absolute URI Scheme names are case-insensitive and not limited to http(s) (RFC 3986, section 3.1). An endpoint such as HTTP://host/path was appended to the SSE directory as a relative path. Detect the scheme by its syntax so that such endpoints go through the origin check like other absolute URIs. --- .../kotlin/sdk/client/SseClientTransport.kt | 5 ++++- .../sdk/client/sse/SseClientTransportTest.kt | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt b/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt index 018a2a3f2..63f174c2b 100644 --- a/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt +++ b/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt @@ -36,6 +36,9 @@ import kotlinx.serialization.SerializationException import kotlin.concurrent.atomics.ExperimentalAtomicApi import kotlin.time.Duration +/** A reference that starts with a scheme is an absolute URI (RFC 3986, section 3.1). */ +private val URI_SCHEME = Regex("^[A-Za-z][A-Za-z0-9+.-]*:") + /** * Client transport for SSE: this will connect to a server using Server-Sent Events for receiving * messages and make separate POST requests for sending messages. @@ -152,7 +155,7 @@ public class SseClientTransport( try { val connectionUrl = session.call.request.url val endpointUrl = when { - eventData.startsWith("http://") || eventData.startsWith("https://") -> eventData + URI_SCHEME.containsMatchIn(eventData) -> eventData eventData.startsWith("//") -> "${connectionUrl.protocol.name}:$eventData" else -> origin + connectionUrl.resolvePathAndQuery(eventData) } diff --git a/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt b/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt index 7128a3862..35818d5a9 100644 --- a/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt +++ b/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt @@ -81,6 +81,21 @@ class SseClientTransportTest { post.url.toString() shouldBe "http://example.com/messages?sessionId=abc" } + @Test + fun `full url endpoint with an upper-case scheme is used as-is`() = runTest { + val post = sendThroughEndpoint(sseUrl = SSE_URL, endpointEvent = "HTTP://example.com/messages?sessionId=abc") + + post.url.toString() shouldBe "http://example.com/messages?sessionId=abc" + } + + @Test + fun `endpoint with a non-http scheme is rejected`() = runTest { + val exception = startWithRejectedEndpoint(sseUrl = SSE_URL, endpointEvent = "mailto:user@example.com") + + exception.message shouldBe + "Endpoint origin mailto://example.com does not match connection origin http://example.com" + } + @Test fun `network-path endpoint with a different host is rejected`() = runTest { val exception = startWithRejectedEndpoint( From 4c0f33cc0160f0720451f38cced9ce12643fbb68 Mon Sep 17 00:00:00 2001 From: gibkim <86421130+gibkim@users.noreply.github.com> Date: Wed, 7 Oct 2026 12:48:09 +0900 Subject: [PATCH 3/5] fix: remove dot segments from full and network-path endpoints RFC 3986 section 5.2.2 removes dot segments from the path of a reference that has a scheme or an authority too. Normalize the path of such endpoints after the origin check. --- .../kotlin/sdk/client/SseClientTransport.kt | 15 ++++++++++++++- .../sdk/client/sse/SseClientTransportTest.kt | 16 ++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt b/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt index 63f174c2b..eab371fe5 100644 --- a/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt +++ b/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt @@ -11,8 +11,10 @@ import io.ktor.client.request.setBody import io.ktor.client.statement.bodyAsText import io.ktor.http.ContentType import io.ktor.http.HttpHeaders +import io.ktor.http.URLBuilder import io.ktor.http.Url import io.ktor.http.append +import io.ktor.http.encodedPath import io.ktor.http.hostWithPortIfSpecified import io.ktor.http.isSuccess import io.ktor.http.protocolWithAuthority @@ -168,7 +170,7 @@ public class SseClientTransport( endpoint.completeExceptionally(error) return } - endpoint.complete(endpointUrl) + endpoint.complete(url.withoutDotSegments() ?: endpointUrl) logger.debug { "Client connected to endpoint: $endpointUrl" } } catch (e: CancellationException) { throw e @@ -237,6 +239,17 @@ private fun Url.resolvePathAndQuery(reference: String): String { return if (query == null) path else "$path?$query" } +/** + * Returns this URL with `.` and `..` segments removed from its path (RFC 3986, section 5.2.2), + * or `null` if the path has none. + */ +private fun Url.withoutDotSegments(): String? { + val path = encodedPath + if (path.isEmpty()) return null + val normalized = removeDotSegments(path) + return if (normalized == path) null else URLBuilder(this).apply { encodedPath = normalized }.buildString() +} + /** Removes `.` and `..` segments from an absolute [path] (RFC 3986, section 5.2.4). */ private fun removeDotSegments(path: String): String { val output = ArrayDeque() diff --git a/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt b/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt index 35818d5a9..28b2ddc8b 100644 --- a/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt +++ b/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt @@ -96,6 +96,22 @@ class SseClientTransportTest { "Endpoint origin mailto://example.com does not match connection origin http://example.com" } + @Test + fun `dot segments are removed from a full url endpoint`() = runTest { + val post = + sendThroughEndpoint(sseUrl = SSE_URL, endpointEvent = "http://example.com/a/../messages?sessionId=abc") + + post.url.toString() shouldBe "http://example.com/messages?sessionId=abc" + } + + @Test + fun `dot segments are removed from a network-path endpoint`() = runTest { + val post = + sendThroughEndpoint(sseUrl = SSE_URL, endpointEvent = "//example.com/a/./b/../messages?sessionId=abc") + + post.url.toString() shouldBe "http://example.com/a/messages?sessionId=abc" + } + @Test fun `network-path endpoint with a different host is rejected`() = runTest { val exception = startWithRejectedEndpoint( From b1c33ddb22ebdb9053012defddb4f300153f0d8f Mon Sep 17 00:00:00 2001 From: gibkim <86421130+gibkim@users.noreply.github.com> Date: Wed, 7 Oct 2026 16:54:00 +0900 Subject: [PATCH 4/5] fix: reject scheme-prefixed and network-path endpoints without an authority Ktor's Url parses a rootless reference such as http:example.com/messages as http://localhost/example.com/messages, so it passed the origin check against an SSE server on localhost and the POST went to a different path. Require a non-empty authority for references with a scheme or a leading double slash. --- .../kotlin/sdk/client/SseClientTransport.kt | 18 +++++++++++++++--- .../sdk/client/sse/SseClientTransportTest.kt | 18 ++++++++++++++++-- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt b/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt index eab371fe5..d0df27635 100644 --- a/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt +++ b/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt @@ -41,6 +41,9 @@ import kotlin.time.Duration /** A reference that starts with a scheme is an absolute URI (RFC 3986, section 3.1). */ private val URI_SCHEME = Regex("^[A-Za-z][A-Za-z0-9+.-]*:") +/** A scheme-prefixed or network-path reference with a non-empty authority (RFC 3986, section 3.2). */ +private val URI_WITH_AUTHORITY = Regex("^([A-Za-z][A-Za-z0-9+.-]*:)?//[^/?#]") + /** * Client transport for SSE: this will connect to a server using Server-Sent Events for receiving * messages and make separate POST requests for sending messages. @@ -156,6 +159,11 @@ public class SseClientTransport( private fun handleEndpoint(eventData: String) { try { val connectionUrl = session.call.request.url + val hasSchemeOrAuthority = URI_SCHEME.containsMatchIn(eventData) || eventData.startsWith("//") + if (hasSchemeOrAuthority && !URI_WITH_AUTHORITY.containsMatchIn(eventData)) { + rejectEndpoint("Endpoint URI with a scheme or authority must have a non-empty authority") + return + } val endpointUrl = when { URI_SCHEME.containsMatchIn(eventData) -> eventData eventData.startsWith("//") -> "${connectionUrl.protocol.name}:$eventData" @@ -163,11 +171,9 @@ public class SseClientTransport( } val url = Url(endpointUrl) if (!url.hasSameOrigin(connectionUrl)) { - val error = IllegalArgumentException( + rejectEndpoint( "Endpoint origin ${url.safeOrigin} does not match connection origin ${connectionUrl.safeOrigin}", ) - _onError(error) - endpoint.completeExceptionally(error) return } endpoint.complete(url.withoutDotSegments() ?: endpointUrl) @@ -181,6 +187,12 @@ public class SseClientTransport( } } + private fun rejectEndpoint(message: String) { + val error = IllegalArgumentException(message) + _onError(error) + endpoint.completeExceptionally(error) + } + private suspend fun handleMessage(data: String) { try { val message = McpJson.decodeFromString(data) diff --git a/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt b/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt index 28b2ddc8b..96f85798d 100644 --- a/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt +++ b/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt @@ -92,8 +92,7 @@ class SseClientTransportTest { fun `endpoint with a non-http scheme is rejected`() = runTest { val exception = startWithRejectedEndpoint(sseUrl = SSE_URL, endpointEvent = "mailto:user@example.com") - exception.message shouldBe - "Endpoint origin mailto://example.com does not match connection origin http://example.com" + exception.message shouldBe "Endpoint URI with a scheme or authority must have a non-empty authority" } @Test @@ -112,6 +111,21 @@ class SseClientTransportTest { post.url.toString() shouldBe "http://example.com/a/messages?sessionId=abc" } + @Test + fun `scheme-prefixed endpoint without an authority is rejected`() = runTest { + val exception = + startWithRejectedEndpoint(sseUrl = SSE_URL, endpointEvent = "http:example.com/messages?sessionId=abc") + + exception.message shouldBe "Endpoint URI with a scheme or authority must have a non-empty authority" + } + + @Test + fun `network-path endpoint with an empty authority is rejected`() = runTest { + val exception = startWithRejectedEndpoint(sseUrl = SSE_URL, endpointEvent = "///messages?sessionId=abc") + + exception.message shouldBe "Endpoint URI with a scheme or authority must have a non-empty authority" + } + @Test fun `network-path endpoint with a different host is rejected`() = runTest { val exception = startWithRejectedEndpoint( From 06eacf65308b783d880b21791c951255be3a7260 Mon Sep 17 00:00:00 2001 From: devcrocod Date: Fri, 16 Oct 2026 10:47:08 +0200 Subject: [PATCH 5/5] Refactor SSE client transport to streamline URL resolution, remove dot-segment handling, and improve validation logic. --- .../kotlin/sdk/client/SseClientTransport.kt | 120 ++++-------------- .../sdk/client/sse/SseClientTransportTest.kt | 107 +++++----------- 2 files changed, 55 insertions(+), 172 deletions(-) diff --git a/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt b/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt index d0df27635..29d1d3b98 100644 --- a/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt +++ b/kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt @@ -11,12 +11,11 @@ import io.ktor.client.request.setBody import io.ktor.client.statement.bodyAsText import io.ktor.http.ContentType import io.ktor.http.HttpHeaders -import io.ktor.http.URLBuilder import io.ktor.http.Url import io.ktor.http.append -import io.ktor.http.encodedPath import io.ktor.http.hostWithPortIfSpecified import io.ktor.http.isSuccess +import io.ktor.http.parseUrl import io.ktor.http.protocolWithAuthority import io.modelcontextprotocol.kotlin.sdk.shared.AbstractClientTransport import io.modelcontextprotocol.kotlin.sdk.shared.TransportSendOptions @@ -38,12 +37,6 @@ import kotlinx.serialization.SerializationException import kotlin.concurrent.atomics.ExperimentalAtomicApi import kotlin.time.Duration -/** A reference that starts with a scheme is an absolute URI (RFC 3986, section 3.1). */ -private val URI_SCHEME = Regex("^[A-Za-z][A-Za-z0-9+.-]*:") - -/** A scheme-prefixed or network-path reference with a non-empty authority (RFC 3986, section 3.2). */ -private val URI_WITH_AUTHORITY = Regex("^([A-Za-z][A-Za-z0-9+.-]*:)?//[^/?#]") - /** * Client transport for SSE: this will connect to a server using Server-Sent Events for receiving * messages and make separate POST requests for sending messages. @@ -64,10 +57,6 @@ public class SseClientTransport( private lateinit var scope: CoroutineScope private var job: Job? = null - private val origin: String by lazy { - session.call.request.url.protocolWithAuthority - } - override suspend fun initialize() { session = urlString?.let { client.sseSession( @@ -82,7 +71,7 @@ public class SseClientTransport( // Endpoints are validated against the origin of the SSE request, so that request must not // have been redirected away from the origin the transport was configured with. - val requestedUrl = urlString?.let { Url(it) }?.takeIf { it.host.isNotEmpty() } + val requestedUrl = urlString?.let(::parseUrl) val connectionUrl = session.call.request.url if (requestedUrl != null) { check(requestedUrl.hasSameOrigin(connectionUrl)) { @@ -122,7 +111,7 @@ public class SseClientTransport( private suspend fun CoroutineScope.collectMessages() { try { session.incoming.collect { event -> - ensureActive() + this@collectMessages.ensureActive() when (event.event) { "error" -> { @@ -151,46 +140,16 @@ public class SseClientTransport( } } - /** - * Resolves and completes [endpoint] based on [eventData]. - * Resolves the reference against the SSE request URL as described in RFC 3986, section 5.2, - * and rejects the result if its origin differs from the SSE connection origin. - */ private fun handleEndpoint(eventData: String) { - try { - val connectionUrl = session.call.request.url - val hasSchemeOrAuthority = URI_SCHEME.containsMatchIn(eventData) || eventData.startsWith("//") - if (hasSchemeOrAuthority && !URI_WITH_AUTHORITY.containsMatchIn(eventData)) { - rejectEndpoint("Endpoint URI with a scheme or authority must have a non-empty authority") - return - } - val endpointUrl = when { - URI_SCHEME.containsMatchIn(eventData) -> eventData - eventData.startsWith("//") -> "${connectionUrl.protocol.name}:$eventData" - else -> origin + connectionUrl.resolvePathAndQuery(eventData) - } - val url = Url(endpointUrl) - if (!url.hasSameOrigin(connectionUrl)) { - rejectEndpoint( - "Endpoint origin ${url.safeOrigin} does not match connection origin ${connectionUrl.safeOrigin}", - ) - return - } - endpoint.complete(url.withoutDotSegments() ?: endpointUrl) - logger.debug { "Client connected to endpoint: $endpointUrl" } - } catch (e: CancellationException) { - throw e - } catch (e: Throwable) { + val endpointUrl = try { + session.call.request.url.resolveEndpoint(eventData) + } catch (e: IllegalArgumentException) { _onError(e) endpoint.completeExceptionally(e) - throw e + return } - } - - private fun rejectEndpoint(message: String) { - val error = IllegalArgumentException(message) - _onError(error) - endpoint.completeExceptionally(error) + endpoint.complete(endpointUrl) + logger.debug { "Client connected to endpoint: $endpointUrl" } } private suspend fun handleMessage(data: String) { @@ -230,54 +189,21 @@ private val Url.safeOrigin: String get() = "${protocol.name}://$hostWithPortIfSpecified" /** - * Resolves a reference without scheme and authority against this URL (RFC 3986, section 5.2.2) - * and returns the resulting path and query. The fragment is dropped. + * Resolves [reference] from an `endpoint` event against this SSE request URL. + * A full URL must have the same origin, and a query-only reference keeps the path (RFC 3986, section 5.2.2). */ -private fun Url.resolvePathAndQuery(reference: String): String { - val withoutFragment = reference.substringBefore('#') - val referencePath = withoutFragment.substringBefore('?') - val referenceQuery = if ('?' in withoutFragment) withoutFragment.substringAfter('?') else null - val path = when { - referencePath.isEmpty() -> encodedPath - - referencePath.startsWith("/") -> removeDotSegments(referencePath) - - else -> removeDotSegments( - encodedPath.substringBeforeLast('/', missingDelimiterValue = "") + "/" + referencePath, - ) - } - val baseQuery = encodedQuery.takeIf { it.isNotEmpty() } - val query = if (referencePath.isEmpty()) referenceQuery ?: baseQuery else referenceQuery - return if (query == null) path else "$path?$query" -} - -/** - * Returns this URL with `.` and `..` segments removed from its path (RFC 3986, section 5.2.2), - * or `null` if the path has none. - */ -private fun Url.withoutDotSegments(): String? { - val path = encodedPath - if (path.isEmpty()) return null - val normalized = removeDotSegments(path) - return if (normalized == path) null else URLBuilder(this).apply { encodedPath = normalized }.buildString() -} - -/** Removes `.` and `..` segments from an absolute [path] (RFC 3986, section 5.2.4). */ -private fun removeDotSegments(path: String): String { - val output = ArrayDeque() - val segments = path.removePrefix("/").split('/') - segments.forEachIndexed { index, segment -> - val isLast = index == segments.lastIndex - when (segment) { - "." -> if (isLast) output.addLast("") - - ".." -> { - output.removeLastOrNull() - if (isLast) output.addLast("") - } - - else -> output.addLast(segment) +private fun Url.resolveEndpoint(reference: String): String { + if (reference.startsWith("http://") || reference.startsWith("https://")) { + val url = requireNotNull(parseUrl(reference)) { "Endpoint is not a valid URL" } + require(url.hasSameOrigin(this)) { + "Endpoint origin ${url.safeOrigin} does not match connection origin $safeOrigin" } + return reference + } + val pathAndQuery = when { + reference.startsWith("/") -> reference + reference.startsWith("?") -> encodedPath.ifEmpty { "/" } + reference + else -> encodedPath.substringBeforeLast('/', missingDelimiterValue = "") + "/" + reference } - return output.joinToString("/", prefix = "/") + return protocolWithAuthority + pathAndQuery } diff --git a/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt b/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt index 96f85798d..265b648cb 100644 --- a/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt +++ b/kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt @@ -4,6 +4,8 @@ import io.kotest.assertions.throwables.shouldThrow import io.kotest.matchers.collections.shouldBeEmpty import io.kotest.matchers.shouldBe import io.ktor.client.HttpClient +import io.ktor.client.HttpClientConfig +import io.ktor.client.plugins.defaultRequest import io.ktor.client.plugins.sse.SSE import io.ktor.client.request.HttpRequestData import io.ktor.client.request.HttpResponseData @@ -33,7 +35,7 @@ class SseClientTransportTest { } @Test - fun `relative path endpoint resolves against baseUrl`() = runTest { + fun `relative path endpoint resolves against the sse directory`() = runTest { val post = sendThroughEndpoint(sseUrl = SSE_URL, endpointEvent = "post?sessionId=xyz") post.url.toString() shouldBe "http://example.com/api/mcp/post?sessionId=xyz" @@ -68,80 +70,14 @@ class SseClientTransportTest { } @Test - fun `dot-segment endpoint resolves against the sse directory`() = runTest { - val post = sendThroughEndpoint(sseUrl = SSE_URL, endpointEvent = "../messages?sessionId=abc") - - post.url.toString() shouldBe "http://example.com/api/messages?sessionId=abc" - } - - @Test - fun `dot segments cannot climb above the root`() = runTest { - val post = sendThroughEndpoint(sseUrl = SSE_URL, endpointEvent = "../../../../messages?sessionId=abc") - - post.url.toString() shouldBe "http://example.com/messages?sessionId=abc" - } - - @Test - fun `full url endpoint with an upper-case scheme is used as-is`() = runTest { - val post = sendThroughEndpoint(sseUrl = SSE_URL, endpointEvent = "HTTP://example.com/messages?sessionId=abc") - - post.url.toString() shouldBe "http://example.com/messages?sessionId=abc" - } - - @Test - fun `endpoint with a non-http scheme is rejected`() = runTest { - val exception = startWithRejectedEndpoint(sseUrl = SSE_URL, endpointEvent = "mailto:user@example.com") - - exception.message shouldBe "Endpoint URI with a scheme or authority must have a non-empty authority" - } - - @Test - fun `dot segments are removed from a full url endpoint`() = runTest { - val post = - sendThroughEndpoint(sseUrl = SSE_URL, endpointEvent = "http://example.com/a/../messages?sessionId=abc") - - post.url.toString() shouldBe "http://example.com/messages?sessionId=abc" - } - - @Test - fun `dot segments are removed from a network-path endpoint`() = runTest { - val post = - sendThroughEndpoint(sseUrl = SSE_URL, endpointEvent = "//example.com/a/./b/../messages?sessionId=abc") - - post.url.toString() shouldBe "http://example.com/a/messages?sessionId=abc" - } - - @Test - fun `scheme-prefixed endpoint without an authority is rejected`() = runTest { - val exception = - startWithRejectedEndpoint(sseUrl = SSE_URL, endpointEvent = "http:example.com/messages?sessionId=abc") - - exception.message shouldBe "Endpoint URI with a scheme or authority must have a non-empty authority" - } - - @Test - fun `network-path endpoint with an empty authority is rejected`() = runTest { - val exception = startWithRejectedEndpoint(sseUrl = SSE_URL, endpointEvent = "///messages?sessionId=abc") - - exception.message shouldBe "Endpoint URI with a scheme or authority must have a non-empty authority" - } - - @Test - fun `network-path endpoint with a different host is rejected`() = runTest { + fun `full url endpoint without a host is rejected`() = runTest { + // Ktor fills an empty host with localhost, which would match this SSE origin. val exception = startWithRejectedEndpoint( - sseUrl = SSE_URL, - endpointEvent = "//evil.example.com/messages?sessionId=abc", + sseUrl = "http://localhost/api/mcp/sse", + endpointEvent = "http://@/messages?sessionId=abc", ) - exception.message shouldBe - "Endpoint origin http://evil.example.com does not match connection origin http://example.com" - } - - @Test - fun `network-path endpoint with the same host is accepted`() = runTest { - val post = sendThroughEndpoint(sseUrl = SSE_URL, endpointEvent = "//example.com/messages?sessionId=abc") - - post.url.toString() shouldBe "http://example.com/messages?sessionId=abc" + exception.message shouldBe "Endpoint is not a valid URL" } @Test @@ -229,6 +165,17 @@ class SseClientTransportTest { post.url.toString() shouldBe "http://example.com/messages?sessionId=abc" } + @Test + fun `relative sse url resolved by the default request is not treated as a redirect`() = runTest { + val post = sendThroughEndpoint( + sseUrl = "sse", + endpointEvent = "/messages?sessionId=abc", + clientConfig = { defaultRequest { url("http://example.com/api/mcp/") } }, + ) + + post.url.toString() shouldBe "http://example.com/messages?sessionId=abc" + } + @Test fun `onClose callback fires when the SSE stream is disconnected by the server`() = runTest { val engine = CapturingSseClientEngine(endpoint = "/messages?sessionId=abc") @@ -244,8 +191,17 @@ class SseClientTransportTest { engine.close() } - private fun sseTransport(sseUrl: String, engine: CapturingSseClientEngine) = - SseClientTransport(HttpClient(engine) { install(SSE) }, sseUrl) + private fun sseTransport( + sseUrl: String, + engine: CapturingSseClientEngine, + clientConfig: HttpClientConfig<*>.() -> Unit = {}, + ) = SseClientTransport( + HttpClient(engine) { + install(SSE) + clientConfig() + }, + sseUrl, + ) private suspend fun startWithRejectedEndpoint(sseUrl: String, endpointEvent: String): IllegalArgumentException { val engine = CapturingSseClientEngine(endpoint = endpointEvent) @@ -267,9 +223,10 @@ class SseClientTransportTest { sseUrl: String, endpointEvent: String, sseRedirectLocation: String? = null, + clientConfig: HttpClientConfig<*>.() -> Unit = {}, ): HttpRequestData { val engine = CapturingSseClientEngine(endpointEvent, sseRedirectLocation) - val transport = sseTransport(sseUrl, engine) + val transport = sseTransport(sseUrl, engine, clientConfig) try { transport.start() transport.send(JSONRPCNotification(method = "test"))