fix: classify malformed request params as invalid params (#886)
## Summary Classify typed request deserialization failures as JSON-RPC `Invalid Params` instead of `Internal error`. Fixes #830. ## Changes - translate `SerializationException` from typed request decoding to `-32602` - return a stable `Invalid params` message without serialization internals - cover malformed `initialize` requests through session and Streamable HTTP paths ## Scope and risk The catch is limited to request parameter deserialization after the JSON-RPC method has been resolved. Envelope parsing, unknown methods, handler failures, and successful requests keep their existing behavior. ## Verification ```bash ./gradlew :integration-test:jvmTest --tests 'io.modelcontextprotocol.kotlin.sdk.server.ServerSessionInitializeTest' ./gradlew :kotlin-sdk-server:jvmTest --tests 'io.modelcontextprotocol.kotlin.sdk.server.StreamableHttpServerTransportTest' ./gradlew :kotlin-sdk-core:jvmTest --tests 'io.modelcontextprotocol.kotlin.sdk.shared.ProtocolTest' ./gradlew ktlintCheck apiCheck ``` Co-authored-by: Pavel Gorgulov <devcrocod@gmail.com>
This commit is contained in:
parent
b6cb547d58
commit
cab669b108
3 changed files with 81 additions and 1 deletions
|
|
@ -7,9 +7,11 @@ import io.modelcontextprotocol.kotlin.sdk.types.InitializeRequest
|
||||||
import io.modelcontextprotocol.kotlin.sdk.types.InitializeRequestParams
|
import io.modelcontextprotocol.kotlin.sdk.types.InitializeRequestParams
|
||||||
import io.modelcontextprotocol.kotlin.sdk.types.JSONRPCError
|
import io.modelcontextprotocol.kotlin.sdk.types.JSONRPCError
|
||||||
import io.modelcontextprotocol.kotlin.sdk.types.JSONRPCMessage
|
import io.modelcontextprotocol.kotlin.sdk.types.JSONRPCMessage
|
||||||
|
import io.modelcontextprotocol.kotlin.sdk.types.JSONRPCRequest
|
||||||
import io.modelcontextprotocol.kotlin.sdk.types.JSONRPCResponse
|
import io.modelcontextprotocol.kotlin.sdk.types.JSONRPCResponse
|
||||||
import io.modelcontextprotocol.kotlin.sdk.types.LATEST_PROTOCOL_VERSION
|
import io.modelcontextprotocol.kotlin.sdk.types.LATEST_PROTOCOL_VERSION
|
||||||
import io.modelcontextprotocol.kotlin.sdk.types.RPCError
|
import io.modelcontextprotocol.kotlin.sdk.types.RPCError
|
||||||
|
import io.modelcontextprotocol.kotlin.sdk.types.RequestId
|
||||||
import io.modelcontextprotocol.kotlin.sdk.types.ServerCapabilities
|
import io.modelcontextprotocol.kotlin.sdk.types.ServerCapabilities
|
||||||
import io.modelcontextprotocol.kotlin.sdk.types.toJSON
|
import io.modelcontextprotocol.kotlin.sdk.types.toJSON
|
||||||
import kotlinx.coroutines.CompletableDeferred
|
import kotlinx.coroutines.CompletableDeferred
|
||||||
|
|
@ -18,9 +20,13 @@ import kotlinx.coroutines.joinAll
|
||||||
import kotlinx.coroutines.launch
|
import kotlinx.coroutines.launch
|
||||||
import kotlinx.coroutines.test.runTest
|
import kotlinx.coroutines.test.runTest
|
||||||
import kotlinx.coroutines.withContext
|
import kotlinx.coroutines.withContext
|
||||||
|
import kotlinx.serialization.json.buildJsonObject
|
||||||
|
import kotlinx.serialization.json.put
|
||||||
|
import kotlinx.serialization.json.putJsonObject
|
||||||
import org.junit.jupiter.api.Test
|
import org.junit.jupiter.api.Test
|
||||||
import java.util.concurrent.CopyOnWriteArrayList
|
import java.util.concurrent.CopyOnWriteArrayList
|
||||||
import kotlin.test.assertEquals
|
import kotlin.test.assertEquals
|
||||||
|
import kotlin.test.assertFalse
|
||||||
import kotlin.test.assertNotNull
|
import kotlin.test.assertNotNull
|
||||||
import kotlin.test.assertNull
|
import kotlin.test.assertNull
|
||||||
import kotlin.test.assertTrue
|
import kotlin.test.assertTrue
|
||||||
|
|
@ -41,6 +47,18 @@ class ServerSessionInitializeTest {
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
private fun createMalformedInitializeRequest(): JSONRPCRequest = JSONRPCRequest(
|
||||||
|
id = RequestId(1),
|
||||||
|
method = "initialize",
|
||||||
|
params = buildJsonObject {
|
||||||
|
putJsonObject("capabilities") {}
|
||||||
|
putJsonObject("clientInfo") {
|
||||||
|
put("name", "repro")
|
||||||
|
put("version", "0.1.0")
|
||||||
|
}
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `should handle first initialize request successfully`() = runTest {
|
fun `should handle first initialize request successfully`() = runTest {
|
||||||
val session = createSession()
|
val session = createSession()
|
||||||
|
|
@ -67,6 +85,28 @@ class ServerSessionInitializeTest {
|
||||||
|
|
||||||
// Both initialize requests arrive before notifications/initialized, in the serial dispatch
|
// Both initialize requests arrive before notifications/initialized, in the serial dispatch
|
||||||
// phase, so processing and response order stay deterministic.
|
// phase, so processing and response order stay deterministic.
|
||||||
|
@Test
|
||||||
|
fun `should classify malformed initialize params as invalid params`() = runTest {
|
||||||
|
val session = createSession()
|
||||||
|
val (clientTransport, serverTransport) = InMemoryTransport.createLinkedPair()
|
||||||
|
|
||||||
|
val responseDone = CompletableDeferred<JSONRPCError>()
|
||||||
|
clientTransport.onMessage { message ->
|
||||||
|
if (message is JSONRPCError) {
|
||||||
|
responseDone.complete(message)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
session.connect(serverTransport)
|
||||||
|
clientTransport.send(createMalformedInitializeRequest())
|
||||||
|
|
||||||
|
val response = responseDone.await()
|
||||||
|
assertEquals(RPCError.ErrorCode.INVALID_PARAMS, response.error.code)
|
||||||
|
assertFalse(response.error.message.contains("kotlinx.serialization"))
|
||||||
|
assertNull(session.clientCapabilities)
|
||||||
|
assertNull(session.clientVersion)
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `should reject duplicate initialize request`() = runTest {
|
fun `should reject duplicate initialize request`() = runTest {
|
||||||
val session = createSession()
|
val session = createSession()
|
||||||
|
|
|
||||||
|
|
@ -48,6 +48,7 @@ import kotlinx.coroutines.sync.Semaphore
|
||||||
import kotlinx.coroutines.sync.withPermit
|
import kotlinx.coroutines.sync.withPermit
|
||||||
import kotlinx.coroutines.withContext
|
import kotlinx.coroutines.withContext
|
||||||
import kotlinx.coroutines.withTimeoutOrNull
|
import kotlinx.coroutines.withTimeoutOrNull
|
||||||
|
import kotlinx.serialization.SerializationException
|
||||||
import kotlinx.serialization.json.JsonObject
|
import kotlinx.serialization.json.JsonObject
|
||||||
import kotlinx.serialization.json.JsonPrimitive
|
import kotlinx.serialization.json.JsonPrimitive
|
||||||
import kotlinx.serialization.json.encodeToJsonElement
|
import kotlinx.serialization.json.encodeToJsonElement
|
||||||
|
|
@ -941,7 +942,15 @@ public abstract class Protocol(@PublishedApi internal val options: ProtocolOptio
|
||||||
|
|
||||||
_requestHandlers.update { current ->
|
_requestHandlers.update { current ->
|
||||||
current.putting(method.value) { jSONRPCRequest, extraHandler ->
|
current.putting(method.value) { jSONRPCRequest, extraHandler ->
|
||||||
val request = jSONRPCRequest.fromJSON()
|
val request = try {
|
||||||
|
jSONRPCRequest.fromJSON()
|
||||||
|
} catch (cause: SerializationException) {
|
||||||
|
throw McpException(
|
||||||
|
code = RPCError.ErrorCode.INVALID_PARAMS,
|
||||||
|
message = "Invalid params",
|
||||||
|
cause = cause,
|
||||||
|
)
|
||||||
|
}
|
||||||
val response = wrapped(request as T, extraHandler)
|
val response = wrapped(request as T, extraHandler)
|
||||||
response
|
response
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -47,6 +47,7 @@ import io.modelcontextprotocol.kotlin.sdk.types.ListResourcesResult
|
||||||
import io.modelcontextprotocol.kotlin.sdk.types.ListToolsResult
|
import io.modelcontextprotocol.kotlin.sdk.types.ListToolsResult
|
||||||
import io.modelcontextprotocol.kotlin.sdk.types.McpJson
|
import io.modelcontextprotocol.kotlin.sdk.types.McpJson
|
||||||
import io.modelcontextprotocol.kotlin.sdk.types.Method
|
import io.modelcontextprotocol.kotlin.sdk.types.Method
|
||||||
|
import io.modelcontextprotocol.kotlin.sdk.types.RPCError
|
||||||
import io.modelcontextprotocol.kotlin.sdk.types.RequestId
|
import io.modelcontextprotocol.kotlin.sdk.types.RequestId
|
||||||
import io.modelcontextprotocol.kotlin.sdk.types.ServerCapabilities
|
import io.modelcontextprotocol.kotlin.sdk.types.ServerCapabilities
|
||||||
import io.modelcontextprotocol.kotlin.sdk.types.Tool
|
import io.modelcontextprotocol.kotlin.sdk.types.Tool
|
||||||
|
|
@ -225,6 +226,36 @@ class StreamableHttpServerTransportTest {
|
||||||
initResponse.headers[MCP_SESSION_ID_HEADER] shouldBe null
|
initResponse.headers[MCP_SESSION_ID_HEADER] shouldBe null
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `malformed initialize params return invalid params over streamable http`() = testApplication {
|
||||||
|
val mcpPath = "/mcp"
|
||||||
|
|
||||||
|
application {
|
||||||
|
mcpStreamableHttp(mcpPath, enableDnsRebindingProtection = false) {
|
||||||
|
Server(
|
||||||
|
Implementation("test-server", "1.0.0"),
|
||||||
|
ServerOptions(capabilities = ServerCapabilities()),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
val client = createTestClient()
|
||||||
|
|
||||||
|
val response = client.post(mcpPath) {
|
||||||
|
addStreamableHeaders()
|
||||||
|
setBody(
|
||||||
|
"""
|
||||||
|
{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"capabilities":{},"clientInfo":{"name":"repro","version":"0.1.0"}}}
|
||||||
|
""".trimIndent(),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
response.status shouldBe HttpStatusCode.OK
|
||||||
|
val error = response.body<JSONRPCError>()
|
||||||
|
error.error.code shouldBe RPCError.ErrorCode.INVALID_PARAMS
|
||||||
|
error.error.message.contains("kotlinx.serialization") shouldBe false
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `request with unsupported protocol version returns an HTTP error`() = testApplication {
|
fun `request with unsupported protocol version returns an HTTP error`() = testApplication {
|
||||||
configTestServer()
|
configTestServer()
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue