mcp-json-jackson3: malformed request bodies are reported as 500 INTERNAL_ERROR instead of 400 INVALID_REQUEST #1166

Summary

With mcp-json-jackson3 providing the McpJsonMapper (the default when depending on the mcp aggregator artifact), a request body that Jackson fails to convert into a protocol record is reported to the client as HTTP 500 / INTERNAL_ERROR. With mcp-json-jackson2, the same body is correctly reported as HTTP 400 / INVALID_REQUEST.

The two modules differ because convertValue throws a different exception type in Jackson 2 and Jackson 3, while the servlet transports only classify IllegalArgumentException | IOException as a client error.

I'm working on a Java MCP server (Spring AI 2 / MCP Java SDK 2.x) and wanted to confirm that malformed payloads from LLM callers come back as a client error the caller can act on, rather than being reported as a server fault.

Environment

• MCP Java SDK: 2.1.0-SNAPSHOT, commit 88863fc
• Comparison: mcp-core + mcp-json-jackson3 (tools.jackson.core:jackson-databind:3.1.4) vs mcp-core + mcp-json-jackson2 (com.fasterxml.jackson.core:jackson-databind:2.21.5; the project pins 2.21.1)
• JDK 17
• Affected transports: HttpServletStatelessServerTransport, HttpServletStreamableServerTransportProvider

Steps to reproduce

The reproducer only uses SDK public API and does not need a servlet container: it calls the same method the transports call inside their try block (McpSchema.deserializeJsonRpcMessage), and the same conversion a transport performs on initialize params.

import io.modelcontextprotocol.json.McpJsonMapper;
import io.modelcontextprotocol.spec.McpSchema;

public class Repro {

    static McpJsonMapper j2 = new io.modelcontextprotocol.json.jackson2.JacksonMcpJsonMapperSupplier().get();
    static McpJsonMapper j3 = new io.modelcontextprotocol.json.jackson3.JacksonMcpJsonMapperSupplier().get();

    static void deserialize(String label, McpJsonMapper mapper, String body) {
        try {
            McpSchema.deserializeJsonRpcMessage(mapper, body);
            System.out.println(label + ": no exception");
        }
        catch (Throwable e) {
            System.out.printf("%-10s deserializeJsonRpcMessage: %s (IllegalArgumentException=%s, IOException=%s)%n",
                    label, e.getClass().getName(),
                    e instanceof IllegalArgumentException, e instanceof java.io.IOException);
        }
    }

    static void convertParams(String label, McpJsonMapper mapper) {
        try {
            mapper.convertValue("not-an-object", McpSchema.InitializeRequest.class);
            System.out.println(label + ": no exception");
        }
        catch (Throwable e) {
            System.out.printf("%-10s convertValue(params, InitializeRequest.class): %s (IllegalArgumentException=%s)%n",
                    label, e.getClass().getName(), e instanceof IllegalArgumentException);
        }
    }

    public static void main(String[] args) {
        // 1. envelope type mismatch: `jsonrpc` is an object instead of a string
        String body = "{\"jsonrpc\":{\"a\":1},\"id\":1,\"method\":\"tools/list\"}";
        deserialize("jackson2", j2, body);
        deserialize("jackson3", j3, body);

        // 2. malformed initialize params: a string instead of an object
        convertParams("jackson2", j2);
        convertParams("jackson3", j3);
    }
}

Actual output:

jackson2   deserializeJsonRpcMessage: java.lang.IllegalArgumentException (IllegalArgumentException=true, IOException=false)
jackson3   deserializeJsonRpcMessage: tools.jackson.databind.exc.MismatchedInputException (IllegalArgumentException=false, IOException=false)
jackson2   convertValue(params, InitializeRequest.class): java.lang.IllegalArgumentException (IllegalArgumentException=true)
jackson3   convertValue(params, InitializeRequest.class): tools.jackson.databind.exc.MismatchedInputException (IllegalArgumentException=false)

Expected behavior

A malformed body is rejected with HTTP 400 and a JSON-RPC error carrying INVALID_REQUEST, regardless of which JSON binding module is installed — invalid input is a client error, not a server fault. McpSchema.deserializeJsonRpcMessage documents exactly that expectation:

// mcp-core/src/main/java/io/modelcontextprotocol/spec/McpSchema.java:207
 * @throws IllegalArgumentException If the JSON structure doesn't match any known
 * message type

Actual behavior

The client is told the server failed, the server logs at ERROR level, and monitoring counts these as server faults.

Root cause

Three individually reasonable pieces combine badly:

1. The deserialization helper uses convertValue — mcp-core/src/main/java/io/modelcontextprotocol/spec/McpSchema.java:210-228:

214:  var map = jsonMapper.readValue(jsonText, MAP_TYPE_REF);
218:  return jsonMapper.convertValue(map, JSONRPCRequest.class);
221:  return jsonMapper.convertValue(map, JSONRPCNotification.class);
224:  return jsonMapper.convertValue(map, JSONRPCResponse.class);

2. The servlet transports treat only IllegalArgumentException | IOException as a client error — HttpServletStatelessServerTransport.java:244-248 and HttpServletStreamableServerTransportProvider.java:641-646:

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());
}
catch (Exception e) {                       // ← jackson3 conversion failures land here
    this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
            McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR) ... );
}

3. convertValue throws different exception types in the two Jackson generations. Both mapper implementations delegate symmetrically and wrap nothing — mcp-json-jackson2/.../JacksonMcpJsonMapper.java:68-70 and mcp-json-jackson3/.../JacksonMcpJsonMapper.java:88-97:

@Override
public <T> T convertValue(Object fromValue, Class<T> type) {
    return jsonMapper.convertValue(fromValue, type);
}

but the underlying libraries disagree: Jackson 2's convertValue wraps mapping failures into IllegalArgumentException, whereas Jackson 3's throws MismatchedInputException, a JacksonException — i.e. a RuntimeException that is neither an IllegalArgumentException nor an IOException. Note that the jackson3 mapper already wraps readValue failures into IOException (mcp-json-jackson3/.../JacksonMcpJsonMapper.java:79-85), so the two methods are inconsistent with each other as well.

Scope of this report

I derived the HTTP status from the catch clauses quoted above; I did not stand up a servlet container to observe the 500 end-to-end. The reproducer exercises the exact call the transports make inside that try block, with both JSON binding modules, so the exception-type difference — and therefore which catch clause matches — is confirmed. If you'd like, I can add a servlet-level integration test as part of the fix.

Suggested fix

Option A (preferred — no transport changes, restores the documented contract). Normalize conversion failures in the Jackson 3 mapper so both modules behave the same, mirroring what that class already does for readValue:

// mcp-json-jackson3/src/main/java/io/modelcontextprotocol/json/jackson3/JacksonMcpJsonMapper.java
@Override
public <T> T convertValue(Object fromValue, Class<T> type) {
    try {
        return jsonMapper.convertValue(fromValue, type);
    }
    catch (JacksonException ex) {
        throw new IllegalArgumentException("Failed to convert value: " + ex.getMessage(), ex);
    }
}

@Override
public <T> T convertValue(Object fromValue, TypeRef<T> type) {
    try {
        JavaType javaType = jsonMapper.getTypeFactory().constructType(type.getType());
        return jsonMapper.convertValue(fromValue, javaType);
    }
    catch (JacksonException ex) {
        throw new IllegalArgumentException("Failed to convert value: " + ex.getMessage(), ex);
    }
}

Option B (defence in depth). Widen the typed catch in the servlet transports so conversion failures from any JSON binding are still reported as INVALID_REQUEST, instead of relying on every mapper implementation to normalize its exception types.

I'm happy to open a PR for Option A, with a test asserting both modules throw IllegalArgumentException for this input, if that's the direction you prefer — I wanted to confirm the approach first, since it pins mapper exception types.