Skip to content

Commit 6d7b3c4

Browse files
authored
Merge pull request #624 from koic/treat_a_json_key_that_is_not_valid_utf_8_as_a_parse_error
Treat a JSON key that is not valid UTF-8 as a parse error
2 parents c8e8b62 + 0814e7a commit 6d7b3c4

6 files changed

Lines changed: 121 additions & 10 deletions

File tree

‎lib/json_rpc_handler.rb‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,19 @@ def handle(request, id_validation_pattern: DEFAULT_ALLOWED_ID_CHARACTERS, &metho
5555

5656
def handle_json(request_json, id_validation_pattern: DEFAULT_ALLOWED_ID_CHARACTERS, &method_finder)
5757
begin
58-
request = JSON.parse(request_json, symbolize_names: true)
58+
begin
59+
request = JSON.parse(request_json, symbolize_names: true)
60+
rescue JSON::ParserError, EncodingError
61+
# `symbolize_names` raises `EncodingError` for a key that is not valid UTF-8, a parse error as well.
62+
# Rescued around the parse only, so an error raised while handling keeps its own classification.
63+
return error_response(id: :unknown_id, id_validation_pattern: id_validation_pattern, error: {
64+
code: ErrorCode::PARSE_ERROR,
65+
message: "Parse error",
66+
data: "Invalid JSON",
67+
}).to_json
68+
end
69+
5970
response = handle(request, id_validation_pattern: id_validation_pattern, &method_finder)
60-
rescue JSON::ParserError
61-
response = error_response(id: :unknown_id, id_validation_pattern: id_validation_pattern, error: {
62-
code: ErrorCode::PARSE_ERROR,
63-
message: "Parse error",
64-
data: "Invalid JSON",
65-
})
6671
rescue StandardError
6772
# Last-resort guard so an unexpected handling error returns a JSON-RPC error
6873
# instead of escaping to the transport loop.

‎lib/mcp/server/transports/stdio_transport.rb‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ def send_request(method, params = nil)
138138
while @open && (line = read_line($stdin))
139139
begin
140140
parsed = JSON.parse(line.strip, symbolize_names: true)
141-
rescue JSON::ParserError => e
141+
rescue JSON::ParserError, EncodingError => e
142142
MCP.configuration.exception_reporter.call(e, { error: "Failed to parse response" })
143143
raise
144144
end
@@ -184,7 +184,8 @@ def read_line(io)
184184
def parse_line(line)
185185
parsed = JSON.parse(line, symbolize_names: true)
186186
parsed.is_a?(Hash) ? parsed : nil
187-
rescue JSON::ParserError
187+
rescue JSON::ParserError, EncodingError
188+
# `symbolize_names` raises `EncodingError` for a key that is not valid UTF-8; `handle_json` answers the parse error.
188189
nil
189190
end
190191

‎lib/mcp/server/transports/streamable_http_transport.rb‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1980,7 +1980,8 @@ def parse_request_body(body_string)
19801980
# `max_nesting` bounds parse depth; a too-deep body raises `JSON::NestingError`,
19811981
# a subclass of `JSON::ParserError`, so it is caught below as a parse error.
19821982
JSON.parse(body_string, symbolize_names: true, max_nesting: MAX_JSON_NESTING)
1983-
rescue JSON::ParserError, TypeError
1983+
# `symbolize_names` raises `EncodingError`, not `JSON::ParserError`, for a key that is not valid UTF-8.
1984+
rescue JSON::ParserError, TypeError, EncodingError
19841985
raise InvalidJsonError
19851986
end
19861987

‎test/json_rpc_handler_test.rb‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,26 @@
448448
}
449449
end
450450

451+
it "returns an error with the code set to -32700 when a key is not valid UTF-8" do
452+
# `JSON.parse` with `symbolize_names` raises `EncodingError` rather than `JSON::ParserError` for such a key.
453+
handle_json "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"add\",\"k\xFF\":1}".b
454+
455+
assert_rpc_error expected_error: {
456+
code: -32700,
457+
message: "Parse error",
458+
data: "Invalid JSON",
459+
}
460+
end
461+
462+
it "returns an error with the code set to -32603 when the input is not a string" do
463+
handle_json nil
464+
465+
assert_rpc_error expected_error: {
466+
code: -32603,
467+
message: "Internal error",
468+
}
469+
end
470+
451471
it "returns an error with code set to -32600 when the request is not an array or a hash" do
452472
handle true
453473

‎test/mcp/server/transports/stdio_transport_test.rb‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,30 @@ class StdioTransportTest < ActiveSupport::TestCase
175175
end
176176
end
177177

178+
test "open answers a frame whose JSON key is not valid UTF-8 with -32700 and keeps reading" do
179+
# `JSON.parse` with `symbolize_names` raises `EncodingError` for such a key, which ended the loop.
180+
# The frames are read as bytes here, so the key reaches the parser whatever `String#strip` does.
181+
frames = "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"ping\",\"k\xFF\":1}\n" \
182+
"{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"ping\"}\n"
183+
input = StringIO.new(frames.b)
184+
output = StringIO.new
185+
original_stdin = $stdin
186+
original_stdout = $stdout
187+
188+
begin
189+
$stdin = input
190+
$stdout = output
191+
@transport.open
192+
193+
responses = output.string.lines.map { |line| JSON.parse(line, symbolize_names: true) }
194+
assert_equal(-32700, responses[0][:error][:code])
195+
assert_equal({ jsonrpc: "2.0", id: 2, result: {} }, responses[1])
196+
ensure
197+
$stdin = original_stdin
198+
$stdout = original_stdout
199+
end
200+
end
201+
178202
test "rejects duplicate initialize on the same stdio session with -32600" do
179203
first = {
180204
jsonrpc: "2.0",
@@ -339,6 +363,37 @@ class StdioTransportTest < ActiveSupport::TestCase
339363
end
340364
end
341365

366+
test "send_request reports a client reply whose JSON key is not valid UTF-8 as a parse failure" do
367+
reader, writer = IO.pipe
368+
369+
# Read as bytes, so the key reaches the parser whatever `String#strip` does.
370+
reader.binmode
371+
output = StringIO.new
372+
original_stdin = $stdin
373+
original_stdout = $stdout
374+
reported = []
375+
original_reporter = MCP.configuration.exception_reporter
376+
377+
begin
378+
MCP.configuration.exception_reporter = ->(e, context) { reported << [e.class, context] }
379+
$stdin = reader
380+
$stdout = output
381+
@transport.instance_variable_set(:@open, true)
382+
383+
writer.write("{\"jsonrpc\":\"2.0\",\"id\":\"x\",\"result\":{\"k\xFF\":1}}\n".b)
384+
writer.flush
385+
386+
assert_raises(EncodingError) { @transport.send_request("test/method") }
387+
assert_equal([[EncodingError, { error: "Failed to parse response" }]], reported)
388+
ensure
389+
MCP.configuration.exception_reporter = original_reporter
390+
$stdin = original_stdin
391+
$stdout = original_stdout
392+
writer.close
393+
reader.close
394+
end
395+
end
396+
342397
test "send_request does not double-report intentional raises via exception_reporter" do
343398
reader, writer = IO.pipe
344399
output = StringIO.new

‎test/mcp/server/transports/streamable_http_transport_test.rb‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,24 @@ def string
252252
assert_equal "Parse error: Invalid JSON", body["error"]["message"]
253253
end
254254

255+
test "handles POST request whose JSON key is not valid UTF-8 as a parse error" do
256+
# `JSON.parse` with `symbolize_names` raises `EncodingError` for such a key, which the rescue
257+
# turned into a 500 with an exception report.
258+
request = create_rack_request(
259+
"POST",
260+
"/",
261+
{ "CONTENT_TYPE" => "application/json" },
262+
"{\"jsonrpc\":\"2.0\",\"id\":\"init\",\"method\":\"initialize\",\"k\xFF\":1}".b,
263+
)
264+
265+
response = @transport.handle_request(request)
266+
267+
assert_equal 400, response[0]
268+
body = JSON.parse(response[2][0])
269+
assert_equal JsonRpcHandler::ErrorCode::PARSE_ERROR, body["error"]["code"]
270+
assert_equal "Parse error: Invalid JSON", body["error"]["message"]
271+
end
272+
255273
test "POST request with JSON array body returns 400" do
256274
init_request = create_rack_request(
257275
"POST",
@@ -6146,6 +6164,17 @@ def string
61466164
assert_equal "ok", body.dig("result", "content", 0, "text")
61476165
end
61486166

6167+
test "modern POST answers a JSON key that is not valid UTF-8 with 400 and -32700" do
6168+
# The era sniff parses a sessionless modern body before any rescue, so the `EncodingError` the parser
6169+
# raises for such a key escaped the transport.
6170+
body = "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"ping\",\"k\xFF\":1}".b
6171+
6172+
response = @transport.handle_request(modern_rack_request(body))
6173+
6174+
assert_equal 400, response[0]
6175+
assert_equal JsonRpcHandler::ErrorCode::PARSE_ERROR, JSON.parse(response[2][0]).dig("error", "code")
6176+
end
6177+
61496178
test "modern POST with an unsupported header version returns 400 with -32022 and the supported list" do
61506179
response = @transport.handle_request(modern_rack_request(
61516180
modern_body("ping", {}, version: "2027-01-01"),

0 commit comments

Comments
 (0)