diff --git a/include/mcp/types.h b/include/mcp/types.h index b760aa44..497d9d02 100644 --- a/include/mcp/types.h +++ b/include/mcp/types.h @@ -499,6 +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) using ResponseResult = variant, std::vector, ListResourcesResult, - ListToolsResult>; + ListToolsResult, + json::JsonValue>; struct Response { std::string jsonrpc = "2.0"; diff --git a/src/config/file_config_source.cc b/src/config/file_config_source.cc index 941e290d..2fd1ef27 100644 --- a/src/config/file_config_source.cc +++ b/src/config/file_config_source.cc @@ -268,8 +268,8 @@ class FileConfigSource : public ConfigSource { int priority, const Options& opts = Options{}) : name_(name), priority_(priority), options_(opts) { - GOPHER_LOG(Info, "FileConfigSource created: name={} priority={}", - name_, priority_); + GOPHER_LOG(Info, "FileConfigSource created: name={} priority={}", name_, + priority_); } std::string getName() const override { return name_; } @@ -285,18 +285,15 @@ class FileConfigSource : public ConfigSource { mcp::json::JsonValue loadConfiguration() override { // Keep logs under config.file so tests that attach a sink to // "config.file" see discovery start/end messages. - GOPHER_LOG(Info, "Starting configuration discovery for source: {}{}", - name_, - (options_.trace_id.empty() - ? "" - : (" trace_id=" + options_.trace_id))); + GOPHER_LOG( + Info, "Starting configuration discovery for source: {}{}", name_, + (options_.trace_id.empty() ? "" : (" trace_id=" + options_.trace_id))); // Determine the config file path using deterministic search order std::string config_path = findConfigFile(); if (config_path.empty()) { - GOPHER_LOG(Warning, "No configuration file found for source: {}", - name_); + GOPHER_LOG(Warning, "No configuration file found for source: {}", name_); return mcp::json::JsonValue::object(); } @@ -321,12 +318,12 @@ class FileConfigSource : public ConfigSource { base_config_path_ = config_path; // Emit a brief summary and also dump top-level keys/types to aid debugging - GOPHER_LOG( - Info, - "Configuration discovery completed: files_parsed={} " - "includes_processed={} env_vars_expanded={} overlays_applied={}", - context.files_parsed_count, context.includes_processed_count, - context.env_vars_expanded_count, context.overlays_applied.size()); + GOPHER_LOG(Info, + "Configuration discovery completed: files_parsed={} " + "includes_processed={} env_vars_expanded={} overlays_applied={}", + context.files_parsed_count, context.includes_processed_count, + context.env_vars_expanded_count, + context.overlays_applied.size()); if (config.isObject()) { for (const auto& key : config.keys()) { @@ -451,8 +448,7 @@ class FileConfigSource : public ConfigSource { if (exists(path)) { // Determine which source won if (i == 0 && !explicit_config_path_.empty()) { - GOPHER_LOG(Info, "Configuration source won: CLI --config={}", - path); + GOPHER_LOG(Info, "Configuration source won: CLI --config={}", path); } else if ((i == 0 || i == 1) && env_config) { GOPHER_LOG( Info, @@ -483,8 +479,7 @@ class FileConfigSource : public ConfigSource { } size_t file_size = st.st_size; if (file_size > 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 a3bb46b5..4b5787fe 100644 --- a/src/filter/http_codec_filter.cc +++ b/src/filter/http_codec_filter.cc @@ -245,6 +245,15 @@ 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 +289,12 @@ 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 +305,12 @@ 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/src/filter/http_sse_filter_chain_factory.cc b/src/filter/http_sse_filter_chain_factory.cc index b3bd5e9c..56b889a9 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,28 @@ 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 +829,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 +847,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 +865,23 @@ 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/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/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/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..c8d6efb0 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -18,6 +18,11 @@ 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) +# 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) @@ -223,7 +228,26 @@ 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 +) + +target_link_libraries(test_cors_headers + gtest + gtest_main + Threads::Threads +) + +target_link_libraries(test_options_notification + gtest gtest_main Threads::Threads ) @@ -1204,6 +1228,9 @@ 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 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_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 diff --git a/tests/filter/test_options_notification.cc b/tests/filter/test_options_notification.cc new file mode 100644 index 00000000..f8ad146f --- /dev/null +++ b/tests/filter/test_options_notification.cc @@ -0,0 +1,132 @@ +/** + * 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 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 // ============================================================================= 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; +}