fix(server): match Accept header as media ranges (#912)
The header is now parsed into media ranges (`parseHeaderValue` + `ContentType.match`) ## Breaking Changes none ## Types of changes - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist - [x] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [x] My code follows the repository's style guidelines - [x] New and existing tests pass locally - [x] I have added appropriate error handling - [x] I have added or updated documentation as needed
This commit is contained in:
parent
7fa438ce00
commit
24c3cd3a69
2 changed files with 127 additions and 2 deletions
|
|
@ -1,9 +1,11 @@
|
|||
package io.modelcontextprotocol.kotlin.sdk.server
|
||||
|
||||
import io.ktor.http.BadContentTypeFormatException
|
||||
import io.ktor.http.ContentType
|
||||
import io.ktor.http.HttpHeaders
|
||||
import io.ktor.http.HttpMethod
|
||||
import io.ktor.http.HttpStatusCode
|
||||
import io.ktor.http.parseHeaderValue
|
||||
import io.ktor.server.application.ApplicationCall
|
||||
import io.ktor.server.request.contentType
|
||||
import io.ktor.server.request.header
|
||||
|
|
@ -864,8 +866,24 @@ public class StreamableHttpServerTransport(private val configuration: Configurat
|
|||
}
|
||||
}
|
||||
|
||||
private fun String?.accepts(mime: ContentType): Boolean =
|
||||
this?.lowercase()?.contains(mime.toString().lowercase()) == true
|
||||
/**
|
||||
* Reports whether the `Accept` header admits [mime].
|
||||
*
|
||||
* Comparison is over parsed media ranges rather than raw substrings, so wildcards are honored
|
||||
* and a near miss such as `application/jsonp` no longer passes as `application/json`. An absent
|
||||
* header states no preference and admits any type.
|
||||
*/
|
||||
private fun String?.accepts(mime: ContentType): Boolean {
|
||||
if (this == null) return true
|
||||
return parseHeaderValue(this).any { range ->
|
||||
if (range.quality <= 0.0) return@any false
|
||||
try {
|
||||
mime.match(ContentType.parse(range.value))
|
||||
} catch (_: BadContentTypeFormatException) {
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private suspend fun emitOnStream(streamId: String, session: ServerSSESession?, message: JSONRPCMessage) {
|
||||
val eventId = configuration.eventStore?.storeEvent(streamId, message)
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import io.ktor.client.call.body
|
|||
import io.ktor.client.plugins.logging.LogLevel
|
||||
import io.ktor.client.plugins.logging.Logging
|
||||
import io.ktor.client.request.HttpRequestBuilder
|
||||
import io.ktor.client.request.get
|
||||
import io.ktor.client.request.header
|
||||
import io.ktor.client.request.post
|
||||
import io.ktor.client.request.prepareGet
|
||||
|
|
@ -21,6 +22,7 @@ import io.ktor.http.contentType
|
|||
import io.ktor.serialization.kotlinx.json.json
|
||||
import io.ktor.server.application.ApplicationCall
|
||||
import io.ktor.server.application.install
|
||||
import io.ktor.server.routing.get
|
||||
import io.ktor.server.routing.post
|
||||
import io.ktor.server.routing.routing
|
||||
import io.ktor.server.sse.ServerSSESession
|
||||
|
|
@ -76,6 +78,7 @@ import kotlin.test.assertEquals
|
|||
import kotlin.test.assertFailsWith
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertNotNull
|
||||
import kotlin.test.assertTrue
|
||||
import kotlin.time.Duration.Companion.seconds
|
||||
import io.ktor.client.plugins.contentnegotiation.ContentNegotiation as ClientContentNegotiation
|
||||
import io.ktor.server.plugins.contentnegotiation.ContentNegotiation as ServerContentNegotiation
|
||||
|
|
@ -100,6 +103,26 @@ class StreamableHttpServerTransportTest {
|
|||
Arguments.of(sizeTestPayload.length.toLong(), HttpStatusCode.BadRequest),
|
||||
Arguments.of(sizeTestPayload.length.toLong() + 1, HttpStatusCode.BadRequest),
|
||||
)
|
||||
|
||||
@JvmStatic
|
||||
fun postAcceptHeaderCases(): List<Arguments> = listOf(
|
||||
Arguments.of("application/json, text/event-stream", HttpStatusCode.OK),
|
||||
Arguments.of("Application/JSON;q=0.9, TEXT/Event-Stream;charset=utf-8", HttpStatusCode.OK),
|
||||
Arguments.of("*/*", HttpStatusCode.OK),
|
||||
Arguments.of("application/*, text/*", HttpStatusCode.OK),
|
||||
Arguments.of(null, HttpStatusCode.OK),
|
||||
Arguments.of("application/json", HttpStatusCode.NotAcceptable),
|
||||
Arguments.of("text/event-stream", HttpStatusCode.NotAcceptable),
|
||||
Arguments.of("application/jsonp, text/event-stream-bogus", HttpStatusCode.NotAcceptable),
|
||||
Arguments.of("application/json, text/event-stream;q=0", HttpStatusCode.NotAcceptable),
|
||||
)
|
||||
|
||||
@JvmStatic
|
||||
fun getAcceptHeaderCases() = listOf(
|
||||
"application/json",
|
||||
"text/event-stream-bogus",
|
||||
"text/event-stream;q=0",
|
||||
)
|
||||
}
|
||||
|
||||
private val path = "/transport"
|
||||
|
|
@ -130,6 +153,90 @@ class StreamableHttpServerTransportTest {
|
|||
assertFalse(onMessageCalled.get(), "Transport should not deliver messages when headers are invalid")
|
||||
}
|
||||
|
||||
@ParameterizedTest
|
||||
@MethodSource("postAcceptHeaderCases")
|
||||
fun `POST Accept header is matched as media ranges`(acceptHeader: String?, expectedStatus: HttpStatusCode) =
|
||||
testApplication {
|
||||
configTestServer()
|
||||
|
||||
// Bare client: the ContentNegotiation plugin would supply an Accept header of its own,
|
||||
// leaving the absent-header case untestable.
|
||||
val client = createClient {}
|
||||
|
||||
val transport = StreamableHttpServerTransport(enableJsonResponse = true)
|
||||
val onMessageCalled = AtomicBoolean(false)
|
||||
transport.onMessage { message ->
|
||||
onMessageCalled.set(true)
|
||||
if (message is JSONRPCRequest) {
|
||||
transport.send(JSONRPCResponse(message.id, EmptyResult()))
|
||||
}
|
||||
}
|
||||
|
||||
configureTransportEndpoint(transport)
|
||||
|
||||
val response = client.post(path) {
|
||||
contentType(ContentType.Application.Json)
|
||||
acceptHeader?.let { header(HttpHeaders.Accept, it) }
|
||||
setBody(McpJson.encodeToString(JSONRPCMessage.serializer(), buildInitializeRequestPayload()))
|
||||
}
|
||||
|
||||
assertEquals(expectedStatus, response.status)
|
||||
assertEquals(
|
||||
expectedStatus == HttpStatusCode.OK,
|
||||
onMessageCalled.get(),
|
||||
"Only a request that clears the Accept gate may reach the transport",
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `POST with an unparsable Accept header is rejected without failing the request`() = testApplication {
|
||||
configTestServer()
|
||||
|
||||
val client = createClient {}
|
||||
|
||||
val transport = StreamableHttpServerTransport(enableJsonResponse = true)
|
||||
val onMessageCalled = AtomicBoolean(false)
|
||||
transport.onMessage {
|
||||
onMessageCalled.set(true)
|
||||
}
|
||||
|
||||
configureTransportEndpoint(transport)
|
||||
|
||||
val response = client.post(path) {
|
||||
contentType(ContentType.Application.Json)
|
||||
header(HttpHeaders.Accept, "not-a-media-type")
|
||||
setBody(McpJson.encodeToString(JSONRPCMessage.serializer(), buildInitializeRequestPayload()))
|
||||
}
|
||||
|
||||
// The exact code is decided by content negotiation once the transport has rejected the request.
|
||||
assertTrue(response.status.value in 400..499, "Expected a client error, got ${response.status}")
|
||||
assertFalse(onMessageCalled.get(), "Transport should not deliver messages when headers are invalid")
|
||||
}
|
||||
|
||||
@ParameterizedTest
|
||||
@MethodSource("getAcceptHeaderCases")
|
||||
fun `GET whose Accept header admits no event-stream range is rejected`(acceptHeader: String) = testApplication {
|
||||
configTestServer()
|
||||
|
||||
val client = createClient {}
|
||||
|
||||
val transport = StreamableHttpServerTransport(enableJsonResponse = true)
|
||||
|
||||
application {
|
||||
routing {
|
||||
get(path) {
|
||||
transport.handleGetRequest(FakeServerSSESession(call, call.coroutineContext), call)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
val response = client.get(path) {
|
||||
header(HttpHeaders.Accept, acceptHeader)
|
||||
}
|
||||
|
||||
assertEquals(HttpStatusCode.NotAcceptable, response.status)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `initialization request establishes session and returns json response`() = testApplication {
|
||||
configTestServer()
|
||||
|
|
|
|||
Loading…
Reference in a new issue