Skip to content

fix: resolve SSE endpoint references per RFC 3986 in SseClientTransport #1100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
devcrocod merged 5 commits into from
Oct 16, 2026
Merged

fix: resolve SSE endpoint references per RFC 3986 in SseClientTransport #1100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c09faf7
fix: resolve SSE endpoint references per RFC 3986 in SseClientTransport
gibkim Oct 7, 2026
0906a5b
fix: treat any scheme-prefixed endpoint as an absolute URI
gibkim Oct 7, 2026
4c0f33c
fix: remove dot segments from full and network-path endpoints
gibkim Oct 7, 2026
b1c33dd
fix: reject scheme-prefixed and network-path endpoints without an aut…
gibkim Oct 7, 2026
06eacf6
Refactor SSE client transport to streamline URL resolution, remove do…
devcrocod Oct 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions ...t/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/integration/sse/SseSubpathTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
75 changes: 29 additions & 46 deletions ...ent/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/SseClientTransport.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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)) {
Expand Down Expand Up @@ -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" -> {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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

@jetbrains-air jetbrains-air Bot Oct 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Problem: a network-path reference (//host/path, RFC 3986 §4.2) also starts with /, so it takes this branch and is pasted after the authority as a path instead of replacing it. It therefore never reaches the origin check, even though it carries its own authority.

Failure scenario: a server behind a TLS-terminating proxy announces endpoint: //api.example.com/messages?sessionId=x; the client POSTs to https://api.example.com//api.example.com/messages?sessionId=x and gets the #1080 404. I confirmed the double-slash path goes out on the wire and misses the route. No origin leak — the POST stays on the SSE origin — but it never reaches the announced endpoint, and a //evil.example.com/... reference is silently rewritten instead of rejected.

Suggested fix: add a reference.startsWith("//") branch ahead of this one that builds "$protocol:$reference" and sends it through the same hasSameOrigin check as the full-URL branch above.

reference.startsWith("?") -> encodedPath.ifEmpty { "/" } + reference
else -> encodedPath.substringBeforeLast('/', missingDelimiterValue = "") + "/" + reference

@jetbrains-air jetbrains-air Bot Oct 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Problem: the merged path is used as-is, so dot segments in the reference are never removed. remove_dot_segments is the other half of RFC 3986 §5.2.2 that the KDoc above cites; it was added in 4c0f33c after Copilot's comment and dropped again (with its tests) in the last refactor.

Failure scenario: a server with the SSE URL /api/mcp/sse announces endpoint: ../messages?sessionId=x; the client POSTs to /api/mcp/../messages?sessionId=x. I checked this on the wire with the CIO engine — the un-normalized path is sent verbatim and a Ktor server does not match it to the registered route — so connect() fails with the same 404 as #1080 for any endpoint reference containing a dot segment. Same for an absolute-path reference such as /api/../messages.

Suggested fix: run remove_dot_segments over the path part of pathAndQuery before appending it to protocolWithAuthority, so it covers the absolute-path and relative-path branches alike.

}
return protocolWithAuthority + pathAndQuery
}
70 changes: 66 additions & 4 deletions ...commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/sse/SseClientTransportTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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")
Expand All @@ -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)
Expand All @@ -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"))
Expand Down
Loading