diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletResponseUtils.java b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletResponseUtils.java index f52737c7d..5e0aa0482 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletResponseUtils.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletResponseUtils.java @@ -5,6 +5,10 @@ package io.modelcontextprotocol.server.transport; import java.io.IOException; +import java.io.PrintWriter; + +import io.modelcontextprotocol.json.McpJsonMapper; +import io.modelcontextprotocol.spec.McpSchema; import jakarta.servlet.http.HttpServletResponse; @@ -12,6 +16,7 @@ * Utility methods for working with {@link HttpServletResponse}. For internal use only. * * @author Daniel Garnier-Moiroux + * @author Taewoong Kim */ final class HttpServletResponseUtils { @@ -20,6 +25,20 @@ final class HttpServletResponseUtils { private HttpServletResponseUtils() { } + /** + * Writes a JSON-RPC response with the supplied HTTP status. + */ + static void writeJsonResponse(HttpServletResponse response, int httpStatus, McpJsonMapper jsonMapper, + McpSchema.JSONRPCResponse jsonRpcResponse) throws IOException { + String json = jsonMapper.writeValueAsString(jsonRpcResponse); + response.setContentType("application/json"); + response.setCharacterEncoding("UTF-8"); + response.setStatus(httpStatus); + PrintWriter writer = response.getWriter(); + writer.write(json); + writer.flush(); + } + /** * Prepares the response for streaming Server-Sent Events: sets the content type and * the headers that keep intermediaries from caching, transforming or buffering the diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProvider.java b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProvider.java index a80d694be..d26605324 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProvider.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProvider.java @@ -9,6 +9,7 @@ import java.time.Duration; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; @@ -18,8 +19,8 @@ import io.modelcontextprotocol.json.McpJsonMapper; import io.modelcontextprotocol.json.TypeRef; import io.modelcontextprotocol.server.McpTransportContextExtractor; -import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpSchema.JSONRPCResponse.JSONRPCError; import io.modelcontextprotocol.spec.McpServerSession; import io.modelcontextprotocol.spec.McpServerTransport; import io.modelcontextprotocol.spec.McpServerTransportProvider; @@ -65,6 +66,7 @@ * {@link HttpServletStreamableServerTransportProvider} or * {@link HttpServletStatelessServerTransport}. * @author Alexandros Pappas + * @author Taewoong Kim * @see McpServerTransportProvider * @see HttpServlet * @see ctx.put(McpTransportContext.KEY, transportContext)) .block(); - response.setContentType(APPLICATION_JSON); - response.setCharacterEncoding(UTF_8); - response.setStatus(HttpServletResponse.SC_OK); - - String jsonResponseText = jsonMapper.writeValueAsString(jsonrpcResponse); - PrintWriter writer = response.getWriter(); - writer.write(jsonResponseText); - writer.flush(); + HttpServletResponseUtils.writeJsonResponse(response, HttpServletResponse.SC_OK, this.jsonMapper, + jsonrpcResponse); } catch (Exception e) { logger.error("Failed to handle request: {}", e.getMessage()); - this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, - McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR) - .message("Failed to handle request: " + e.getMessage()) - .build()); + this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, jsonrpcRequest.id(), + new JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, + "Failed to handle request: " + e.getMessage())); } } else if (message instanceof McpSchema.JSONRPCNotification jsonrpcNotification) { @@ -225,17 +218,13 @@ else if (message instanceof McpSchema.JSONRPCNotification jsonrpcNotification) { } catch (Exception e) { logger.error("Failed to handle notification: {}", e.getMessage()); - this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, - McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR) - .message("Failed to handle notification: " + e.getMessage()) - .build()); + this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, null, new JSONRPCError( + McpSchema.ErrorCodes.INTERNAL_ERROR, "Failed to handle notification: " + e.getMessage())); } } else { - this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, - McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST) - .message("The server accepts either requests or notifications") - .build()); + this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, null, new JSONRPCError( + McpSchema.ErrorCodes.INVALID_REQUEST, "The server accepts either requests or notifications")); } } catch (MaxSizeExceededException e) { @@ -243,15 +232,13 @@ else if (message instanceof McpSchema.JSONRPCNotification jsonrpcNotification) { } catch (IllegalArgumentException | IOException e) { logger.error("Failed to deserialize message: {}", e.getMessage()); - this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, - McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST).message("Invalid message format").build()); + this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, requestId, + new JSONRPCError(McpSchema.ErrorCodes.INVALID_REQUEST, "Invalid message format")); } catch (Exception e) { logger.error("Unexpected error handling message: {}", e.getMessage()); - this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, - McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR) - .message("Unexpected error: " + e.getMessage()) - .build()); + this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, requestId, + new JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, "Unexpected error: " + e.getMessage())); } } @@ -259,17 +246,14 @@ else if (message instanceof McpSchema.JSONRPCNotification jsonrpcNotification) { * Sends an error response to the client. * @param response The HTTP servlet response * @param httpCode The HTTP status code - * @param mcpError The MCP error to send + * @param requestId The request identifier, or null when unavailable + * @param error The JSON-RPC error to send * @throws IOException If an I/O error occurs */ - private void responseError(HttpServletResponse response, int httpCode, McpError mcpError) throws IOException { - response.setContentType(APPLICATION_JSON); - response.setCharacterEncoding(UTF_8); - response.setStatus(httpCode); - String jsonError = jsonMapper.writeValueAsString(mcpError); - PrintWriter writer = response.getWriter(); - writer.write(jsonError); - writer.flush(); + private void responseError(HttpServletResponse response, int httpCode, Object requestId, JSONRPCError error) + throws IOException { + McpSchema.JSONRPCResponse errorResponse = McpSchema.JSONRPCResponse.error(requestId, error); + HttpServletResponseUtils.writeJsonResponse(response, httpCode, this.jsonMapper, errorResponse); } /** diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStreamableServerTransportProvider.java b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStreamableServerTransportProvider.java index ffc9cbdf4..0fe465be6 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStreamableServerTransportProvider.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStreamableServerTransportProvider.java @@ -9,6 +9,7 @@ import java.time.Duration; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicReference; @@ -22,6 +23,7 @@ import io.modelcontextprotocol.spec.HttpHeaders; import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpSchema.JSONRPCResponse.JSONRPCError; import io.modelcontextprotocol.spec.McpSession; import io.modelcontextprotocol.spec.McpStreamableServerSession; import io.modelcontextprotocol.spec.McpStreamableServerTransport; @@ -61,6 +63,7 @@ * @author Christian Tzolov * @author Dariusz Jędrzejczyk * @author Daniel Garnier-Moiroux + * @author Taewoong Kim * @see McpStreamableServerTransportProvider * @see HttpServlet */ @@ -408,8 +411,8 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) if (!badRequestErrors.isEmpty()) { String combinedMessage = String.join("; ", badRequestErrors); - this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, - McpError.builder(McpSchema.ErrorCodes.METHOD_NOT_FOUND).message(combinedMessage).build()); + this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, null, + new JSONRPCError(McpSchema.ErrorCodes.METHOD_NOT_FOUND, combinedMessage)); return; } @@ -519,27 +522,30 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) } if (!HttpServletRequestUtils.isJsonContentType(request.getContentType())) { - this.responseError(response, HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE, - McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST) - .message("Unsupported Media Type: Content-Type must be application/json") - .build()); + this.responseError(response, HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE, null, + new JSONRPCError(McpSchema.ErrorCodes.INVALID_REQUEST, + "Unsupported Media Type: Content-Type must be application/json")); return; } McpTransportContext transportContext = this.contextExtractor.extract(request); + Object requestId = null; try { String body = HttpServletRequestUtils.readBody(request, this.requestMaxSize); McpSchema.JSONRPCMessage message = McpSchema.deserializeJsonRpcMessage(jsonMapper, body); + if (message instanceof McpSchema.JSONRPCRequest jsonrpcRequest) { + requestId = jsonrpcRequest.id(); + } // Handle initialization request if (message instanceof McpSchema.JSONRPCRequest jsonrpcRequest && jsonrpcRequest.method().equals(McpSchema.METHOD_INITIALIZE)) { if (!badRequestErrors.isEmpty()) { String combinedMessage = String.join("; ", badRequestErrors); - this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, - McpError.builder(McpSchema.ErrorCodes.METHOD_NOT_FOUND).message(combinedMessage).build()); + this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, jsonrpcRequest.id(), + new JSONRPCError(McpSchema.ErrorCodes.METHOD_NOT_FOUND, combinedMessage)); return; } @@ -554,25 +560,18 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) try { McpSchema.InitializeResult initResult = init.initResult().block(); - response.setContentType(APPLICATION_JSON); - response.setCharacterEncoding(UTF_8); + McpSchema.JSONRPCResponse initializeResponse = McpSchema.JSONRPCResponse.result(jsonrpcRequest.id(), + initResult); response.setHeader(HttpHeaders.MCP_SESSION_ID, init.session().getId()); - response.setStatus(HttpServletResponse.SC_OK); - - String jsonResponse = jsonMapper - .writeValueAsString(McpSchema.JSONRPCResponse.result(jsonrpcRequest.id(), initResult)); - - PrintWriter writer = response.getWriter(); - writer.write(jsonResponse); - writer.flush(); + HttpServletResponseUtils.writeJsonResponse(response, HttpServletResponse.SC_OK, this.jsonMapper, + initializeResponse); return; } catch (Exception e) { logger.error("Failed to initialize session: {}", e.getMessage()); - this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, - McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR) - .message("Failed to initialize session: " + e.getMessage()) - .build()); + this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, jsonrpcRequest.id(), + new JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, + "Failed to initialize session: " + e.getMessage())); return; } } @@ -585,18 +584,16 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) if (!badRequestErrors.isEmpty()) { String combinedMessage = String.join("; ", badRequestErrors); - this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, - McpError.builder(McpSchema.ErrorCodes.METHOD_NOT_FOUND).message(combinedMessage).build()); + this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, requestId, + new JSONRPCError(McpSchema.ErrorCodes.METHOD_NOT_FOUND, combinedMessage)); return; } McpStreamableServerSession session = this.sessions.get(sessionId); if (session == null) { - this.responseError(response, HttpServletResponse.SC_NOT_FOUND, - McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR) - .message("Session not found: " + sessionId) - .build()); + this.responseError(response, HttpServletResponse.SC_NOT_FOUND, requestId, + new JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, "Session not found: " + sessionId)); return; } @@ -666,8 +663,8 @@ else if (message instanceof McpSchema.JSONRPCRequest jsonrpcRequest) { } } else { - this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, - McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST).message("Unknown message type").build()); + this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, requestId, + new JSONRPCError(McpSchema.ErrorCodes.INVALID_REQUEST, "Unknown message type")); } } catch (MaxSizeExceededException e) { @@ -675,18 +672,14 @@ else if (message instanceof McpSchema.JSONRPCRequest jsonrpcRequest) { } catch (IllegalArgumentException | IOException e) { logger.error("Failed to deserialize message: {}", e.getMessage()); - this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, - McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST) - .message("Invalid message format: " + e.getMessage()) - .build()); + this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, requestId, new JSONRPCError( + McpSchema.ErrorCodes.INVALID_REQUEST, "Invalid message format: " + e.getMessage())); } catch (Exception e) { logger.error("Error handling message: {}", e.getMessage()); try { - this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, - McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR) - .message("Error processing message: " + e.getMessage()) - .build()); + this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, requestId, new JSONRPCError( + McpSchema.ErrorCodes.INTERNAL_ERROR, "Error processing message: " + e.getMessage())); } catch (IOException ex) { logger.error(FAILED_TO_SEND_ERROR_RESPONSE, ex.getMessage()); @@ -733,10 +726,8 @@ protected void doDelete(HttpServletRequest request, HttpServletResponse response McpTransportContext transportContext = this.contextExtractor.extract(request); if (request.getHeader(HttpHeaders.MCP_SESSION_ID) == null) { - this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, - McpError.builder(McpSchema.ErrorCodes.METHOD_NOT_FOUND) - .message("Session ID required in mcp-session-id header") - .build()); + this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, null, new JSONRPCError( + McpSchema.ErrorCodes.METHOD_NOT_FOUND, "Session ID required in mcp-session-id header")); return; } @@ -756,8 +747,9 @@ protected void doDelete(HttpServletRequest request, HttpServletResponse response catch (Exception e) { logger.error("Failed to delete session {}: {}", sessionId, e.getMessage()); try { - this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, - McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR).message(e.getMessage()).build()); + this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, null, + new JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, + Objects.requireNonNullElse(e.getMessage(), "Internal error"))); } catch (IOException ex) { logger.error(FAILED_TO_SEND_ERROR_RESPONSE, ex.getMessage()); @@ -767,14 +759,13 @@ protected void doDelete(HttpServletRequest request, HttpServletResponse response } public void responseError(HttpServletResponse response, int httpCode, McpError mcpError) throws IOException { - response.setContentType(APPLICATION_JSON); - response.setCharacterEncoding(UTF_8); - response.setStatus(httpCode); - String jsonError = jsonMapper.writeValueAsString(mcpError); - PrintWriter writer = response.getWriter(); - writer.write(jsonError); - writer.flush(); - return; + this.responseError(response, httpCode, null, mcpError.getJsonRpcError()); + } + + private void responseError(HttpServletResponse response, int httpCode, Object requestId, JSONRPCError error) + throws IOException { + McpSchema.JSONRPCResponse errorResponse = McpSchema.JSONRPCResponse.error(requestId, error); + HttpServletResponseUtils.writeJsonResponse(response, httpCode, this.jsonMapper, errorResponse); } /** diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpSchema.java b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpSchema.java index fa467b213..b433d4edd 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpSchema.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpSchema.java @@ -35,6 +35,7 @@ * @author Surbhi Bansal * @author Anurag Pant * @author Dariusz Jędrzejczyk + * @author Taewoong Kim */ public final class McpSchema { @@ -306,9 +307,10 @@ public JSONRPCNotification(String method) { * A response to a request (successful, or error). * * @param jsonrpc The JSON-RPC version (must be "2.0") - * @param id The request identifier that this response corresponds to - * @param result The result of the successful request; null if error - * @param error Error information if the request failed; null if has result + * @param id The request identifier that this response corresponds to. May be null for + * an error response when the request identifier could not be determined. + * @param result The result of the successful request. Null if error + * @param error Error information if the request failed. Null if has result */ @JsonInclude(JsonInclude.Include.NON_ABSENT) @JsonIgnoreProperties(ignoreUnknown = true) @@ -320,10 +322,10 @@ public record JSONRPCResponse( // @formatter:off public JSONRPCResponse { Assert.hasText(jsonrpc, "jsonrpc must not be empty"); - Assert.notNull(id, "MCP responses MUST include an ID - null IDs are not allowed"); - Assert.isTrue(id instanceof String || id instanceof Integer || id instanceof Long, - "MCP responses MUST have an ID that is either a string or integer"); Assert.isTrue((result != null) ^ (error != null), "MCP responses MUST either have a result or error"); + Assert.isTrue(id != null || error != null, "MCP result responses MUST include an ID"); + Assert.isTrue(id == null || id instanceof String || id instanceof Integer || id instanceof Long, + "MCP responses MUST have an ID that is either a string or integer"); } public static JSONRPCResponse result(Object id, Object result) { diff --git a/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletSseIntegrationTests.java b/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletSseIntegrationTests.java index 9777cfe7b..08e8f8f11 100644 --- a/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletSseIntegrationTests.java +++ b/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletSseIntegrationTests.java @@ -46,6 +46,7 @@ import reactor.core.publisher.Mono; import static io.modelcontextprotocol.util.ToolsUtils.EMPTY_JSON_SCHEMA; +import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -312,7 +313,11 @@ void rejectsNonJsonContentType(String contentType) throws Exception { var response = httpClient.send(request, HttpResponse.BodyHandlers.ofString()); assertThat(response.statusCode()).isEqualTo(HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE); - assertThat(response.body()).contains("Unsupported Media Type: Content-Type must be application/json"); + assertThat(response.headers().firstValue("Content-Type").orElseThrow()).startsWith("application/json"); + assertThatJson(response.body()).isEqualTo(""" + {"jsonrpc":"2.0","error":{"code":-32600, + "message":"Unsupported Media Type: Content-Type must be application/json"}} + """); assertThat(toolCalled).isFalse(); } } diff --git a/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java b/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java index 0065ea568..39536ec1d 100644 --- a/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java +++ b/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java @@ -934,29 +934,37 @@ void rejectsNonJsonContentType(String contentType) throws Exception { var response = HttpClient.newHttpClient().send(request, HttpResponse.BodyHandlers.ofString()); assertThat(response.statusCode()).isEqualTo(HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE); - assertThatJson(response.body()).inPath("message") - .isEqualTo("Unsupported Media Type: Content-Type must be application/json"); + assertThat(response.headers().firstValue("Content-Type").orElseThrow()).startsWith("application/json"); + assertThatJson(response.body()).isEqualTo(""" + {"jsonrpc":"2.0","error":{"code":-32600, + "message":"Unsupported Media Type: Content-Type must be application/json"}} + """); assertThat(toolCalled).isFalse(); } - @Test - void rejectsMalformedMessageAsInvalidRequest() throws Exception { + @ParameterizedTest + @ValueSource(strings = { """ + {"jsonrpc":{"a":1},"id":1,"method":"tools/list"} + """, """ + {"jsonrpc":"2.0","id":1,"method":"tools/list" + """ }) + void rejectsMalformedMessageAsInvalidRequest(String body) throws Exception { McpServer.sync(mcpStatelessServerTransport).build(); - // Valid JSON, but "jsonrpc" is an object instead of a string, so it cannot be - // converted into a JSONRPCRequest var request = HttpRequest.newBuilder() .uri(URI.create("http://localhost:" + PORT + CUSTOM_MESSAGE_ENDPOINT)) .header("Content-Type", APPLICATION_JSON) .header("Accept", APPLICATION_JSON + ", " + TEXT_EVENT_STREAM) - .POST(HttpRequest.BodyPublishers.ofString(""" - {"jsonrpc":{"a":1},"id":1,"method":"tools/list"}""")) + .POST(HttpRequest.BodyPublishers.ofString(body)) .build(); var response = HttpClient.newHttpClient().send(request, HttpResponse.BodyHandlers.ofString()); assertThat(response.statusCode()).isEqualTo(HttpServletResponse.SC_BAD_REQUEST); - assertThatJson(response.body()).inPath("message").isEqualTo("Invalid message format"); + assertThat(response.headers().firstValue("Content-Type").orElseThrow()).startsWith("application/json"); + assertThatJson(response.body()).isEqualTo(""" + {"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid message format"}} + """); } private double evaluateExpression(String expression) { diff --git a/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStreamableIntegrationTests.java b/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStreamableIntegrationTests.java index 135e7ced5..b868f3070 100644 --- a/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStreamableIntegrationTests.java +++ b/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStreamableIntegrationTests.java @@ -64,6 +64,7 @@ import static io.modelcontextprotocol.util.ToolsUtils.EMPTY_JSON_SCHEMA; import static java.nio.charset.StandardCharsets.UTF_8; +import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.awaitility.Awaitility.await; @@ -406,7 +407,11 @@ void rejectsInitializeWithNonJsonContentType(String contentType) throws Exceptio var response = httpClient.send(initialize, HttpResponse.BodyHandlers.ofString()); assertThat(response.statusCode()).isEqualTo(HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE); - assertThat(response.body()).contains("Unsupported Media Type: Content-Type must be application/json"); + assertThat(response.headers().firstValue("Content-Type").orElseThrow()).startsWith("application/json"); + assertThatJson(response.body()).isEqualTo(""" + {"jsonrpc":"2.0","error":{"code":-32600, + "message":"Unsupported Media Type: Content-Type must be application/json"}} + """); assertThat(response.headers().firstValue(HttpHeaders.MCP_SESSION_ID)).isEmpty(); } diff --git a/mcp-test/src/test/java/io/modelcontextprotocol/server/transport/HttpServletJsonRpcErrorTests.java b/mcp-test/src/test/java/io/modelcontextprotocol/server/transport/HttpServletJsonRpcErrorTests.java new file mode 100644 index 000000000..daa828af3 --- /dev/null +++ b/mcp-test/src/test/java/io/modelcontextprotocol/server/transport/HttpServletJsonRpcErrorTests.java @@ -0,0 +1,318 @@ +/* + * Copyright 2026-2026 the original author or authors. + */ + +package io.modelcontextprotocol.server.transport; + +import java.io.PrintWriter; +import java.io.StringWriter; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.util.Map; +import java.util.stream.Stream; + +import io.modelcontextprotocol.json.TypeRef; +import io.modelcontextprotocol.server.McpStatelessServerHandler; +import io.modelcontextprotocol.spec.HttpHeaders; +import io.modelcontextprotocol.spec.McpError; +import io.modelcontextprotocol.spec.McpSchema; +import io.modelcontextprotocol.spec.McpServerSession; +import io.modelcontextprotocol.spec.McpStreamableServerSession; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletResponse; +import org.apache.catalina.startup.Tomcat; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; +import reactor.core.publisher.Mono; + +import static io.modelcontextprotocol.util.McpJsonMapperUtils.JSON_MAPPER; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * @author Taewoong Kim + */ +@Timeout(15) +class HttpServletJsonRpcErrorTests { + + private static final HttpClient HTTP_CLIENT = HttpClient.newHttpClient(); + + private Tomcat tomcat; + + private URI endpoint; + + @AfterEach + void stopServer() throws Exception { + if (this.tomcat != null) { + this.tomcat.stop(); + this.tomcat.destroy(); + } + } + + @Test + void statelessInvalidAcceptReturnsErrorWithoutId() throws Exception { + startServer(HttpServletStatelessServerTransport.builder().build()); + + HttpResponse response = send(postRequest(""" + {"jsonrpc":"2.0","id":"unparsed-id","method":"tools/list"} + """).setHeader("Accept", "application/json")); + + assertErrorResponse(response, 400, null, McpSchema.ErrorCodes.METHOD_NOT_FOUND); + } + + @ParameterizedTest + @MethodSource("statelessHandlerFailures") + void statelessHandlerFailureReturnsInternalErrorWithRequestId(RuntimeException failure) throws Exception { + HttpServletStatelessServerTransport transport = HttpServletStatelessServerTransport.builder().build(); + McpStatelessServerHandler handler = mock(McpStatelessServerHandler.class); + when(handler.handleRequest(any(), any())).thenReturn(Mono.error(failure)); + transport.setMcpHandler(handler); + startServer(transport); + + HttpResponse response = post(""" + {"jsonrpc":"2.0","id":"handler-error","method":"tools/list"} + """); + + assertErrorResponse(response, 500, "handler-error", McpSchema.ErrorCodes.INTERNAL_ERROR); + } + + @Test + void statelessNotificationFailureReturnsErrorWithoutId() throws Exception { + HttpServletStatelessServerTransport transport = HttpServletStatelessServerTransport.builder().build(); + McpStatelessServerHandler handler = mock(McpStatelessServerHandler.class); + when(handler.handleNotification(any(), any())).thenReturn(Mono.error(new IllegalStateException("boom"))); + transport.setMcpHandler(handler); + startServer(transport); + + HttpResponse response = post(""" + {"jsonrpc":"2.0","method":"notifications/test"} + """); + + assertErrorResponse(response, 500, null, McpSchema.ErrorCodes.INTERNAL_ERROR); + } + + @Test + void streamableInvalidAcceptPreservesParsedId() throws Exception { + startServer(HttpServletStreamableServerTransportProvider.builder().build()); + + HttpResponse response = send(postRequest(""" + {"jsonrpc":"2.0","id":"init-request","method":"initialize"} + """).setHeader("Accept", "application/json")); + + assertErrorResponse(response, 400, "init-request", McpSchema.ErrorCodes.METHOD_NOT_FOUND); + } + + @Test + void streamableInvalidInitializeParamsPreserveParsedId() throws Exception { + startServer(HttpServletStreamableServerTransportProvider.builder().build()); + + HttpResponse response = post(""" + {"jsonrpc":"2.0","id":"bad-initialize","method":"initialize","params":[]} + """); + + assertErrorResponse(response, 400, "bad-initialize", McpSchema.ErrorCodes.INVALID_REQUEST); + } + + @Test + void streamableUnknownSessionPreservesNumericId() throws Exception { + startServer(HttpServletStreamableServerTransportProvider.builder().build()); + + HttpResponse response = send(postRequest(""" + {"jsonrpc":"2.0","id":42,"method":"tools/list"} + """).header(HttpHeaders.MCP_SESSION_ID, "missing-session")); + + assertErrorResponse(response, 404, 42, McpSchema.ErrorCodes.INTERNAL_ERROR); + } + + @Test + void streamableInitializationFailurePreservesId() throws Exception { + HttpServletStreamableServerTransportProvider transport = HttpServletStreamableServerTransportProvider.builder() + .build(); + McpStreamableServerSession session = mock(McpStreamableServerSession.class); + when(session.getId()).thenReturn("session-1"); + when(session.closeGracefully()).thenReturn(Mono.empty()); + transport.setSessionFactory(request -> new McpStreamableServerSession.McpStreamableServerSessionInit(session, + Mono.error(new IllegalStateException("initialization failed")))); + startServer(transport); + + HttpResponse response = post(""" + {"jsonrpc":"2.0","id":"init-failed","method":"initialize","params":{ + "protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"test","version":"1"}}} + """); + + assertErrorResponse(response, 500, "init-failed", McpSchema.ErrorCodes.INTERNAL_ERROR); + } + + @ParameterizedTest + @ValueSource(strings = { "GET", "DELETE" }) + void streamableMissingSessionIdReturnsErrorWithoutId(String method) throws Exception { + startServer(HttpServletStreamableServerTransportProvider.builder().build()); + + HttpResponse response = send(HttpRequest.newBuilder(this.endpoint) + .header("Accept", "text/event-stream") + .method(method, HttpRequest.BodyPublishers.noBody())); + + assertErrorResponse(response, 400, null, McpSchema.ErrorCodes.METHOD_NOT_FOUND); + } + + @ParameterizedTest + @CsvSource({ "deletion failed, deletion failed", ", Internal error" }) + void streamableDeletionFailureReturnsErrorWithoutId(String exceptionMessage, String expectedErrorMessage) + throws Exception { + HttpServletStreamableServerTransportProvider transport = HttpServletStreamableServerTransportProvider.builder() + .build(); + McpStreamableServerSession session = mock(McpStreamableServerSession.class); + when(session.getId()).thenReturn("session-1"); + when(session.delete()).thenThrow(new IllegalStateException(exceptionMessage)); + when(session.closeGracefully()).thenReturn(Mono.empty()); + McpSchema.InitializeResult initializeResult = McpSchema.InitializeResult + .builder("2025-11-25", McpSchema.ServerCapabilities.builder().build(), + new McpSchema.Implementation("test", "1")) + .build(); + transport.setSessionFactory(request -> new McpStreamableServerSession.McpStreamableServerSessionInit(session, + Mono.just(initializeResult))); + startServer(transport); + HttpResponse initializeResponse = post(""" + {"jsonrpc":"2.0","id":"init","method":"initialize","params":{ + "protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"test","version":"1"}}} + """); + assertThat(initializeResponse.statusCode()).isEqualTo(200); + String sessionId = initializeResponse.headers().firstValue(HttpHeaders.MCP_SESSION_ID).orElseThrow(); + + HttpResponse response = send( + HttpRequest.newBuilder(this.endpoint).header(HttpHeaders.MCP_SESSION_ID, sessionId).DELETE()); + + McpSchema.JSONRPCResponse errorResponse = assertErrorResponse(response, 500, null, + McpSchema.ErrorCodes.INTERNAL_ERROR); + assertThat(errorResponse.error().message()).isEqualTo(expectedErrorMessage); + } + + @ParameterizedTest + @CsvSource({ ", 400, -32601", "missing-session, 404, -32603" }) + void sseMissingOrUnknownSessionReturnsErrorWithoutId(String sessionId, int httpStatus, int errorCode) + throws Exception { + startServer(HttpServletSseServerTransportProvider.builder().messageEndpoint("/mcp").build()); + if (sessionId != null) { + this.endpoint = URI.create(this.endpoint + "?sessionId=" + sessionId); + } + + HttpResponse response = post("{}"); + + assertErrorResponse(response, httpStatus, null, errorCode); + } + + @ParameterizedTest + @CsvSource({ "processing failed, processing failed", ", Internal error" }) + void sseProcessingFailurePreservesParsedId(String exceptionMessage, String expectedErrorMessage) throws Exception { + HttpServletSseServerTransportProvider transport = HttpServletSseServerTransportProvider.builder() + .messageEndpoint("/mcp") + .sseEndpoint("/sse") + .build(); + McpServerSession session = mock(McpServerSession.class); + when(session.handle(any())).thenReturn(Mono.error(new IllegalStateException(exceptionMessage))); + when(session.closeGracefully()).thenReturn(Mono.empty()); + transport.setSessionFactory(sessionTransport -> session); + startServer(transport); + HttpRequest request = HttpRequest.newBuilder(this.endpoint.resolve("/sse")) + .header("Accept", "text/event-stream") + .build(); + HttpResponse> streamResponse = HTTP_CLIENT.send(request, HttpResponse.BodyHandlers.ofLines()); + try (Stream events = streamResponse.body()) { + String endpointEvent = events.filter(line -> line.startsWith("data: ")).findFirst().orElseThrow(); + URI messageEndpoint = this.endpoint.resolve(endpointEvent.substring("data: ".length())); + + HttpResponse response = send(postRequest(""" + {"jsonrpc":"2.0","id":"sse-request","method":"tools/list"} + """).uri(messageEndpoint)); + + McpSchema.JSONRPCResponse errorResponse = assertErrorResponse(response, 500, "sse-request", + McpSchema.ErrorCodes.INTERNAL_ERROR); + assertThat(errorResponse.error().message()).isEqualTo(expectedErrorMessage); + } + } + + @Test + void streamableResponseErrorUnwrapsMcpErrorWithoutExceptionFields() throws Exception { + HttpServletStreamableServerTransportProvider transport = HttpServletStreamableServerTransportProvider.builder() + .keepAliveInterval(null) + .sessionSweepInterval(null) + .build(); + McpError error = McpError.builder(McpSchema.ErrorCodes.INVALID_PARAMS) + .message("Invalid request") + .data(Map.of("argument", "name")) + .build(); + error.initCause(new IllegalStateException("Internal cause")); + error.addSuppressed(new IllegalStateException("Internal suppressed exception")); + StringWriter body = new StringWriter(); + HttpServletResponse response = mock(HttpServletResponse.class); + when(response.getWriter()).thenReturn(new PrintWriter(body)); + + transport.responseError(response, 400, error); + + verify(response).setStatus(400); + verify(response).setContentType("application/json"); + verify(response).setCharacterEncoding("UTF-8"); + McpSchema.JSONRPCResponse errorResponse = JSON_MAPPER.readValue(body.toString(), + McpSchema.JSONRPCResponse.class); + assertThat(errorResponse.id()).isNull(); + assertThat(errorResponse.error()).isEqualTo(error.getJsonRpcError()); + assertThat(body.toString()).doesNotContain("stackTrace", "cause", "suppressed", "jsonRpcError"); + } + + static Stream statelessHandlerFailures() { + return Stream.of(new IllegalStateException("boom"), + McpError.builder(McpSchema.ErrorCodes.METHOD_NOT_FOUND).message("Missing handler").build()); + } + + private void startServer(HttpServlet servlet) throws Exception { + this.tomcat = TomcatTestUtil.createTomcatServer("", 0, servlet); + this.tomcat.start(); + this.endpoint = URI.create("http://localhost:" + this.tomcat.getConnector().getLocalPort() + "/mcp"); + } + + private HttpRequest.Builder postRequest(String body) { + return HttpRequest.newBuilder(this.endpoint) + .header("Content-Type", "application/json") + .header("Accept", "application/json, text/event-stream") + .POST(HttpRequest.BodyPublishers.ofString(body)); + } + + private HttpResponse post(String body) throws Exception { + return send(postRequest(body)); + } + + private HttpResponse send(HttpRequest.Builder request) throws Exception { + return HTTP_CLIENT.send(request.build(), HttpResponse.BodyHandlers.ofString()); + } + + private McpSchema.JSONRPCResponse assertErrorResponse(HttpResponse response, int expectedHttpStatus, + Object expectedId, int expectedErrorCode) throws Exception { + assertThat(response.statusCode()).isEqualTo(expectedHttpStatus); + assertThat(response.headers().firstValue("Content-Type").orElseThrow()).startsWith("application/json"); + Map body = JSON_MAPPER.readValue(response.body(), new TypeRef>() { + }); + if (expectedId == null) { + assertThat(body).containsOnlyKeys("jsonrpc", "error"); + } + else { + assertThat(body).containsOnlyKeys("jsonrpc", "id", "error"); + } + McpSchema.JSONRPCResponse errorResponse = JSON_MAPPER.readValue(response.body(), + McpSchema.JSONRPCResponse.class); + assertThat(errorResponse.jsonrpc()).isEqualTo(McpSchema.JSONRPC_VERSION); + assertThat(errorResponse.id()).isEqualTo(expectedId); + assertThat(errorResponse.error().code()).isEqualTo(expectedErrorCode); + return errorResponse; + } + +} diff --git a/mcp-test/src/test/java/io/modelcontextprotocol/spec/McpSchemaTests.java b/mcp-test/src/test/java/io/modelcontextprotocol/spec/McpSchemaTests.java index a9cddf41b..03f92802c 100644 --- a/mcp-test/src/test/java/io/modelcontextprotocol/spec/McpSchemaTests.java +++ b/mcp-test/src/test/java/io/modelcontextprotocol/spec/McpSchemaTests.java @@ -30,6 +30,7 @@ /** * @author Christian Tzolov * @author Anurag Pant + * @author Taewoong Kim */ public class McpSchemaTests { @@ -326,6 +327,58 @@ void testJSONRPCResponseWithError() throws Exception { {"jsonrpc":"2.0","id":1,"error":{"code":-32600,"message":"Invalid request"}}""")); } + @Test + void jsonRpcErrorResponseWithoutIdOmitsIdOnWire() throws Exception { + McpSchema.JSONRPCResponse.JSONRPCError error = new McpSchema.JSONRPCResponse.JSONRPCError( + McpSchema.ErrorCodes.INVALID_REQUEST, "Invalid request"); + McpSchema.JSONRPCResponse response = McpSchema.JSONRPCResponse.error(null, error); + + String value = JSON_MAPPER.writeValueAsString(response); + + assertThatJson(value).isEqualTo(""" + {"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid request"}} + """); + } + + @Test + void jsonRpcErrorResponseWithoutIdDeserializes() throws Exception { + McpSchema.JSONRPCResponse response = JSON_MAPPER.readValue(""" + {"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid request"}} + """, McpSchema.JSONRPCResponse.class); + + assertThat(response.id()).isNull(); + assertThat(response.result()).isNull(); + assertThat(response.error().code()).isEqualTo(McpSchema.ErrorCodes.INVALID_REQUEST); + } + + @Test + void jsonRpcErrorResponseWithoutIdIgnoresUnknownFields() throws Exception { + McpSchema.JSONRPCResponse response = JSON_MAPPER.readValue(""" + {"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid request"},"futureField":true} + """, McpSchema.JSONRPCResponse.class); + + assertThat(response.id()).isNull(); + assertThat(response.error().message()).isEqualTo("Invalid request"); + } + + @Test + void jsonRpcResultResponseRequiresId() { + assertThatThrownBy(() -> McpSchema.JSONRPCResponse.result(null, Map.of())) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> JSON_MAPPER.readValue(""" + {"jsonrpc":"2.0","result":{}} + """, McpSchema.JSONRPCResponse.class)).isInstanceOf(IOException.class); + } + + @Test + void jsonRpcErrorResponseRejectsNonIntegralId() { + McpSchema.JSONRPCResponse.JSONRPCError error = new McpSchema.JSONRPCResponse.JSONRPCError( + McpSchema.ErrorCodes.INVALID_REQUEST, "Invalid request"); + + assertThatThrownBy(() -> McpSchema.JSONRPCResponse.error(1.5, error)) + .isInstanceOf(IllegalArgumentException.class); + } + // Initialization Tests @Test