Skip to content

fix(mcp): handle null JSON-RPC request payloads safely#987

Merged
igorls merged 4 commits into
MemPalace:developfrom
alpiua:fix-mcp-null-payload
May 6, 2026
Merged

fix(mcp): handle null JSON-RPC request payloads safely#987
igorls merged 4 commits into
MemPalace:developfrom
alpiua:fix-mcp-null-payload

Conversation

@alpiua

@alpiua alpiua commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

When an MCP client sends a malformed or null top-level request, handle_request(request) crashes with an AttributeError because it attempts to call .get() on a non-dictionary object.

This PR adds explicit dictionary type validation at the top of the handler. Instead of crashing the server and disconnecting the client, it now gracefully returns a standard JSON-RPC Error -32600 (Invalid Request), adhering to robust null-handling practices for JSON-RPC parameters.

How to test

  1. Run the MemPalace MCP server locally.
  2. Send a malformed payload (e.g., null, [], or a plain string) to the standard input.
  3. Observe that instead of throwing an exception (which forces a restart), the server correctly replies with:
    {"jsonrpc": "2.0", "error": {"code": -32600, "message": "Invalid Request"}}

Checklist

  • Tests pass (python -m pytest tests/ -v)
  • No hardcoded paths
  • Linter passes (ruff check .)

@mvalentsev

Copy link
Copy Markdown
Contributor

Good catch on the AttributeError. Two small nits before this lands:

1. Missing "id": null per JSON-RPC 2.0 §5.1

Other error branches in handle_request include "id": req_id (tool errors, unknown method). Spec says: "If there was an error in detecting the id in the Request object ... it MUST be Null". Strict clients correlate request-response via id; without it they may ignore the response. Suggested:

return {"jsonrpc": "2.0", "id": None, "error": {"code": -32600, "message": "Invalid Request"}}

2. No regression test

A parametrised test covering None, [], "str", 42, True payloads would lock the fix in. Something like:

@pytest.mark.parametrize("payload", [None, [], "plain", 42, True])
def test_handle_request_invalid_payload_returns_jsonrpc_error(payload):
    resp = handle_request(payload)
    assert resp == {"jsonrpc": "2.0", "id": None, "error": {"code": -32600, "message": "Invalid Request"}}

Otherwise a narrow fix on a real bug.

@alpiua alpiua requested a review from igorls as a code owner April 18, 2026 13:54
@alpiua

alpiua commented Apr 18, 2026

Copy link
Copy Markdown
Contributor Author

@mvalentsev faced the issue when agent just refused to send a proper request.

Mitigated this by adding test_tools_call_missing_params and the corresponding fix.
So there was a gap at defense layers. Now:

  1. The previous test (-32600 Invalid Request) ensures the entire payload is a valid JSON dictionary before routing.
  2. This new test (-32602 Invalid params) ensures that once the request reaches tools/call, it explicitly contains the mandatory params object and name field, preventing an AttributeError crash on the server end.

@igorls igorls added bug Something isn't working area/mcp MCP server and tools labels Apr 24, 2026
@igorls igorls added this to the v3.3.5 milestone May 2, 2026
alpiua and others added 4 commits May 6, 2026 02:19
When the MCP client sends a malformed or null top-level request, prevent the AttributeError on request.get() by explicitly validating that the request is a dictionary. Returns standard JSON-RPC Error -32600 (Invalid Request) instead of crashing the server.
@igorls igorls force-pushed the fix-mcp-null-payload branch from fcb9709 to 869ab38 Compare May 6, 2026 05:20
@igorls igorls merged commit 9b24cfc into MemPalace:develop May 6, 2026
6 checks passed
@alpiua alpiua deleted the fix-mcp-null-payload branch May 6, 2026 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/mcp MCP server and tools bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants