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..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 @@ -15,6 +15,7 @@ import io.ktor.http.Url import io.ktor.http.append 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 @@ -56,21 +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 - } - - 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( @@ -85,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)) { @@ -125,7 +111,7 @@ public class SseClientTransport( private suspend fun CoroutineScope.collectMessages() { try { session.incoming.collect { event -> - ensureActive() + this@collectMessages.ensureActive() when (event.event) { "error" -> { @@ -154,39 +140,16 @@ 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]. - */ 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" - } - endpoint.complete(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 } + endpoint.complete(endpointUrl) + logger.debug { "Client connected to endpoint: $endpointUrl" } } private suspend fun handleMessage(data: String) { @@ -224,3 +187,23 @@ 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 [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.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 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 01eb1f7df..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,12 +35,51 @@ 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" } + @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 `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 = "http://localhost/api/mcp/sse", + endpointEvent = "http://@/messages?sessionId=abc", + ) + + exception.message shouldBe "Endpoint is not a valid URL" + } + @Test fun `full url endpoint with a different host is rejected without exposing credentials`() = runTest { val exception = startWithRejectedEndpoint( @@ -124,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") @@ -139,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) @@ -162,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"))