From 507eb2829462691b79aeb30372380f6afaad09e1 Mon Sep 17 00:00:00 2001 From: Sebastian Ratz Date: Wed, 4 Nov 2026 19:59:15 +0100 Subject: [PATCH] Fix non-compliant JSON-RPC error responses in servlet transports The servlet transports returned error bodies that did not conform to the MCP JSON-RPC response format: they omitted the required "jsonrpc" field, used "jsonRpcError" instead of "error", and did not include the request ID even where it was available. These malformed responses resulted from serializing McpError directly rather than constructing a JSONRPCResponse error envelope. Because McpError extends RuntimeException, this also caused information disclosure: Jackson included exception internals such as stack traces in the response. Before (wrong; illustrative response with an abbreviated stack trace): ```json { "jsonRpcError": { "code": -123, "message": "foo" }, "message": "foo", "localizedMessage": "foo", "cause": null, "suppressed": [], "stackTrace": [ { "className": "io.modelcontextprotocol.spec.McpError$Builder", "methodName": "build", "fileName": "McpError.java", "lineNumber": 81, "nativeMethod": false } ] } ``` After (correct; no request ID is available in this example): ```json { "jsonrpc": "2.0", "error": { "code": -123, "message": "foo" } } ``` Write typed JSONRPCResponse envelopes across the SSE, streamable, and stateless servlet transports. Allow error responses without an ID while continuing to require IDs on successful responses, and propagate request IDs where already parsed. Preserve the public McpError response writer as a wrapper that unwraps the protocol error. Add regression coverage for HTTP error envelopes, request IDs, and exclusion of exception properties, including causes and suppressed exceptions. Update tests that expected a top-level exception message. --- .../transport/HttpServletResponseUtils.java | 17 ++ ...HttpServletSseServerTransportProvider.java | 59 ++----- .../HttpServletStatelessServerTransport.java | 62 +++---- ...vletStreamableServerTransportProvider.java | 78 ++++---- .../modelcontextprotocol/spec/McpSchema.java | 9 +- .../HttpServletStatelessIntegrationTests.java | 6 +- .../HttpServletErrorResponseTests.java | 166 ++++++++++++++++++ .../spec/JsonRpcDispatchTests.java | 36 ++++ 8 files changed, 308 insertions(+), 125 deletions(-) create mode 100644 mcp-test/src/test/java/io/modelcontextprotocol/server/transport/HttpServletErrorResponseTests.java 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..842e16170 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,7 +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; /** @@ -20,6 +23,20 @@ final class HttpServletResponseUtils { private HttpServletResponseUtils() { } + /** + * Writes a protocol response rather than serializing an exception as a Java bean. + */ + static void writeJsonResponse(HttpServletResponse response, int httpCode, McpJsonMapper jsonMapper, + McpSchema.JSONRPCResponse message) throws IOException { + String json = jsonMapper.writeValueAsString(message); + response.setContentType("application/json"); + response.setCharacterEncoding("UTF-8"); + response.setStatus(httpCode); + 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..09b10f15c 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 @@ -18,8 +18,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; @@ -375,48 +375,37 @@ 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; } // Get the session ID from the request parameter String sessionId = request.getParameter("sessionId"); if (sessionId == null) { - response.setContentType(APPLICATION_JSON); - response.setCharacterEncoding(UTF_8); - response.setStatus(HttpServletResponse.SC_BAD_REQUEST); - String jsonError = jsonMapper.writeValueAsString(McpError.builder(McpSchema.ErrorCodes.METHOD_NOT_FOUND) - .message("Session ID missing in message endpoint") - .build()); - PrintWriter writer = response.getWriter(); - writer.write(jsonError); - writer.flush(); + this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, null, + new JSONRPCError(McpSchema.ErrorCodes.METHOD_NOT_FOUND, "Session ID missing in message endpoint")); return; } // Get the session from the sessions map McpServerSession session = sessions.get(sessionId); if (session == null) { - response.setContentType(APPLICATION_JSON); - response.setCharacterEncoding(UTF_8); - response.setStatus(HttpServletResponse.SC_NOT_FOUND); - String jsonError = jsonMapper.writeValueAsString(McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR) - .message("Session not found: " + sessionId) - .build()); - PrintWriter writer = response.getWriter(); - writer.write(jsonError); - writer.flush(); + this.responseError(response, HttpServletResponse.SC_NOT_FOUND, null, + new JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, "Session not found: " + sessionId)); return; } + Object requestId = null; try { String body = HttpServletRequestUtils.readBody(request, this.requestMaxSize); final McpTransportContext transportContext = this.contextExtractor.extract(request); McpSchema.JSONRPCMessage message = McpSchema.deserializeJsonRpcMessage(jsonMapper, body); + if (message instanceof McpSchema.JSONRPCRequest jsonrpcRequest) { + requestId = jsonrpcRequest.id(); + } // Process the message through the session's handle method // Block for Servlet compatibility @@ -430,16 +419,8 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) catch (Exception e) { logger.error("Error processing message: {}", e.getMessage()); try { - McpError mcpError = McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR) - .message(e.getMessage()) - .build(); - response.setContentType(APPLICATION_JSON); - response.setCharacterEncoding(UTF_8); - response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); - String jsonError = jsonMapper.writeValueAsString(mcpError); - PrintWriter writer = response.getWriter(); - writer.write(jsonError); - writer.flush(); + this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, requestId, + new JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, e.getMessage())); } catch (IOException ex) { logger.error(FAILED_TO_SEND_ERROR_RESPONSE, ex.getMessage()); @@ -486,14 +467,10 @@ private void sendEvent(PrintWriter writer, String eventType, String data) throws } } - 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 { + HttpServletResponseUtils.writeJsonResponse(response, httpCode, this.jsonMapper, + McpSchema.JSONRPCResponse.error(requestId, error)); } /** diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStatelessServerTransport.java b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStatelessServerTransport.java index 4d346b732..8b771bd43 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStatelessServerTransport.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletStatelessServerTransport.java @@ -17,8 +17,8 @@ import io.modelcontextprotocol.common.McpTransportContext; import io.modelcontextprotocol.server.McpStatelessServerHandler; 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.McpStatelessServerTransport; import io.modelcontextprotocol.util.Assert; import jakarta.servlet.ServletException; @@ -169,30 +169,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; } String accept = request.getHeader(ACCEPT); if (accept == null || !(accept.contains(APPLICATION_JSON) && accept.contains(TEXT_EVENT_STREAM))) { - this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, - McpError.builder(McpSchema.ErrorCodes.METHOD_NOT_FOUND) - .message("Both application/json and text/event-stream required in Accept header") - .build()); + this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, null, + new JSONRPCError(McpSchema.ErrorCodes.METHOD_NOT_FOUND, + "Both application/json and text/event-stream required in Accept header")); 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(); try { McpSchema.JSONRPCResponse jsonrpcResponse = this.mcpHandler .handleRequest(transportContext, jsonrpcRequest) @@ -210,10 +210,9 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) } 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, requestId, + new JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, + "Failed to handle request: " + e.getMessage())); } } else if (message instanceof McpSchema.JSONRPCNotification jsonrpcNotification) { @@ -225,17 +224,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 +238,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 +252,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 ID, if available + * @param error The protocol 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 { + HttpServletResponseUtils.writeJsonResponse(response, httpCode, this.jsonMapper, + McpSchema.JSONRPCResponse.error(requestId, error)); } /** 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..1e43c79ab 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 @@ -22,6 +22,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; @@ -408,8 +409,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 +520,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, requestId, + new JSONRPCError(McpSchema.ErrorCodes.METHOD_NOT_FOUND, combinedMessage)); return; } @@ -569,10 +573,9 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) } 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, requestId, + new JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, + "Failed to initialize session: " + e.getMessage())); return; } } @@ -585,18 +588,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 +667,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 +676,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 +730,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 +751,8 @@ 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, e.getMessage())); } catch (IOException ex) { logger.error(FAILED_TO_SEND_ERROR_RESPONSE, ex.getMessage()); @@ -767,14 +762,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 { + HttpServletResponseUtils.writeJsonResponse(response, httpCode, this.jsonMapper, + McpSchema.JSONRPCResponse.error(requestId, error)); } /** 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..100d7e441 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpSchema.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpSchema.java @@ -306,7 +306,8 @@ 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 id The request identifier that this response corresponds to; may be null for + * an error when the request ID 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 */ @@ -320,10 +321,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/HttpServletStatelessIntegrationTests.java b/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java index 0065ea568..58b121ef4 100644 --- a/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java +++ b/mcp-test/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java @@ -934,7 +934,7 @@ 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") + assertThatJson(response.body()).inPath("error.message") .isEqualTo("Unsupported Media Type: Content-Type must be application/json"); assertThat(toolCalled).isFalse(); } @@ -956,7 +956,9 @@ void rejectsMalformedMessageAsInvalidRequest() throws Exception { 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"); + 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/transport/HttpServletErrorResponseTests.java b/mcp-test/src/test/java/io/modelcontextprotocol/server/transport/HttpServletErrorResponseTests.java new file mode 100644 index 000000000..7448251e7 --- /dev/null +++ b/mcp-test/src/test/java/io/modelcontextprotocol/server/transport/HttpServletErrorResponseTests.java @@ -0,0 +1,166 @@ +/* + * 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 io.modelcontextprotocol.server.McpStatelessServerHandler; +import io.modelcontextprotocol.spec.McpError; +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.ValueSource; +import reactor.core.publisher.Mono; + +import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; +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.when; + +@Timeout(15) +class HttpServletErrorResponseTests { + + private Tomcat tomcat; + + private URI endpoint; + + private void start(HttpServlet servlet) throws Exception { + this.tomcat = TomcatTestUtil.createTomcatServer("", 0, servlet); + this.tomcat.start(); + this.endpoint = URI.create("http://localhost:" + this.tomcat.getConnector().getLocalPort() + "/mcp"); + } + + @AfterEach + void stop() throws Exception { + if (this.tomcat != null) { + this.tomcat.stop(); + this.tomcat.destroy(); + } + } + + @ParameterizedTest + @ValueSource(strings = { "sse", "streamable", "stateless" }) + void earlyErrorsHaveOnlyProtocolFields(String transport) throws Exception { + HttpServlet servlet = switch (transport) { + case "sse" -> HttpServletSseServerTransportProvider.builder().messageEndpoint("/mcp").build(); + case "streamable" -> HttpServletStreamableServerTransportProvider.builder().mcpEndpoint("/mcp").build(); + case "stateless" -> HttpServletStatelessServerTransport.builder().messageEndpoint("/mcp").build(); + default -> throw new IllegalArgumentException(transport); + }; + start(servlet); + var response = post("text/plain", "{}"); + assertThat(response.statusCode()).isEqualTo(415); + 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"}} + """); + } + + @ParameterizedTest + @ValueSource(strings = { "", "?sessionId=missing" }) + void sseSessionErrorsHaveOnlyProtocolFields(String query) throws Exception { + start(HttpServletSseServerTransportProvider.builder().messageEndpoint("/mcp").build()); + this.endpoint = URI.create(this.endpoint + query); + var response = post("application/json", "{}"); + assertThat(response.statusCode()).isEqualTo(query.isEmpty() ? 400 : 404); + assertThatJson(response.body()).isEqualTo(query.isEmpty() ? """ + {"jsonrpc":"2.0","error":{"code":-32601,"message":"Session ID missing in message endpoint"}} + """ : """ + {"jsonrpc":"2.0","error":{"code":-32603,"message":"Session not found: missing"}} + """); + } + + @Test + void streamableErrorPreservesParsedRequestId() throws Exception { + start(HttpServletStreamableServerTransportProvider.builder().mcpEndpoint("/mcp").build()); + var response = post("application/json", """ + {"jsonrpc":"2.0","id":"req-1","method":"ping"} + """); + assertThat(response.statusCode()).isEqualTo(400); + assertThatJson(response.body()).isEqualTo(""" + {"jsonrpc":"2.0","id":"req-1","error":{"code":-32601, + "message":"Session ID required in mcp-session-id header"}} + """); + } + + @Test + void initializationFailurePreservesRequestId() throws Exception { + var transport = HttpServletStreamableServerTransportProvider.builder().mcpEndpoint("/mcp").build(); + var 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 RuntimeException("initialization failed")))); + start(transport); + var response = post("application/json", """ + {"jsonrpc":"2.0","id":17,"method":"initialize","params":{ + "protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"test","version":"1"}}} + """); + assertThat(response.statusCode()).isEqualTo(500); + assertThatJson(response.body()).isEqualTo(""" + {"jsonrpc":"2.0","id":17,"error":{"code":-32603, + "message":"Failed to initialize session: initialization failed"}} + """); + } + + @Test + void statelessHandlerFailurePreservesRequestId() throws Exception { + var transport = HttpServletStatelessServerTransport.builder().messageEndpoint("/mcp").build(); + var handler = mock(McpStatelessServerHandler.class); + when(handler.handleRequest(any(), any())).thenReturn(Mono.error(new RuntimeException("handler failed"))); + transport.setMcpHandler(handler); + start(transport); + var response = post("application/json", """ + {"jsonrpc":"2.0","id":23,"method":"ping"} + """); + assertThat(response.statusCode()).isEqualTo(500); + assertThatJson(response.body()).isEqualTo(""" + {"jsonrpc":"2.0","id":23,"error":{"code":-32603,"message":"Failed to handle request: handler failed"}} + """); + } + + @Test + void publicErrorWriterUnwrapsExceptionIncludingCauseAndSuppressed() throws Exception { + var transport = HttpServletStreamableServerTransportProvider.builder().mcpEndpoint("/mcp").build(); + try { + var error = McpError.builder(-123).message("foo").data(Map.of("detail", "public")).build(); + error.initCause(new RuntimeException("private cause")); + error.addSuppressed(new RuntimeException("private suppressed")); + var output = new StringWriter(); + var response = mock(HttpServletResponse.class); + when(response.getWriter()).thenReturn(new PrintWriter(output)); + transport.responseError(response, 400, error); + assertThatJson(output.toString()).isEqualTo(""" + {"jsonrpc":"2.0","error":{"code":-123,"message":"foo","data":{"detail":"public"}}} + """); + } + finally { + transport.closeGracefully().block(); + } + } + + private HttpResponse post(String contentType, String body) throws Exception { + var request = HttpRequest.newBuilder(this.endpoint) + .header("Content-Type", contentType) + .header("Accept", "application/json, text/event-stream") + .POST(HttpRequest.BodyPublishers.ofString(body)) + .build(); + return HttpClient.newHttpClient().send(request, HttpResponse.BodyHandlers.ofString()); + } + +} diff --git a/mcp-test/src/test/java/io/modelcontextprotocol/spec/JsonRpcDispatchTests.java b/mcp-test/src/test/java/io/modelcontextprotocol/spec/JsonRpcDispatchTests.java index 6e5a6efb2..ff89814e2 100644 --- a/mcp-test/src/test/java/io/modelcontextprotocol/spec/JsonRpcDispatchTests.java +++ b/mcp-test/src/test/java/io/modelcontextprotocol/spec/JsonRpcDispatchTests.java @@ -5,7 +5,10 @@ package io.modelcontextprotocol.spec; import static io.modelcontextprotocol.util.McpJsonMapperUtils.JSON_MAPPER; +import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.io.IOException; import java.util.Map; @@ -13,6 +16,8 @@ import io.modelcontextprotocol.json.McpJsonMapper; import io.modelcontextprotocol.json.TypeRef; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; /** * Verifies that {@link McpSchema#deserializeJsonRpcMessage} dispatches to the correct @@ -97,4 +102,35 @@ void paramsMapSurvivesConvertValue() throws IOException { assertThat(((Number) args.get("x")).intValue()).isEqualTo(42); } + @ParameterizedTest + @ValueSource(strings = { "", ",\"futureField\":true" }) + void errorWithoutIdRoundTrips(String extraField) throws IOException { + String json = """ + {"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid request"}%s} + """.formatted(extraField); + var response = (McpSchema.JSONRPCResponse) McpSchema.deserializeJsonRpcMessage(mapper, json); + assertThat(response.id()).isNull(); + assertThatJson(mapper.writeValueAsString(response)).isEqualTo(""" + {"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid request"}} + """); + } + + @Test + void resultStillRequiresId() { + assertThatIllegalArgumentException().isThrownBy(() -> McpSchema.JSONRPCResponse.result(null, Map.of())); + assertThatThrownBy(() -> McpSchema.deserializeJsonRpcMessage(mapper, """ + {"jsonrpc":"2.0","result":{}} + """)).isInstanceOf(IllegalArgumentException.class); + } + + @Test + void errorStillRequiresValidIdTypeAndExclusivePayload() { + var error = new McpSchema.JSONRPCResponse.JSONRPCError(-32600, "Invalid request"); + assertThatIllegalArgumentException().isThrownBy(() -> McpSchema.JSONRPCResponse.error(true, error)); + assertThatIllegalArgumentException().isThrownBy(() -> McpSchema.JSONRPCResponse.error(1.5, error)); + assertThatIllegalArgumentException() + .isThrownBy(() -> new McpSchema.JSONRPCResponse("2.0", null, Map.of(), error)); + assertThatIllegalArgumentException().isThrownBy(() -> McpSchema.JSONRPCResponse.error(null, null)); + } + }