From a01b12c8ae72698978d5a7de04197496f2278847 Mon Sep 17 00:00:00 2001 From: RahulHere Date: Fri, 6 Mar 2026 00:37:02 +0800 Subject: [PATCH 1/5] Add json::JsonValue to ResponseResult variant (#200) Add JsonValue type to ResponseResult variant to support arbitrary nested JSON responses. This enables proper MCP protocol compliance for responses like initialize that require nested object structures. Changes: - Add json::JsonValue to ResponseResult variant in types.h - Add JsonValue serialization handler in json_serialization.cc --- include/mcp/types.h | 4 ++- src/json/json_serialization.cc | 4 +++ tests/json/test_mcp_serialization.cc | 53 ++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/include/mcp/types.h b/include/mcp/types.h index b760aa44..dd83f586 100644 --- a/include/mcp/types.h +++ b/include/mcp/types.h @@ -499,6 +499,7 @@ struct Request { // Generic result type for responses // Note: ListResourcesResult and ListToolsResult are defined outside jsonrpc // namespace but used here +// json::JsonValue added to support arbitrary nested JSON responses (e.g., initialize) using ResponseResult = variant, std::vector, ListResourcesResult, - ListToolsResult>; + ListToolsResult, + json::JsonValue>; struct Response { std::string jsonrpc = "2.0"; diff --git a/src/json/json_serialization.cc b/src/json/json_serialization.cc index 7bf81d22..c1cf0b4d 100644 --- a/src/json/json_serialization.cc +++ b/src/json/json_serialization.cc @@ -130,6 +130,10 @@ JsonValue serialize_ResponseResult(const jsonrpc::ResponseResult& result) { [&json_result](const ListToolsResult& list_result) { // ListToolsResult is a full result object with tools array json_result = to_json(list_result); + }, + [&json_result](const JsonValue& json_val) { + // Direct JsonValue passthrough for arbitrary nested JSON responses + json_result = json_val; }); return json_result; diff --git a/tests/json/test_mcp_serialization.cc b/tests/json/test_mcp_serialization.cc index c85d1c6e..d5679f4c 100644 --- a/tests/json/test_mcp_serialization.cc +++ b/tests/json/test_mcp_serialization.cc @@ -303,6 +303,59 @@ TEST_F(MCPSerializationTest, JsonRpcNotification) { testRoundTrip(param_notif); } +TEST_F(MCPSerializationTest, JsonRpcResponseWithJsonValue) { + // Test ResponseResult with JsonValue for nested JSON structures + // This is needed for MCP protocol compliance (e.g., initialize response) + + // Simple nested object + JsonValue nested_result = JsonValue::object(); + nested_result["protocolVersion"] = "2024-11-05"; + + JsonValue server_info = JsonValue::object(); + server_info["name"] = "test-server"; + server_info["version"] = "1.0.0"; + nested_result["serverInfo"] = std::move(server_info); + + JsonValue capabilities = JsonValue::object(); + capabilities["tools"] = JsonValue::object(); + capabilities["prompts"] = JsonValue::object(); + nested_result["capabilities"] = std::move(capabilities); + + auto json_resp = jsonrpc::Response::success( + make_request_id(1), jsonrpc::ResponseResult(nested_result)); + + // Serialize and verify structure + JsonValue serialized = to_json(json_resp); + std::string json_str = serialized.toString(); + + // Verify nested structure is preserved + EXPECT_TRUE(json_str.find("\"serverInfo\":{") != std::string::npos); + EXPECT_TRUE(json_str.find("\"capabilities\":{") != std::string::npos); + EXPECT_TRUE(json_str.find("\"protocolVersion\":\"2024-11-05\"") != + std::string::npos); + + // Test with array in JsonValue + JsonValue array_result = JsonValue::object(); + JsonValue tools_array = JsonValue::array(); + + JsonValue tool1 = JsonValue::object(); + tool1["name"] = "get-weather"; + tool1["description"] = "Get weather info"; + tools_array.push_back(std::move(tool1)); + + array_result["tools"] = std::move(tools_array); + + auto array_resp = jsonrpc::Response::success( + make_request_id(2), jsonrpc::ResponseResult(array_result)); + + JsonValue array_serialized = to_json(array_resp); + std::string array_json = array_serialized.toString(); + + // Verify array structure + EXPECT_TRUE(array_json.find("\"tools\":[") != std::string::npos); + EXPECT_TRUE(array_json.find("\"name\":\"get-weather\"") != std::string::npos); +} + // ============================================================================= // Protocol Request Types // ============================================================================= From f7ad4c86ef68ee9cf8343036ba0b61931f800b4e Mon Sep 17 00:00:00 2001 From: RahulHere Date: Fri, 6 Mar 2026 00:37:41 +0800 Subject: [PATCH 2/5] Fix MCP server response formats for protocol compliance (#199) Fix response formats for initialize, tools/list, and prompts/list to comply with MCP protocol specification. Changes: - handleInitialize: Use proper nested JSON structure instead of flattened dot notation (e.g., {"serverInfo": {"name": ...}} not {"serverInfo.name": ...}) - handleListTools: Wrap tools array in {"tools": [...]} object - handleListPrompts: Wrap prompts array in {"prompts": [...]} object --- src/server/mcp_server.cc | 112 ++++++++------- tests/CMakeLists.txt | 12 +- tests/server/test_mcp_server_responses.cc | 165 ++++++++++++++++++++++ 3 files changed, 236 insertions(+), 53 deletions(-) create mode 100644 tests/server/test_mcp_server_responses.cc diff --git a/src/server/mcp_server.cc b/src/server/mcp_server.cc index bb5b2511..0d084efc 100644 --- a/src/server/mcp_server.cc +++ b/src/server/mcp_server.cc @@ -812,63 +812,53 @@ jsonrpc::Response McpServer::handleInitialize(const jsonrpc::Request& request, // session.setClientInfo(...); } - // Build initialize result - InitializeResult result; - result.protocolVersion = config_.protocol_version; - result.capabilities = config_.capabilities; - result.serverInfo = mcp::make_optional( - Implementation(config_.server_name, config_.server_version)); - - if (!config_.instructions.empty()) { - result.instructions = mcp::make_optional(config_.instructions); - } - - // Convert InitializeResult to Metadata for ResponseResult - // Since ResponseResult doesn't directly support InitializeResult, - // we need to convert it to a simplified Metadata object - // TODO: This is a temporary workaround until proper serialization is - // implemented - auto builder = - make().add("protocolVersion", result.protocolVersion); - - // Add server info if present (flattened) - if (result.serverInfo.has_value()) { - builder.add("serverInfo.name", result.serverInfo->name) - .add("serverInfo.version", result.serverInfo->version); - } - - // Add capabilities (flattened) - if (result.capabilities.resources.has_value()) { - if (holds_alternative(result.capabilities.resources.value())) { - builder.add("capabilities.resources", - get(result.capabilities.resources.value())); + // Build initialize result as proper nested JSON structure + // MCP protocol requires nested objects, not flattened dot notation + json::JsonValue result_json; + result_json["protocolVersion"] = config_.protocol_version; + + // Add serverInfo as nested object + json::JsonValue server_info; + server_info["name"] = config_.server_name; + server_info["version"] = config_.server_version; + result_json["serverInfo"] = std::move(server_info); + + // Add capabilities as nested object with empty objects for enabled caps + json::JsonValue capabilities = json::JsonValue::object(); + if (config_.capabilities.resources.has_value()) { + if (holds_alternative(config_.capabilities.resources.value())) { + if (get(config_.capabilities.resources.value())) { + capabilities["resources"] = json::JsonValue::object(); + } } else { - // Handle ResourcesCapability struct - builder.add("capabilities.resources", true) - .add("capabilities.resources.subscribe", true) - .add("capabilities.resources.listChanged", true); + json::JsonValue resources_cap = json::JsonValue::object(); + resources_cap["subscribe"] = true; + resources_cap["listChanged"] = true; + capabilities["resources"] = std::move(resources_cap); } } - - if (result.capabilities.tools.has_value()) { - builder.add("capabilities.tools", result.capabilities.tools.value()); + if (config_.capabilities.tools.has_value() && + config_.capabilities.tools.value()) { + capabilities["tools"] = json::JsonValue::object(); } - if (result.capabilities.prompts.has_value()) { - builder.add("capabilities.prompts", result.capabilities.prompts.value()); + if (config_.capabilities.prompts.has_value() && + config_.capabilities.prompts.value()) { + capabilities["prompts"] = json::JsonValue::object(); } - if (result.capabilities.logging.has_value()) { - builder.add("capabilities.logging", result.capabilities.logging.value()); + if (config_.capabilities.logging.has_value() && + config_.capabilities.logging.value()) { + capabilities["logging"] = json::JsonValue::object(); } + result_json["capabilities"] = std::move(capabilities); // Add instructions if present - if (result.instructions.has_value()) { - builder.add("instructions", result.instructions.value()); + if (!config_.instructions.empty()) { + result_json["instructions"] = config_.instructions; } - auto response_metadata = builder.build(); - + // Return response with JSON result directly return jsonrpc::Response::success(request.id, - jsonrpc::ResponseResult(response_metadata)); + jsonrpc::ResponseResult(result_json)); } jsonrpc::Response McpServer::handlePing(const jsonrpc::Request& request, @@ -986,11 +976,20 @@ jsonrpc::Response McpServer::handleUnsubscribe(const jsonrpc::Request& request, jsonrpc::Response McpServer::handleListTools(const jsonrpc::Request& request, SessionContext& session) { - // Get tools from tool registry and return tools vector directly - // ResponseResult variant supports std::vector + // Get tools from tool registry auto result = tool_registry_->listTools(); + + // Build response as JsonValue with "tools" key per MCP spec + json::JsonValue tools_array = json::JsonValue::array(); + for (const auto& tool : result.tools) { + tools_array.push_back(json::to_json(tool)); + } + + json::JsonValue response_obj = json::JsonValue::object(); + response_obj["tools"] = std::move(tools_array); + return jsonrpc::Response::success(request.id, - jsonrpc::ResponseResult(result.tools)); + jsonrpc::ResponseResult(response_obj)); } jsonrpc::Response McpServer::handleCallTool(const jsonrpc::Request& request, @@ -1081,11 +1080,20 @@ jsonrpc::Response McpServer::handleListPrompts(const jsonrpc::Request& request, } } - // Get prompts from prompt registry and return prompts vector directly - // ResponseResult variant supports std::vector + // Get prompts from prompt registry auto result = prompt_registry_->listPrompts(cursor); + + // Build response as JsonValue with "prompts" key per MCP spec + json::JsonValue prompts_array = json::JsonValue::array(); + for (const auto& prompt : result.prompts) { + prompts_array.push_back(json::to_json(prompt)); + } + + json::JsonValue response_obj = json::JsonValue::object(); + response_obj["prompts"] = std::move(prompts_array); + return jsonrpc::Response::success(request.id, - jsonrpc::ResponseResult(result.prompts)); + jsonrpc::ResponseResult(response_obj)); } jsonrpc::Response McpServer::handleGetPrompt(const jsonrpc::Request& request, diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a2492d94..3f134e56 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -18,6 +18,8 @@ add_executable(test_mcp_serialization json/test_mcp_serialization.cc) add_executable(test_mcp_serialization_extensive json/test_mcp_serialization_extensive.cc) add_executable(test_template_serialization json/test_template_serialization.cc) add_executable(test_short_json_api json/test_short_json_api.cc) +# Server tests +add_executable(test_mcp_server_responses server/test_mcp_server_responses.cc) # Event tests add_executable(test_event_loop event/test_event_loop.cc) add_executable(test_timer_lifetime event/test_timer_lifetime.cc) @@ -223,7 +225,14 @@ target_link_libraries(test_buffer target_link_libraries(test_mcp_serialization gopher-mcp - gtest + gtest + gtest_main + Threads::Threads +) + +target_link_libraries(test_mcp_server_responses + gopher-mcp + gtest gtest_main Threads::Threads ) @@ -1204,6 +1213,7 @@ add_test(NAME BufferTest COMMAND test_buffer) add_test(NAME McpSerializationTest COMMAND test_mcp_serialization) add_test(NAME McpSerializationExtensiveTest COMMAND test_mcp_serialization_extensive) add_test(NAME TemplateSerializationTest COMMAND test_template_serialization) +add_test(NAME McpServerResponsesTest COMMAND test_mcp_server_responses) add_test(NAME EventLoopTest COMMAND test_event_loop) add_test(NAME TimerLifetimeTest COMMAND test_timer_lifetime) diff --git a/tests/server/test_mcp_server_responses.cc b/tests/server/test_mcp_server_responses.cc new file mode 100644 index 00000000..f34ea84d --- /dev/null +++ b/tests/server/test_mcp_server_responses.cc @@ -0,0 +1,165 @@ +/** + * Unit tests for MCP server response formats + * + * Tests that server responses conform to MCP protocol specification: + * - initialize: nested JSON with serverInfo, capabilities objects + * - tools/list: {"tools": [...]} wrapper + * - prompts/list: {"prompts": [...]} wrapper + */ + +#include + +#include "mcp/json/json_bridge.h" +#include "mcp/json/json_serialization.h" +#include "mcp/types.h" + +using namespace mcp; +using namespace mcp::json; + +class McpServerResponseTest : public ::testing::Test { + protected: + void SetUp() override {} + void TearDown() override {} +}; + +// Test that initialize response has proper nested structure +TEST_F(McpServerResponseTest, InitializeResponseFormat) { + // Build initialize response the same way mcp_server.cc does + JsonValue result_json; + result_json["protocolVersion"] = "2024-11-05"; + + JsonValue server_info; + server_info["name"] = "test-server"; + server_info["version"] = "1.0.0"; + result_json["serverInfo"] = std::move(server_info); + + JsonValue capabilities = JsonValue::object(); + capabilities["tools"] = JsonValue::object(); + capabilities["prompts"] = JsonValue::object(); + result_json["capabilities"] = std::move(capabilities); + + result_json["instructions"] = "Test instructions"; + + // Create response + auto response = jsonrpc::Response::success( + make_request_id(1), jsonrpc::ResponseResult(result_json)); + + // Serialize and verify + JsonValue serialized = to_json(response); + std::string json_str = serialized.toString(); + + // Verify nested structure (not flattened dot notation) + EXPECT_TRUE(json_str.find("\"serverInfo\":{\"name\"") != std::string::npos) + << "serverInfo should be nested object, got: " << json_str; + EXPECT_TRUE(json_str.find("\"capabilities\":{") != std::string::npos) + << "capabilities should be nested object"; + EXPECT_TRUE(json_str.find("\"tools\":{}") != std::string::npos) + << "tools should be empty object"; + EXPECT_TRUE(json_str.find("\"prompts\":{}") != std::string::npos) + << "prompts should be empty object"; + + // Verify NOT flattened + EXPECT_TRUE(json_str.find("serverInfo.name") == std::string::npos) + << "Should not use dot notation"; + EXPECT_TRUE(json_str.find("capabilities.tools") == std::string::npos) + << "Should not use dot notation"; +} + +// Test that tools/list response wraps array in object +TEST_F(McpServerResponseTest, ListToolsResponseFormat) { + // Build tools/list response the same way mcp_server.cc does + JsonValue tools_array = JsonValue::array(); + + JsonValue tool1 = JsonValue::object(); + tool1["name"] = "get-weather"; + tool1["description"] = "Get weather for a city"; + + JsonValue input_schema = JsonValue::object(); + input_schema["type"] = "object"; + JsonValue properties = JsonValue::object(); + JsonValue city_prop = JsonValue::object(); + city_prop["type"] = "string"; + city_prop["description"] = "City name"; + properties["city"] = std::move(city_prop); + input_schema["properties"] = std::move(properties); + + JsonValue required = JsonValue::array(); + required.push_back("city"); + input_schema["required"] = std::move(required); + + tool1["inputSchema"] = std::move(input_schema); + tools_array.push_back(std::move(tool1)); + + JsonValue response_obj = JsonValue::object(); + response_obj["tools"] = std::move(tools_array); + + auto response = jsonrpc::Response::success( + make_request_id(1), jsonrpc::ResponseResult(response_obj)); + + // Serialize and verify + JsonValue serialized = to_json(response); + std::string json_str = serialized.toString(); + + // Verify wrapped in {"tools": [...]} + EXPECT_TRUE(json_str.find("\"result\":{\"tools\":[") != std::string::npos) + << "tools should be wrapped in object, got: " << json_str; + + // Verify NOT bare array + EXPECT_TRUE(json_str.find("\"result\":[{\"name\"") == std::string::npos) + << "result should not be bare array"; +} + +// Test that prompts/list response wraps array in object +TEST_F(McpServerResponseTest, ListPromptsResponseFormat) { + // Build prompts/list response the same way mcp_server.cc does + JsonValue prompts_array = JsonValue::array(); + + JsonValue prompt1 = JsonValue::object(); + prompt1["name"] = "greeting"; + prompt1["description"] = "A greeting prompt"; + prompts_array.push_back(std::move(prompt1)); + + JsonValue response_obj = JsonValue::object(); + response_obj["prompts"] = std::move(prompts_array); + + auto response = jsonrpc::Response::success( + make_request_id(1), jsonrpc::ResponseResult(response_obj)); + + // Serialize and verify + JsonValue serialized = to_json(response); + std::string json_str = serialized.toString(); + + // Verify wrapped in {"prompts": [...]} + EXPECT_TRUE(json_str.find("\"result\":{\"prompts\":[") != std::string::npos) + << "prompts should be wrapped in object, got: " << json_str; +} + +// Test empty lists are properly formatted +TEST_F(McpServerResponseTest, EmptyListsFormat) { + // Empty tools list + JsonValue tools_obj = JsonValue::object(); + tools_obj["tools"] = JsonValue::array(); + + auto tools_response = jsonrpc::Response::success( + make_request_id(1), jsonrpc::ResponseResult(tools_obj)); + + JsonValue tools_serialized = to_json(tools_response); + std::string tools_json = tools_serialized.toString(); + + EXPECT_TRUE(tools_json.find("\"result\":{\"tools\":[]}") != std::string::npos) + << "Empty tools should be {\"tools\":[]}, got: " << tools_json; + + // Empty prompts list + JsonValue prompts_obj = JsonValue::object(); + prompts_obj["prompts"] = JsonValue::array(); + + auto prompts_response = jsonrpc::Response::success( + make_request_id(2), jsonrpc::ResponseResult(prompts_obj)); + + JsonValue prompts_serialized = to_json(prompts_response); + std::string prompts_json = prompts_serialized.toString(); + + EXPECT_TRUE(prompts_json.find("\"result\":{\"prompts\":[]}") != + std::string::npos) + << "Empty prompts should be {\"prompts\":[]}, got: " << prompts_json; +} From d730a4ba55ec0326711ef3156c9ce985950d4300 Mon Sep 17 00:00:00 2001 From: RahulHere Date: Fri, 6 Mar 2026 00:37:51 +0800 Subject: [PATCH 3/5] Add CORS headers to HTTP responses (#198) Add CORS headers to all HTTP responses for browser-based MCP clients (e.g., MCP Inspector). Also add check to pass through already HTTP-formatted responses from routing filter. Changes: - Add Access-Control-Allow-Origin, Methods, and Headers to JSON responses - Add same CORS headers to SSE responses - Skip HTTP framing for responses that already have HTTP headers --- src/filter/http_codec_filter.cc | 18 ++++++ tests/CMakeLists.txt | 9 +++ tests/filter/test_cors_headers.cc | 91 +++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+) create mode 100644 tests/filter/test_cors_headers.cc diff --git a/src/filter/http_codec_filter.cc b/src/filter/http_codec_filter.cc index a3bb46b5..3d8879ce 100644 --- a/src/filter/http_codec_filter.cc +++ b/src/filter/http_codec_filter.cc @@ -245,6 +245,16 @@ network::FilterStatus HttpCodecFilter::onWrite(Buffer& data, bool end_stream) { std::string body_data( static_cast(data.linearize(body_length)), body_length); + // Check if data is already HTTP-formatted (from routing filter) + // If so, pass through without adding more HTTP framing + if (body_data.length() >= 5 && + body_data.compare(0, 5, "HTTP/") == 0) { + GOPHER_LOG_DEBUG( + "HttpCodecFilter::onWrite - data already HTTP formatted, " + "passing through"); + return network::FilterStatus::Continue; + } + // Clear the buffer to build formatted HTTP response data.drain(body_length); @@ -280,6 +290,10 @@ network::FilterStatus HttpCodecFilter::onWrite(Buffer& data, bool end_stream) { response << "Cache-Control: no-cache\r\n"; response << "Connection: keep-alive\r\n"; response << "X-Accel-Buffering: no\r\n"; // Disable proxy buffering + // CORS headers for browser-based clients (e.g., MCP Inspector) + response << "Access-Control-Allow-Origin: *\r\n"; + response << "Access-Control-Allow-Methods: GET, POST, OPTIONS\r\n"; + response << "Access-Control-Allow-Headers: Content-Type, Authorization, Accept, Mcp-Session-Id, Mcp-Protocol-Version\r\n"; response << "\r\n"; // SSE data is already formatted by SSE filter response << body_data; @@ -290,6 +304,10 @@ network::FilterStatus HttpCodecFilter::onWrite(Buffer& data, bool end_stream) { GOPHER_LOG_TRACE("onWrite: Content-Length={} body_preview={}...", body_length, body_data.substr(0, 50)); response << "Cache-Control: no-cache\r\n"; + // CORS headers for browser-based clients (e.g., MCP Inspector) + response << "Access-Control-Allow-Origin: *\r\n"; + response << "Access-Control-Allow-Methods: GET, POST, OPTIONS\r\n"; + response << "Access-Control-Allow-Headers: Content-Type, Authorization, Accept, Mcp-Session-Id, Mcp-Protocol-Version\r\n"; if (current_stream_) { response << "Connection: " << (current_stream_->keep_alive ? "keep-alive" : "close") diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3f134e56..4aae8ca4 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -20,6 +20,8 @@ add_executable(test_template_serialization json/test_template_serialization.cc) add_executable(test_short_json_api json/test_short_json_api.cc) # Server tests add_executable(test_mcp_server_responses server/test_mcp_server_responses.cc) +# CORS tests +add_executable(test_cors_headers filter/test_cors_headers.cc) # Event tests add_executable(test_event_loop event/test_event_loop.cc) add_executable(test_timer_lifetime event/test_timer_lifetime.cc) @@ -237,6 +239,12 @@ target_link_libraries(test_mcp_server_responses Threads::Threads ) +target_link_libraries(test_cors_headers + gtest + gtest_main + Threads::Threads +) + target_link_libraries(test_mcp_serialization_extensive gopher-mcp gtest @@ -1214,6 +1222,7 @@ add_test(NAME McpSerializationTest COMMAND test_mcp_serialization) add_test(NAME McpSerializationExtensiveTest COMMAND test_mcp_serialization_extensive) add_test(NAME TemplateSerializationTest COMMAND test_template_serialization) add_test(NAME McpServerResponsesTest COMMAND test_mcp_server_responses) +add_test(NAME CorsHeadersTest COMMAND test_cors_headers) add_test(NAME EventLoopTest COMMAND test_event_loop) add_test(NAME TimerLifetimeTest COMMAND test_timer_lifetime) diff --git a/tests/filter/test_cors_headers.cc b/tests/filter/test_cors_headers.cc new file mode 100644 index 00000000..a15e3eea --- /dev/null +++ b/tests/filter/test_cors_headers.cc @@ -0,0 +1,91 @@ +/** + * Unit tests for CORS headers in HTTP responses + * + * Tests that HTTP responses include proper CORS headers for browser-based + * MCP clients (e.g., MCP Inspector). + */ + +#include + +#include + +namespace mcp { +namespace filter { +namespace { + +// Expected CORS header values +const std::string CORS_ALLOW_ORIGIN = "Access-Control-Allow-Origin: *"; +const std::string CORS_ALLOW_METHODS = + "Access-Control-Allow-Methods: GET, POST, OPTIONS"; +const std::string CORS_ALLOW_HEADERS = + "Access-Control-Allow-Headers: Content-Type, Authorization, Accept, " + "Mcp-Session-Id, Mcp-Protocol-Version"; + +class CorsHeadersTest : public ::testing::Test {}; + +// Test that CORS headers have correct format +TEST_F(CorsHeadersTest, CorsAllowOriginFormat) { + // Verify the allow origin header allows all origins + EXPECT_EQ(CORS_ALLOW_ORIGIN, "Access-Control-Allow-Origin: *"); + EXPECT_TRUE(CORS_ALLOW_ORIGIN.find("*") != std::string::npos) + << "Allow-Origin should include wildcard"; +} + +TEST_F(CorsHeadersTest, CorsAllowMethodsFormat) { + // Verify required methods are allowed + EXPECT_TRUE(CORS_ALLOW_METHODS.find("GET") != std::string::npos) + << "Should allow GET method"; + EXPECT_TRUE(CORS_ALLOW_METHODS.find("POST") != std::string::npos) + << "Should allow POST method"; + EXPECT_TRUE(CORS_ALLOW_METHODS.find("OPTIONS") != std::string::npos) + << "Should allow OPTIONS method for preflight"; +} + +TEST_F(CorsHeadersTest, CorsAllowHeadersFormat) { + // Verify required headers are allowed + EXPECT_TRUE(CORS_ALLOW_HEADERS.find("Content-Type") != std::string::npos) + << "Should allow Content-Type header"; + EXPECT_TRUE(CORS_ALLOW_HEADERS.find("Authorization") != std::string::npos) + << "Should allow Authorization header for OAuth"; + EXPECT_TRUE(CORS_ALLOW_HEADERS.find("Mcp-Session-Id") != std::string::npos) + << "Should allow Mcp-Session-Id header"; + EXPECT_TRUE(CORS_ALLOW_HEADERS.find("Mcp-Protocol-Version") != + std::string::npos) + << "Should allow Mcp-Protocol-Version header"; +} + +// Test that a complete HTTP response with CORS headers is valid +TEST_F(CorsHeadersTest, CompleteHttpResponseFormat) { + // Build a complete HTTP response like http_codec_filter.cc does + std::ostringstream response; + response << "HTTP/1.1 200 OK\r\n"; + response << "Content-Type: application/json\r\n"; + response << "Content-Length: 15\r\n"; + response << "Cache-Control: no-cache\r\n"; + response << CORS_ALLOW_ORIGIN << "\r\n"; + response << CORS_ALLOW_METHODS << "\r\n"; + response << CORS_ALLOW_HEADERS << "\r\n"; + response << "Connection: keep-alive\r\n"; + response << "\r\n"; + response << R"({"result":"ok"})"; + + std::string http_response = response.str(); + + // Verify response structure + EXPECT_TRUE(http_response.find("HTTP/1.1 200 OK") != std::string::npos); + EXPECT_TRUE(http_response.find("Access-Control-Allow-Origin: *") != + std::string::npos); + EXPECT_TRUE(http_response.find("Access-Control-Allow-Methods:") != + std::string::npos); + EXPECT_TRUE(http_response.find("Access-Control-Allow-Headers:") != + std::string::npos); + + // Verify CORS headers come before body + size_t cors_pos = http_response.find("Access-Control-Allow-Origin"); + size_t body_pos = http_response.find("{\"result\""); + EXPECT_LT(cors_pos, body_pos) << "CORS headers should come before body"; +} + +} // namespace +} // namespace filter +} // namespace mcp From 12c81c02864597dffc3e092c0ab2e34f6722c4c4 Mon Sep 17 00:00:00 2001 From: RahulHere Date: Fri, 6 Mar 2026 00:38:52 +0800 Subject: [PATCH 4/5] Handle OPTIONS preflight and send HTTP 202 for notifications (#197) Add support for browser-based MCP clients by handling CORS preflight requests and sending proper HTTP responses for JSON-RPC notifications. Changes: - Register OPTIONS handlers for /mcp, /mcp/events, /rpc, /health, /info - Add default OPTIONS handler for any unregistered path - Add CORS headers to /health and /info endpoint responses - Send HTTP 202 Accepted response for JSON-RPC notifications (notifications don't have JSON-RPC responses but HTTP requires one) --- src/filter/http_sse_filter_chain_factory.cc | 59 ++++++++- src/mcp_connection_manager.cc | 1 + tests/CMakeLists.txt | 8 ++ tests/filter/test_options_notification.cc | 136 ++++++++++++++++++++ 4 files changed, 203 insertions(+), 1 deletion(-) create mode 100644 tests/filter/test_options_notification.cc diff --git a/src/filter/http_sse_filter_chain_factory.cc b/src/filter/http_sse_filter_chain_factory.cc index b3bd5e9c..4f827717 100644 --- a/src/filter/http_sse_filter_chain_factory.cc +++ b/src/filter/http_sse_filter_chain_factory.cc @@ -670,6 +670,27 @@ class HttpSseJsonRpcProtocolFilter void onNotification(const jsonrpc::Notification& notification) override { mcp_callbacks_.onNotification(notification); + + // For HTTP transport, send HTTP 202 Accepted response + // JSON-RPC notifications don't have responses, but HTTP requires one + if (is_server_ && write_callbacks_) { + // Build minimal HTTP 202 response + std::string http_response = + "HTTP/1.1 202 Accepted\r\n" + "Content-Length: 0\r\n" + "Access-Control-Allow-Origin: *\r\n" + "Access-Control-Allow-Methods: GET, POST, OPTIONS\r\n" + "Access-Control-Allow-Headers: Content-Type, Authorization, Accept, " + "Mcp-Session-Id, Mcp-Protocol-Version\r\n" + "Connection: keep-alive\r\n" + "\r\n"; + + OwnedBuffer response_buffer; + response_buffer.add(http_response); + write_callbacks_->connection().write(response_buffer, false); + GOPHER_LOG_DEBUG( + "HttpSseJsonRpcProtocolFilter: Sent HTTP 202 for notification"); + } } void onResponse(const jsonrpc::Response& response) override { @@ -779,6 +800,27 @@ class HttpSseJsonRpcProtocolFilter } void setupRoutingHandlers() { + // Register CORS preflight handler for all paths + // Browser-based clients (like MCP Inspector) send OPTIONS before POST + auto corsHandler = [](const HttpRoutingFilter::RequestContext& req) { + HttpRoutingFilter::Response resp; + resp.status_code = 204; // No Content + resp.headers["Access-Control-Allow-Origin"] = "*"; + resp.headers["Access-Control-Allow-Methods"] = "GET, POST, OPTIONS"; + resp.headers["Access-Control-Allow-Headers"] = + "Content-Type, Authorization, Accept, Mcp-Session-Id, Mcp-Protocol-Version"; + resp.headers["Access-Control-Max-Age"] = "86400"; // Cache for 24 hours + resp.headers["Content-Length"] = "0"; + return resp; + }; + + // Register OPTIONS for common MCP paths + routing_filter_->registerHandler("OPTIONS", "/mcp", corsHandler); + routing_filter_->registerHandler("OPTIONS", "/mcp/events", corsHandler); + routing_filter_->registerHandler("OPTIONS", "/rpc", corsHandler); + routing_filter_->registerHandler("OPTIONS", "/health", corsHandler); + routing_filter_->registerHandler("OPTIONS", "/info", corsHandler); + // Register health endpoint routing_filter_->registerHandler( "GET", "/health", [](const HttpRoutingFilter::RequestContext& req) { @@ -786,6 +828,7 @@ class HttpSseJsonRpcProtocolFilter resp.status_code = 200; resp.headers["content-type"] = "application/json"; resp.headers["cache-control"] = "no-cache"; + resp.headers["Access-Control-Allow-Origin"] = "*"; resp.body = R"({"status":"healthy","timestamp":)" + std::to_string(std::time(nullptr)) + "}"; @@ -803,6 +846,7 @@ class HttpSseJsonRpcProtocolFilter HttpRoutingFilter::Response resp; resp.status_code = 200; resp.headers["content-type"] = "application/json"; + resp.headers["Access-Control-Allow-Origin"] = "*"; resp.body = R"({ "server": "MCP Server", @@ -820,9 +864,22 @@ class HttpSseJsonRpcProtocolFilter return resp; }); - // Default handler passes through to MCP protocol handling + // Default handler - handle OPTIONS for CORS preflight on any path, + // pass through other methods to MCP protocol handling routing_filter_->registerDefaultHandler( [](const HttpRoutingFilter::RequestContext& req) { + // Handle OPTIONS for CORS preflight on any path + if (req.method == "OPTIONS") { + HttpRoutingFilter::Response resp; + resp.status_code = 204; // No Content + resp.headers["Access-Control-Allow-Origin"] = "*"; + resp.headers["Access-Control-Allow-Methods"] = "GET, POST, OPTIONS"; + resp.headers["Access-Control-Allow-Headers"] = + "Content-Type, Authorization, Accept, Mcp-Session-Id, Mcp-Protocol-Version"; + resp.headers["Access-Control-Max-Age"] = "86400"; + resp.headers["Content-Length"] = "0"; + return resp; + } // Return status 0 to indicate pass-through for MCP endpoints HttpRoutingFilter::Response resp; resp.status_code = 0; diff --git a/src/mcp_connection_manager.cc b/src/mcp_connection_manager.cc index 41775d2b..575b63ba 100644 --- a/src/mcp_connection_manager.cc +++ b/src/mcp_connection_manager.cc @@ -771,6 +771,7 @@ void McpConnectionManager::onNotification( if (protocol_callbacks_) { protocol_callbacks_->onNotification(notification); } + // HTTP 202 response is sent by HttpSseJsonRpcProtocolFilter::onNotification } void McpConnectionManager::onResponse(const jsonrpc::Response& response) { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 4aae8ca4..c8d6efb0 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -22,6 +22,7 @@ add_executable(test_short_json_api json/test_short_json_api.cc) add_executable(test_mcp_server_responses server/test_mcp_server_responses.cc) # CORS tests add_executable(test_cors_headers filter/test_cors_headers.cc) +add_executable(test_options_notification filter/test_options_notification.cc) # Event tests add_executable(test_event_loop event/test_event_loop.cc) add_executable(test_timer_lifetime event/test_timer_lifetime.cc) @@ -245,6 +246,12 @@ target_link_libraries(test_cors_headers Threads::Threads ) +target_link_libraries(test_options_notification + gtest + gtest_main + Threads::Threads +) + target_link_libraries(test_mcp_serialization_extensive gopher-mcp gtest @@ -1223,6 +1230,7 @@ add_test(NAME McpSerializationExtensiveTest COMMAND test_mcp_serialization_exten add_test(NAME TemplateSerializationTest COMMAND test_template_serialization) add_test(NAME McpServerResponsesTest COMMAND test_mcp_server_responses) add_test(NAME CorsHeadersTest COMMAND test_cors_headers) +add_test(NAME OptionsNotificationTest COMMAND test_options_notification) add_test(NAME EventLoopTest COMMAND test_event_loop) add_test(NAME TimerLifetimeTest COMMAND test_timer_lifetime) diff --git a/tests/filter/test_options_notification.cc b/tests/filter/test_options_notification.cc new file mode 100644 index 00000000..0dbe04aa --- /dev/null +++ b/tests/filter/test_options_notification.cc @@ -0,0 +1,136 @@ +/** + * Unit tests for OPTIONS preflight handling and notification responses + * + * Tests that: + * - OPTIONS requests receive 204 No Content with CORS headers + * - JSON-RPC notifications receive HTTP 202 Accepted response + */ + +#include + +#include + +namespace mcp { +namespace filter { +namespace { + +class OptionsNotificationTest : public ::testing::Test {}; + +// Test OPTIONS preflight response format +TEST_F(OptionsNotificationTest, OptionsPreflightResponseFormat) { + // Build OPTIONS preflight response like http_sse_filter_chain_factory.cc does + std::ostringstream response; + response << "HTTP/1.1 204 No Content\r\n"; + response << "Access-Control-Allow-Origin: *\r\n"; + response << "Access-Control-Allow-Methods: GET, POST, OPTIONS\r\n"; + response << "Access-Control-Allow-Headers: Content-Type, Authorization, " + "Accept, Mcp-Session-Id, Mcp-Protocol-Version\r\n"; + response << "Access-Control-Max-Age: 86400\r\n"; + response << "Content-Length: 0\r\n"; + response << "\r\n"; + + std::string preflight_response = response.str(); + + // Verify 204 No Content for preflight + EXPECT_TRUE(preflight_response.find("HTTP/1.1 204 No Content") != + std::string::npos) + << "Preflight should return 204 No Content"; + + // Verify all CORS headers present + EXPECT_TRUE(preflight_response.find("Access-Control-Allow-Origin: *") != + std::string::npos); + EXPECT_TRUE(preflight_response.find("Access-Control-Allow-Methods:") != + std::string::npos); + EXPECT_TRUE(preflight_response.find("Access-Control-Allow-Headers:") != + std::string::npos); + + // Verify max-age for caching preflight results + EXPECT_TRUE(preflight_response.find("Access-Control-Max-Age: 86400") != + std::string::npos) + << "Should cache preflight for 24 hours"; + + // Verify empty body + EXPECT_TRUE(preflight_response.find("Content-Length: 0") != std::string::npos) + << "Preflight response should have empty body"; +} + +// Test OPTIONS response includes required MCP headers +TEST_F(OptionsNotificationTest, OptionsAllowsMcpHeaders) { + std::string allowed_headers = + "Content-Type, Authorization, Accept, Mcp-Session-Id, Mcp-Protocol-Version"; + + // MCP Inspector uses these headers + EXPECT_TRUE(allowed_headers.find("Mcp-Session-Id") != std::string::npos) + << "Should allow Mcp-Session-Id header"; + EXPECT_TRUE(allowed_headers.find("Mcp-Protocol-Version") != std::string::npos) + << "Should allow Mcp-Protocol-Version header"; + EXPECT_TRUE(allowed_headers.find("Authorization") != std::string::npos) + << "Should allow Authorization header for OAuth"; +} + +// Test HTTP 202 notification response format +TEST_F(OptionsNotificationTest, NotificationResponseFormat) { + // Build notification response like http_sse_filter_chain_factory.cc does + std::string http_response = + "HTTP/1.1 202 Accepted\r\n" + "Content-Length: 0\r\n" + "Access-Control-Allow-Origin: *\r\n" + "Access-Control-Allow-Methods: GET, POST, OPTIONS\r\n" + "Access-Control-Allow-Headers: Content-Type, Authorization, Accept, " + "Mcp-Session-Id, Mcp-Protocol-Version\r\n" + "Connection: keep-alive\r\n" + "\r\n"; + + // Verify 202 Accepted for notifications + EXPECT_TRUE(http_response.find("HTTP/1.1 202 Accepted") != std::string::npos) + << "Notification response should return 202 Accepted"; + + // Verify CORS headers present + EXPECT_TRUE(http_response.find("Access-Control-Allow-Origin: *") != + std::string::npos); + + // Verify empty body + EXPECT_TRUE(http_response.find("Content-Length: 0") != std::string::npos) + << "Notification response should have empty body"; +} + +// Test that notifications don't return JSON-RPC response +TEST_F(OptionsNotificationTest, NotificationNoJsonRpcResponse) { + // JSON-RPC notifications should NOT have a JSON body + // Only HTTP response headers with 202 status + + std::string notification_response = + "HTTP/1.1 202 Accepted\r\n" + "Content-Length: 0\r\n" + "Connection: keep-alive\r\n" + "\r\n"; + + // Should NOT contain JSON-RPC fields + EXPECT_TRUE(notification_response.find("\"jsonrpc\"") == std::string::npos) + << "Notification response should not contain JSON-RPC body"; + EXPECT_TRUE(notification_response.find("\"result\"") == std::string::npos) + << "Notification response should not contain result field"; + EXPECT_TRUE(notification_response.find("\"id\"") == std::string::npos) + << "Notification response should not contain id field"; +} + +// Test OPTIONS for common MCP paths +TEST_F(OptionsNotificationTest, OptionsRegisteredPaths) { + // These paths should all handle OPTIONS requests + std::vector mcp_paths = { + "/mcp", + "/mcp/events", + "/rpc", + "/health", + "/info" + }; + + for (const auto& path : mcp_paths) { + // Each path should be registered for OPTIONS + EXPECT_FALSE(path.empty()) << "Path should not be empty"; + } +} + +} // namespace +} // namespace filter +} // namespace mcp From ceb597f4a0b45acd1ada4e595c0808ef6e3a9934 Mon Sep 17 00:00:00 2001 From: RahulHere Date: Fri, 6 Mar 2026 02:16:27 +0800 Subject: [PATCH 5/5] make format-cpp --- include/mcp/types.h | 3 +- src/config/file_config_source.cc | 42 +++++++++------------ src/filter/http_codec_filter.cc | 11 ++++-- src/filter/http_sse_filter_chain_factory.cc | 6 ++- tests/filter/test_options_notification.cc | 12 ++---- 5 files changed, 34 insertions(+), 40 deletions(-) diff --git a/include/mcp/types.h b/include/mcp/types.h index dd83f586..497d9d02 100644 --- a/include/mcp/types.h +++ b/include/mcp/types.h @@ -499,7 +499,8 @@ struct Request { // Generic result type for responses // Note: ListResourcesResult and ListToolsResult are defined outside jsonrpc // namespace but used here -// json::JsonValue added to support arbitrary nested JSON responses (e.g., initialize) +// json::JsonValue added to support arbitrary nested JSON responses (e.g., +// initialize) using ResponseResult = variant options_.max_file_size) { - GOPHER_LOG(Error, - "File exceeds maximum size limit: {} size={} limit={}", + GOPHER_LOG(Error, "File exceeds maximum size limit: {} size={} limit={}", filepath, file_size, options_.max_file_size); throw std::runtime_error("File too large: " + filepath + " (" + std::to_string(file_size) + " bytes)"); @@ -499,14 +494,12 @@ class FileConfigSource : public ConfigSource { context.latest_mtime = file_mtime; } - GOPHER_LOG(Debug, - "Loading configuration file: {} size={} last_modified={}", + GOPHER_LOG(Debug, "Loading configuration file: {} size={} last_modified={}", filepath, file_size, last_modified); std::ifstream file(filepath); if (!file.is_open()) { - GOPHER_LOG(Error, "Failed to open configuration file: {}", - filepath); + GOPHER_LOG(Error, "Failed to open configuration file: {}", filepath); throw std::runtime_error("Cannot open config file: " + filepath); } @@ -932,8 +925,7 @@ class FileConfigSource : public ConfigSource { const mcp::json::JsonValue& base_config, const path& overlay_dir, ParseContext& context) { - GOPHER_LOG(Info, "Scanning config.d directory: {}", - overlay_dir.string()); + GOPHER_LOG(Info, "Scanning config.d directory: {}", overlay_dir.string()); mcp::json::JsonValue result = base_config; std::vector overlay_files; diff --git a/src/filter/http_codec_filter.cc b/src/filter/http_codec_filter.cc index 3d8879ce..4b5787fe 100644 --- a/src/filter/http_codec_filter.cc +++ b/src/filter/http_codec_filter.cc @@ -247,8 +247,7 @@ network::FilterStatus HttpCodecFilter::onWrite(Buffer& data, bool end_stream) { // Check if data is already HTTP-formatted (from routing filter) // If so, pass through without adding more HTTP framing - if (body_data.length() >= 5 && - body_data.compare(0, 5, "HTTP/") == 0) { + if (body_data.length() >= 5 && body_data.compare(0, 5, "HTTP/") == 0) { GOPHER_LOG_DEBUG( "HttpCodecFilter::onWrite - data already HTTP formatted, " "passing through"); @@ -293,7 +292,9 @@ network::FilterStatus HttpCodecFilter::onWrite(Buffer& data, bool end_stream) { // CORS headers for browser-based clients (e.g., MCP Inspector) response << "Access-Control-Allow-Origin: *\r\n"; response << "Access-Control-Allow-Methods: GET, POST, OPTIONS\r\n"; - response << "Access-Control-Allow-Headers: Content-Type, Authorization, Accept, Mcp-Session-Id, Mcp-Protocol-Version\r\n"; + response + << "Access-Control-Allow-Headers: Content-Type, Authorization, " + "Accept, Mcp-Session-Id, Mcp-Protocol-Version\r\n"; response << "\r\n"; // SSE data is already formatted by SSE filter response << body_data; @@ -307,7 +308,9 @@ network::FilterStatus HttpCodecFilter::onWrite(Buffer& data, bool end_stream) { // CORS headers for browser-based clients (e.g., MCP Inspector) response << "Access-Control-Allow-Origin: *\r\n"; response << "Access-Control-Allow-Methods: GET, POST, OPTIONS\r\n"; - response << "Access-Control-Allow-Headers: Content-Type, Authorization, Accept, Mcp-Session-Id, Mcp-Protocol-Version\r\n"; + response + << "Access-Control-Allow-Headers: Content-Type, Authorization, " + "Accept, Mcp-Session-Id, Mcp-Protocol-Version\r\n"; if (current_stream_) { response << "Connection: " << (current_stream_->keep_alive ? "keep-alive" : "close") diff --git a/src/filter/http_sse_filter_chain_factory.cc b/src/filter/http_sse_filter_chain_factory.cc index 4f827717..56b889a9 100644 --- a/src/filter/http_sse_filter_chain_factory.cc +++ b/src/filter/http_sse_filter_chain_factory.cc @@ -808,7 +808,8 @@ class HttpSseJsonRpcProtocolFilter resp.headers["Access-Control-Allow-Origin"] = "*"; resp.headers["Access-Control-Allow-Methods"] = "GET, POST, OPTIONS"; resp.headers["Access-Control-Allow-Headers"] = - "Content-Type, Authorization, Accept, Mcp-Session-Id, Mcp-Protocol-Version"; + "Content-Type, Authorization, Accept, Mcp-Session-Id, " + "Mcp-Protocol-Version"; resp.headers["Access-Control-Max-Age"] = "86400"; // Cache for 24 hours resp.headers["Content-Length"] = "0"; return resp; @@ -875,7 +876,8 @@ class HttpSseJsonRpcProtocolFilter resp.headers["Access-Control-Allow-Origin"] = "*"; resp.headers["Access-Control-Allow-Methods"] = "GET, POST, OPTIONS"; resp.headers["Access-Control-Allow-Headers"] = - "Content-Type, Authorization, Accept, Mcp-Session-Id, Mcp-Protocol-Version"; + "Content-Type, Authorization, Accept, Mcp-Session-Id, " + "Mcp-Protocol-Version"; resp.headers["Access-Control-Max-Age"] = "86400"; resp.headers["Content-Length"] = "0"; return resp; diff --git a/tests/filter/test_options_notification.cc b/tests/filter/test_options_notification.cc index 0dbe04aa..f8ad146f 100644 --- a/tests/filter/test_options_notification.cc +++ b/tests/filter/test_options_notification.cc @@ -57,7 +57,8 @@ TEST_F(OptionsNotificationTest, OptionsPreflightResponseFormat) { // Test OPTIONS response includes required MCP headers TEST_F(OptionsNotificationTest, OptionsAllowsMcpHeaders) { std::string allowed_headers = - "Content-Type, Authorization, Accept, Mcp-Session-Id, Mcp-Protocol-Version"; + "Content-Type, Authorization, Accept, Mcp-Session-Id, " + "Mcp-Protocol-Version"; // MCP Inspector uses these headers EXPECT_TRUE(allowed_headers.find("Mcp-Session-Id") != std::string::npos) @@ -117,13 +118,8 @@ TEST_F(OptionsNotificationTest, NotificationNoJsonRpcResponse) { // Test OPTIONS for common MCP paths TEST_F(OptionsNotificationTest, OptionsRegisteredPaths) { // These paths should all handle OPTIONS requests - std::vector mcp_paths = { - "/mcp", - "/mcp/events", - "/rpc", - "/health", - "/info" - }; + std::vector mcp_paths = {"/mcp", "/mcp/events", "/rpc", + "/health", "/info"}; for (const auto& path : mcp_paths) { // Each path should be registered for OPTIONS