From 61edd2c0692570063900b25a558ec7a35914db0c Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 31 Oct 2026 00:33:26 +0900 Subject: [PATCH] Parse request headers as bytes so invalid UTF-8 cannot raise ## Motivation and Context The streamable HTTP transport validates the `Host`, `Origin`, `Accept`, and `Content-Type` headers with `downcase`, `split`, and regexps. A Rack server hands header values over as ASCII-8BIT, where every byte is valid, but Rack::Test, `Rack::MockRequest`, and a framework that re-tags the env can pass them as UTF-8, and then a header carrying an invalid byte sequence makes those calls raise `ArgumentError`. For `Host` and `Origin` on any method and for `Accept` on a GET the exception escapes the Rack call, and on a POST the `Accept` and `Content-Type` checks turn it into a 500 with an exception report, instead of the rejection the check exists to produce. The transport now reads those headers as bytes, so both environments behave like a Rack server does: a `Host` or `Origin` carrying an invalid byte in the compared part answers 403, an `Accept` whose required media type carries one answers 406, and a `Content-Type` whose media type carries one answers 415. Nothing changes for ASCII values, which host names and media types are on the wire (an internationalized host travels as punycode). ## How Has This Been Tested? A new transport test sends each of the four headers with a trailing invalid byte in a UTF-8-tagged string and asserts the status and message the check produces. It errors with `ArgumentError: input string invalid` without the change. A second test sends such an `Accept` on a GET, which answers 406 now and raised before, since nothing rescues in `handle_get`. ## Breaking Changes An `allowed_hosts:` or `allowed_origins:` entry with a non-ASCII character no longer matches a header tagged UTF-8 in a test or framework environment; it never matched the bytes a Rack server hands over, so a deployment is unchanged. --- .../transports/streamable_http_transport.rb | 18 +++++-- .../streamable_http_transport_test.rb | 47 +++++++++++++++++++ 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/lib/mcp/server/transports/streamable_http_transport.rb b/lib/mcp/server/transports/streamable_http_transport.rb index 29b3518e..ce20a012 100644 --- a/lib/mcp/server/transports/streamable_http_transport.rb +++ b/lib/mcp/server/transports/streamable_http_transport.rb @@ -1899,7 +1899,7 @@ def session_principal_matches?(session, request) end def validate_accept_header(request, required_types) - accept_header = request.env["HTTP_ACCEPT"] + accept_header = header_bytes(request, "HTTP_ACCEPT") return not_acceptable_response(required_types) unless accept_header accepted_types = parse_accept_header(accept_header) @@ -1918,7 +1918,7 @@ def parse_accept_header(header) end def validate_content_type(request) - content_type = request.env["CONTENT_TYPE"] + content_type = header_bytes(request, "CONTENT_TYPE") media_type = content_type&.split(";")&.first&.strip&.downcase return if media_type == "application/json" @@ -2428,10 +2428,18 @@ def validate_dns_rebinding(request) validate_host(request) || validate_origin(request) end + # A header value as bytes. Rack servers hand header values over as ASCII-8BIT, where every byte is valid, but a framework + # or a test request can tag them UTF-8, and then `downcase`, `split`, and regexps raise on an invalid byte sequence + # before any rescue applies. Reading the value as bytes puts both on the same footing: a malformed value fails its comparison + # and gets the rejection that comparison produces, instead of failing the request. + def header_bytes(request, name) + request.env[name]&.b + end + # Rejects a rebound `Host` (e.g. `evil.example.com` re-pointed at 127.0.0.1). # A request without a `Host` header (e.g. HTTP/1.0) is allowed; the rebinding vector this guards against always carries one. def validate_host(request) - host = request.env["HTTP_HOST"] + host = header_bytes(request, "HTTP_HOST") return if host.nil? # An `allowed_hosts:` entry matches either the bare host name (any port) @@ -2446,7 +2454,7 @@ def validate_host(request) # A request without an `Origin` header (typical for non-browser MCP clients) is allowed. A browser cross-origin request is # rejected unless the origin is same-origin or explicitly allow-listed via `allowed_origins:`. def validate_origin(request) - origin = request.env["HTTP_ORIGIN"] + origin = header_bytes(request, "HTTP_ORIGIN") return if origin.nil? return if same_origin?(origin, request) return if @allowed_origins.include?(origin.downcase) @@ -2467,7 +2475,7 @@ def request_hostname(host) # but the `Origin`'s scheme is used to drop a redundant default port (`:80` for http, `:443` for https) from # both sides so `http://example.com` matches `Host: example.com:80`. Comparison is case-insensitive. def same_origin?(origin, request) - host = request.env["HTTP_HOST"] + host = header_bytes(request, "HTTP_HOST") return false if host.nil? normalized = origin.downcase diff --git a/test/mcp/server/transports/streamable_http_transport_test.rb b/test/mcp/server/transports/streamable_http_transport_test.rb index df1d173c..8a6eca2c 100644 --- a/test/mcp/server/transports/streamable_http_transport_test.rb +++ b/test/mcp/server/transports/streamable_http_transport_test.rb @@ -5750,6 +5750,53 @@ def string end end + test "handle_request rejects headers carrying invalid UTF-8 bytes instead of raising" do + # A Rack server hands headers over as ASCII-8BIT, but a test request or a framework can tag them UTF-8, + # and String#downcase, #split, and regexps raise on invalid bytes in a UTF-8 string. + invalid = ->(value) { "#{value}\xFF".dup.force_encoding(Encoding::UTF_8) } + body = { jsonrpc: "2.0", method: "ping", id: "1" }.to_json + expectations = [ + [{ "HTTP_HOST" => invalid.call("localhost") }, 403, "Forbidden: Invalid Host header"], + [{ "HTTP_ORIGIN" => invalid.call("https://app.example.com") }, 403, "Forbidden: Invalid Origin header"], + [{ "HTTP_ACCEPT" => invalid.call("application/json") }, 406, nil], + [{ "CONTENT_TYPE" => invalid.call("application/json") }, 415, nil], + ] + + expectations.each do |headers, expected_status, expected_message| + request = create_rack_request("POST", "/", { "CONTENT_TYPE" => "application/json" }.merge(headers), body) + + status, _headers, response_body = @transport.handle_request(request) + + assert_equal expected_status, status, headers.keys.first + assert_equal expected_message, JSON.parse(response_body.first)["error"]["message"] if expected_message + end + end + + test "GET request with an Accept header carrying invalid UTF-8 bytes returns 406 instead of raising" do + # The GET path validates `Accept` outside any rescue, so the raise reached the web server before. + init_request = create_rack_request( + "POST", + "/", + { "CONTENT_TYPE" => "application/json" }, + { jsonrpc: "2.0", method: "initialize", id: "123", params: initialize_params }.to_json, + ) + session_id = @transport.handle_request(init_request)[1]["mcp-session-id"] + + request = create_rack_request( + "GET", + "/", + { + "HTTP_MCP_SESSION_ID" => session_id, + "HTTP_ACCEPT" => "text/event-stream\xFF".dup.force_encoding(Encoding::UTF_8), + }, + ) + + status, _headers, body = @transport.handle_request(request) + + assert_equal 406, status + assert_equal JsonRpcHandler::ErrorCode::INVALID_REQUEST, JSON.parse(body.first)["error"]["code"] + end + test "handle_request returns 403 for POST when Origin is not in allowed_origins" do transport = StreamableHTTPTransport.new(@server, allowed_origins: ["https://app.example.com"])