Skip to content

Fix non-compliant JSON-RPC error responses in servlet transports #1314

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Closed
sratz wants to merge 1 commit into from
+308 −125
Closed

Fix non-compliant JSON-RPC error responses in servlet transports #1314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions ...core/src/main/java/io/modelcontextprotocol/server/transport/HttpServletResponseUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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
Expand Down
59 changes: 18 additions & 41 deletions .../java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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());
Expand Down Expand Up @@ -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));
}

/**
Expand Down
62 changes: 26 additions & 36 deletions ...in/java/io/modelcontextprotocol/server/transport/HttpServletStatelessServerTransport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand All @@ -225,51 +224,42 @@ 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) {
response.sendError(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE);
}
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()));
}
}

/**
* 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));
}

/**
Expand Down
Loading
Loading