From 0814e7a916397c0bbb4cd997ce23a611a149c0c1 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Mon, 19 Oct 2026 17:48:18 +0900 Subject: [PATCH] Treat a JSON key that is not valid UTF-8 as a parse error ## Motivation and Context `JSON.parse` with `symbolize_names: true` raises `EncodingError`, not `JSON::ParserError`, for an object key that is not valid UTF-8, since such a key cannot become a Symbol; the same bytes in a value are accepted. `StreamableHTTPTransport#parse_request_body`, `StdioTransport#parse_line` and `JsonRpcHandler.handle_json` rescued only the parser's own error. A sessionless modern POST is parsed from `handle_request` to tell the eras apart, before any rescue, so such a body raised out of the transport into the web server, whose error page then answered; a legacy POST answered 500 with the error reported, `handle_json` answered `-32603`, and over stdio the error raised out of `open` and ended the read loop. JSON text must be UTF-8 (RFC 8259, section 8.1), and the Python server refuses such a body as invalid JSON. The three now answer it as the parse error it is: `-32700`, over HTTP with 400 as for any other body that is not JSON. `handle_json` rescues the parse error around the parse only, so an error raised while handling keeps its own classification and a non-string input still answers `-32603`. The loop of `#send_request` that reads the client's reply rescues it the same way, so such a reply is reported as a parse failure before it fails the request, as an unparsable reply is. Over stdio on Ruby 3.2 and later, a frame holding such a byte is stopped earlier, by `String#strip` in the read loop; stripping frames as bytes is a separate change, and the stdio tests read their frames as bytes so that they cover the parse step alone. ## How Has This Been Tested? `test/json_rpc_handler_test.rb`, `test/mcp/server/transports/streamable_http_transport_test.rb` (a sessionless modern POST and a legacy `initialize`) and `test/mcp/server/transports/stdio_transport_test.rb`. Against the previous library the handler test gets `-32603`, the modern test raises `EncodingError`, the legacy test gets a 500 and the stdio test raises out of `open`. A client reply with such a key is reported as a parse failure; against the previous library it raises without the report. ## Breaking Changes None. --- lib/json_rpc_handler.rb | 19 ++++--- lib/mcp/server/transports/stdio_transport.rb | 5 +- .../transports/streamable_http_transport.rb | 3 +- test/json_rpc_handler_test.rb | 20 +++++++ .../server/transports/stdio_transport_test.rb | 55 +++++++++++++++++++ .../streamable_http_transport_test.rb | 29 ++++++++++ 6 files changed, 121 insertions(+), 10 deletions(-) diff --git a/lib/json_rpc_handler.rb b/lib/json_rpc_handler.rb index 07ef13c9..4d59f877 100644 --- a/lib/json_rpc_handler.rb +++ b/lib/json_rpc_handler.rb @@ -55,14 +55,19 @@ def handle(request, id_validation_pattern: DEFAULT_ALLOWED_ID_CHARACTERS, &metho def handle_json(request_json, id_validation_pattern: DEFAULT_ALLOWED_ID_CHARACTERS, &method_finder) begin - request = JSON.parse(request_json, symbolize_names: true) + begin + request = JSON.parse(request_json, symbolize_names: true) + rescue JSON::ParserError, EncodingError + # `symbolize_names` raises `EncodingError` for a key that is not valid UTF-8, a parse error as well. + # Rescued around the parse only, so an error raised while handling keeps its own classification. + return error_response(id: :unknown_id, id_validation_pattern: id_validation_pattern, error: { + code: ErrorCode::PARSE_ERROR, + message: "Parse error", + data: "Invalid JSON", + }).to_json + end + response = handle(request, id_validation_pattern: id_validation_pattern, &method_finder) - rescue JSON::ParserError - response = error_response(id: :unknown_id, id_validation_pattern: id_validation_pattern, error: { - code: ErrorCode::PARSE_ERROR, - message: "Parse error", - data: "Invalid JSON", - }) rescue StandardError # Last-resort guard so an unexpected handling error returns a JSON-RPC error # instead of escaping to the transport loop. diff --git a/lib/mcp/server/transports/stdio_transport.rb b/lib/mcp/server/transports/stdio_transport.rb index 37d33fb3..f654a9ab 100644 --- a/lib/mcp/server/transports/stdio_transport.rb +++ b/lib/mcp/server/transports/stdio_transport.rb @@ -112,7 +112,7 @@ def send_request(method, params = nil) while @open && (line = read_line($stdin)) begin parsed = JSON.parse(line.strip, symbolize_names: true) - rescue JSON::ParserError => e + rescue JSON::ParserError, EncodingError => e MCP.configuration.exception_reporter.call(e, { error: "Failed to parse response" }) raise end @@ -158,7 +158,8 @@ def read_line(io) def parse_line(line) parsed = JSON.parse(line, symbolize_names: true) parsed.is_a?(Hash) ? parsed : nil - rescue JSON::ParserError + rescue JSON::ParserError, EncodingError + # `symbolize_names` raises `EncodingError` for a key that is not valid UTF-8; `handle_json` answers the parse error. nil end diff --git a/lib/mcp/server/transports/streamable_http_transport.rb b/lib/mcp/server/transports/streamable_http_transport.rb index 8318cebf..edc776ea 100644 --- a/lib/mcp/server/transports/streamable_http_transport.rb +++ b/lib/mcp/server/transports/streamable_http_transport.rb @@ -1974,7 +1974,8 @@ def parse_request_body(body_string) # `max_nesting` bounds parse depth; a too-deep body raises `JSON::NestingError`, # a subclass of `JSON::ParserError`, so it is caught below as a parse error. JSON.parse(body_string, symbolize_names: true, max_nesting: MAX_JSON_NESTING) - rescue JSON::ParserError, TypeError + # `symbolize_names` raises `EncodingError`, not `JSON::ParserError`, for a key that is not valid UTF-8. + rescue JSON::ParserError, TypeError, EncodingError raise InvalidJsonError end diff --git a/test/json_rpc_handler_test.rb b/test/json_rpc_handler_test.rb index df263415..7bbc5136 100644 --- a/test/json_rpc_handler_test.rb +++ b/test/json_rpc_handler_test.rb @@ -448,6 +448,26 @@ } end + it "returns an error with the code set to -32700 when a key is not valid UTF-8" do + # `JSON.parse` with `symbolize_names` raises `EncodingError` rather than `JSON::ParserError` for such a key. + handle_json "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"add\",\"k\xFF\":1}".b + + assert_rpc_error expected_error: { + code: -32700, + message: "Parse error", + data: "Invalid JSON", + } + end + + it "returns an error with the code set to -32603 when the input is not a string" do + handle_json nil + + assert_rpc_error expected_error: { + code: -32603, + message: "Internal error", + } + end + it "returns an error with code set to -32600 when the request is not an array or a hash" do handle true diff --git a/test/mcp/server/transports/stdio_transport_test.rb b/test/mcp/server/transports/stdio_transport_test.rb index 86fbb2c0..03a7897a 100644 --- a/test/mcp/server/transports/stdio_transport_test.rb +++ b/test/mcp/server/transports/stdio_transport_test.rb @@ -175,6 +175,30 @@ class StdioTransportTest < ActiveSupport::TestCase end end + test "open answers a frame whose JSON key is not valid UTF-8 with -32700 and keeps reading" do + # `JSON.parse` with `symbolize_names` raises `EncodingError` for such a key, which ended the loop. + # The frames are read as bytes here, so the key reaches the parser whatever `String#strip` does. + frames = "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"ping\",\"k\xFF\":1}\n" \ + "{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"ping\"}\n" + input = StringIO.new(frames.b) + output = StringIO.new + original_stdin = $stdin + original_stdout = $stdout + + begin + $stdin = input + $stdout = output + @transport.open + + responses = output.string.lines.map { |line| JSON.parse(line, symbolize_names: true) } + assert_equal(-32700, responses[0][:error][:code]) + assert_equal({ jsonrpc: "2.0", id: 2, result: {} }, responses[1]) + ensure + $stdin = original_stdin + $stdout = original_stdout + end + end + test "rejects duplicate initialize on the same stdio session with -32600" do first = { jsonrpc: "2.0", @@ -339,6 +363,37 @@ class StdioTransportTest < ActiveSupport::TestCase end end + test "send_request reports a client reply whose JSON key is not valid UTF-8 as a parse failure" do + reader, writer = IO.pipe + + # Read as bytes, so the key reaches the parser whatever `String#strip` does. + reader.binmode + output = StringIO.new + original_stdin = $stdin + original_stdout = $stdout + reported = [] + original_reporter = MCP.configuration.exception_reporter + + begin + MCP.configuration.exception_reporter = ->(e, context) { reported << [e.class, context] } + $stdin = reader + $stdout = output + @transport.instance_variable_set(:@open, true) + + writer.write("{\"jsonrpc\":\"2.0\",\"id\":\"x\",\"result\":{\"k\xFF\":1}}\n".b) + writer.flush + + assert_raises(EncodingError) { @transport.send_request("test/method") } + assert_equal([[EncodingError, { error: "Failed to parse response" }]], reported) + ensure + MCP.configuration.exception_reporter = original_reporter + $stdin = original_stdin + $stdout = original_stdout + writer.close + reader.close + end + end + test "send_request does not double-report intentional raises via exception_reporter" do reader, writer = IO.pipe output = StringIO.new diff --git a/test/mcp/server/transports/streamable_http_transport_test.rb b/test/mcp/server/transports/streamable_http_transport_test.rb index e3782553..0043751f 100644 --- a/test/mcp/server/transports/streamable_http_transport_test.rb +++ b/test/mcp/server/transports/streamable_http_transport_test.rb @@ -252,6 +252,24 @@ def string assert_equal "Parse error: Invalid JSON", body["error"]["message"] end + test "handles POST request whose JSON key is not valid UTF-8 as a parse error" do + # `JSON.parse` with `symbolize_names` raises `EncodingError` for such a key, which the rescue + # turned into a 500 with an exception report. + request = create_rack_request( + "POST", + "/", + { "CONTENT_TYPE" => "application/json" }, + "{\"jsonrpc\":\"2.0\",\"id\":\"init\",\"method\":\"initialize\",\"k\xFF\":1}".b, + ) + + response = @transport.handle_request(request) + + assert_equal 400, response[0] + body = JSON.parse(response[2][0]) + assert_equal JsonRpcHandler::ErrorCode::PARSE_ERROR, body["error"]["code"] + assert_equal "Parse error: Invalid JSON", body["error"]["message"] + end + test "POST request with JSON array body returns 400" do init_request = create_rack_request( "POST", @@ -6068,6 +6086,17 @@ def string assert_equal "ok", body.dig("result", "content", 0, "text") end + test "modern POST answers a JSON key that is not valid UTF-8 with 400 and -32700" do + # The era sniff parses a sessionless modern body before any rescue, so the `EncodingError` the parser + # raises for such a key escaped the transport. + body = "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"ping\",\"k\xFF\":1}".b + + response = @transport.handle_request(modern_rack_request(body)) + + assert_equal 400, response[0] + assert_equal JsonRpcHandler::ErrorCode::PARSE_ERROR, JSON.parse(response[2][0]).dig("error", "code") + end + test "modern POST with an unsupported header version returns 400 with -32022 and the supported list" do response = @transport.handle_request(modern_rack_request( modern_body("ping", {}, version: "2027-01-01"),