Merge pull request #639 from koic/parse_request_headers_as_bytes

Parse request headers as bytes so invalid UTF-8 cannot raise

Author: koic

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

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"])