From 970799e161a9ecca465f76c008750ae66e281b2a Mon Sep 17 00:00:00 2001 From: lyf Date: Sun, 16 Aug 2026 15:05:48 +0800 Subject: [PATCH 1/2] fix: route MCP requests carrying the Streamable HTTP Accept header MCP's Streamable HTTP transport requires clients to send `Accept: application/json, text/event-stream` on every POST. ESPAsyncWebServer reads any Accept containing text/event-stream as an SSE subscription and marks the request RCT_EVENT regardless of method, after which AsyncCallbackWebHandler::canHandle() declines it (it insists on isHTTP()) and the request falls through to the catch-all. Every spec-conformant client got a 404 while a bare curl worked. * replace the three /mcp server->on() routes with a custom AsyncWebHandler that matches on path alone, keeping the connection type out of routing * dispatch by method in handleEndpointRequest, so the client's GET probe for a server-initiated stream gets the 405 the spec prescribes, not 404 * return false from begin() when the endpoint handler cannot be allocated, rolling back the worker task instead of listening on a dead server * model the Accept-driven RCT_EVENT downgrade in the test mock and resolve routes per request, so the gap is reproducible rather than assumed away * cover both the POST and the GET stream probe with regression tests --- include/MCPServer.h | 10 +- src/MCPServer.cpp | 107 +++++++++++++++------ test/mock/ESPAsyncWebServer.h | 98 ++++++++++++++++++- test/test_http_server/test_http_server.cpp | 42 +++++++- 4 files changed, 220 insertions(+), 37 deletions(-) diff --git a/include/MCPServer.h b/include/MCPServer.h index 14b43f9..37ed707 100644 --- a/include/MCPServer.h +++ b/include/MCPServer.h @@ -284,14 +284,16 @@ class MCPServer { /* Starts the HTTP listener. Register all tools first: doing so afterwards * would race the request handlers against mutations of the tool registry. * Call only once WiFi is connected — setupMDNS() reads WiFi.localIP() to - * publish the endpoint TXT record. Returns false if the server could not - * be created; a worker-task failure is not fatal (tool calls then run - * inline) and still returns true. */ + * publish the endpoint TXT record. Returns false if the server or its + * endpoint handler could not be created; a worker-task failure is not fatal + * (tool calls then run inline) and still returns true. */ bool begin(); private: - void setupWebServer(); + // False only when the endpoint handler could not be allocated. + bool setupWebServer(); void setupMDNS(); + void handleEndpointRequest(AsyncWebServerRequest* request); void handlePostComplete(AsyncWebServerRequest* request); void handleJsonBody(AsyncWebServerRequest* request, const char* body); void deferToolCall(AsyncWebServerRequest* request, MCPRequest&& mcpRequest); diff --git a/src/MCPServer.cpp b/src/MCPServer.cpp index a6814f0..737f2db 100644 --- a/src/MCPServer.cpp +++ b/src/MCPServer.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -92,6 +93,52 @@ class JobRef { HttpToolJob* job_; }; +/* Routes the MCP endpoint by path alone. + * + * server->on() cannot be used here. MCP's Streamable HTTP transport requires + * every client POST to carry `Accept: application/json, text/event-stream`, and + * ESPAsyncWebServer reads any Accept containing text/event-stream as an SSE + * subscription: it marks the request RCT_EVENT, after which + * AsyncCallbackWebHandler::canHandle() declines it (that check insists on + * isHTTP()) and the request falls through to the catch-all. The result is a 404 + * for every spec-conformant MCP client, on both POST and the GET stream probe, + * while a hand-written curl without the header works fine. + * + * Matching on the path and dispatching on the method ourselves keeps the + * connection type out of the routing decision. Nothing downstream consults it: + * RCT_EVENT only ever gates handler selection. */ +class MCPEndpointHandler : public AsyncWebHandler { +public: + using RequestFn = std::function; + using BodyFn = std::function; + + MCPEndpointHandler(const char* uri, RequestFn onRequest, BodyFn onBody) + : uri_(uri), onRequest_(std::move(onRequest)), onBody_(std::move(onBody)) {} + + bool canHandle(AsyncWebServerRequest* request) const override { + return request->url() == uri_; + } + + void handleRequest(AsyncWebServerRequest* request) override { + onRequest_(request); + } + + void handleBody(AsyncWebServerRequest* request, uint8_t* data, size_t len, size_t index, + size_t total) override { + onBody_(request, data, len, index, total); + } + + // A "trivial" handler never has the request body delivered to it. + bool isRequestHandlerTrivial() const override { + return false; + } + +private: + String uri_; + RequestFn onRequest_; + BodyFn onBody_; +}; + /* Appending sink for serializeJson. ArduinoJson's std::string writer clears the * destination in its constructor, so emitting several documents into one buffer * needs a writer that only ever appends; the default Writer template picks this @@ -145,7 +192,10 @@ bool MCPServer::begin() { // Best-effort: on failure tools/call degrades to inline execution. startWorker(); - setupWebServer(); + if (!setupWebServer()) { + stopWorker(); // nothing will ever feed it + return false; + } setupMDNS(); started = true; return true; @@ -241,12 +291,12 @@ void MCPServer::workerEntry(void* ctx) { // HTTP transport // --------------------------------------------------------------------------- -void MCPServer::setupWebServer() { +bool MCPServer::setupWebServer() { /* The body callback only accumulates; the response is always sent from the - * onRequest callback, which runs once the request is complete — including + * request callback, which runs once the request is complete — including * when there is no body at all. */ - server->on( - "/mcp", HTTP_POST, [this](AsyncWebServerRequest* request) { handlePostComplete(request); }, NULL, + auto* endpoint = new (std::nothrow) MCPEndpointHandler( + "/mcp", [this](AsyncWebServerRequest* request) { handleEndpointRequest(request); }, [this](AsyncWebServerRequest* request, uint8_t* data, size_t len, size_t index, size_t total) { BodyBuffer* body = static_cast(request->_tempObject); if (!body) { @@ -281,28 +331,10 @@ void MCPServer::setupWebServer() { memcpy(body->data + index, data, len); body->received = index + len; }); - - server->on("/mcp", HTTP_DELETE, [this](AsyncWebServerRequest* request) { - if (!validateOriginHeader(request)) { - sendJSONRPCError(request, 403, ErrorCode::INVALID_REQUEST, "Forbidden Origin"); - return; - } - AsyncWebServerResponse* response = - request->beginResponse(405, "application/json", "{\"error\":\"Method Not Allowed\"}"); - response->addHeader("Allow", "POST"); - request->send(response); - }); - - server->on("/mcp", HTTP_GET, [this](AsyncWebServerRequest* request) { - if (!validateOriginHeader(request)) { - sendJSONRPCError(request, 403, ErrorCode::INVALID_REQUEST, "Forbidden Origin"); - return; - } - AsyncWebServerResponse* response = - request->beginResponse(405, "application/json", "{\"error\":\"Method Not Allowed\"}"); - response->addHeader("Allow", "POST"); - request->send(response); - }); + if (!endpoint) { + return false; + } + server->addHandler(endpoint); // the server owns it from here server->onNotFound([this](AsyncWebServerRequest* request) { if (!validateOriginHeader(request)) { @@ -313,6 +345,27 @@ void MCPServer::setupWebServer() { }); server->begin(); + return true; +} + +/* Every method reaching /mcp lands here, since the endpoint handler matches on + * path alone. POST carries the JSON-RPC traffic; everything else — including the + * GET the MCP client uses to probe for a server-initiated SSE stream, which this + * server does not offer — is answered with the 405 the spec prescribes. */ +void MCPServer::handleEndpointRequest(AsyncWebServerRequest* request) { + if (request->method() == HTTP_POST) { + handlePostComplete(request); + return; + } + + if (!validateOriginHeader(request)) { + sendJSONRPCError(request, 403, ErrorCode::INVALID_REQUEST, "Forbidden Origin"); + return; + } + AsyncWebServerResponse* response = + request->beginResponse(405, "application/json", "{\"error\":\"Method Not Allowed\"}"); + response->addHeader("Allow", "POST"); + request->send(response); } std::string MCPServer::generateUUID() { diff --git a/test/mock/ESPAsyncWebServer.h b/test/mock/ESPAsyncWebServer.h index 3ce1515..addf863 100644 --- a/test/mock/ESPAsyncWebServer.h +++ b/test/mock/ESPAsyncWebServer.h @@ -7,8 +7,12 @@ * like the real library — so an aborted upload exercises the same teardown * path (a C++ object stored there would leak / corrupt the heap). * - Header lookup is case-insensitive. - * - Handlers are registered per (uri, method); tests fetch them via - * mock_async_web::lastServer() and drive body/request callbacks manually. + * - The MCP endpoint is registered as an AsyncWebHandler that matches on path + * alone (the real library would refuse a server->on() route for any request + * whose Accept carries text/event-stream). findRoute() probes the registered + * handlers with a synthetic request the same way AsyncWebServer::_attachHandler + * does, so tests still drive body/request callbacks per (uri, method) via + * mock_async_web::lastServer(). * - Chunked/deferred responses mirror AsyncChunkedResponse: send() keeps the * response object alive and the filler is pulled via pumpChunked() (the * test-side stand-in for the ack/poll cycle) until it returns 0; a filler @@ -24,7 +28,9 @@ #include #include #include +#include #include +#include #include #include @@ -111,6 +117,16 @@ class AsyncWebServerRequest { uint8_t version() const { return version_; } + const String& url() const { return url_; } + + int method() const { return method_; } + + /* The real library downgrades any request whose Accept carries + * text/event-stream to RCT_EVENT, and AsyncCallbackWebHandler::canHandle() + * then declines it. MCP clients send exactly that header on every POST, so + * routing has to be exercised with it modelled. */ + bool isHTTP() const { return !eventStreamAccept_; } + AsyncWebServerResponse* beginResponse(int code, const String& contentType, const String& body) { return new AsyncWebServerResponse(code, contentType, body); } @@ -172,6 +188,9 @@ class AsyncWebServerRequest { void setHeader(const char* name, const char* value) { headers_.emplace(lower(name), AsyncWebHeader(value)); + if (lower(name) == "accept" && lower(value).find("text/event-stream") != std::string::npos) { + eventStreamAccept_ = true; + } } void setContentLength(size_t length) { contentLength_ = length; } @@ -180,6 +199,10 @@ class AsyncWebServerRequest { void setVersion(uint8_t version) { version_ = version; } + void setUrl(const char* url) { url_ = url; } + + void setMethod(int method) { method_ = method; } + int responseCount = 0; int lastCode = -1; std::string lastContentType; @@ -206,12 +229,38 @@ class AsyncWebServerRequest { std::map headers_; size_t contentLength_ = 0; uint8_t version_ = 1; + String url_ = String("/mcp"); + int method_ = HTTP_POST; + bool eventStreamAccept_ = false; String contentType_ = String("application/json"); AsyncWebServerResponse* _pendingResponse = nullptr; std::string _chunkBody; size_t _chunkIndex = 0; }; +/* Mirrors the subset of the real base class that MCPServer overrides. The + * default canHandle() declines, matching the library. */ +class AsyncWebHandler { +public: + virtual ~AsyncWebHandler() = default; + + virtual bool canHandle(AsyncWebServerRequest* request) const { + (void)request; + return false; + } + virtual void handleRequest(AsyncWebServerRequest* request) { (void)request; } + virtual void handleBody(AsyncWebServerRequest* request, uint8_t* data, size_t len, size_t index, + size_t total) { + (void)request; + (void)data; + (void)len; + (void)index; + (void)total; + } + // The real server skips body delivery entirely for a trivial handler. + virtual bool isRequestHandlerTrivial() const { return true; } +}; + using ArRequestHandlerFunction = std::function; using ArUploadHandlerFunction = std::function; @@ -257,21 +306,62 @@ class AsyncWebServer { routes.push_back({uri, method, std::move(onRequest), std::move(onBody)}); } + // Takes ownership, like the real AsyncWebServer (a unique_ptr list). + AsyncWebHandler& addHandler(AsyncWebHandler* handler) { + handlers.emplace_back(handler); + return *handlers.back(); + } + void onNotFound(ArRequestHandlerFunction handler) { notFound = std::move(handler); } void begin() { started = true; } - const Route* findRoute(const char* uri, int method) const { + /* Resolves a request the way AsyncWebServer::_attachHandler does: explicit + * on() routes first — which, like AsyncCallbackWebHandler::canHandle(), + * decline anything that is not plain HTTP — then the registered handlers, + * which decide for themselves. A matched handler is surfaced as a + * forwarding Route so tests see one shape either way; a nullptr return is + * the catch-all (onNotFound) taking over. */ + const Route* findRoute(AsyncWebServerRequest* request) const { for (const auto& route : routes) { - if (route.uri == uri && (route.method & method) != 0) { + if (request->isHTTP() && route.uri == request->url().c_str() && + (route.method & request->method()) != 0) { return &route; } } + + for (const auto& handler : handlers) { + if (!handler->canHandle(request)) { + continue; + } + AsyncWebHandler* target = handler.get(); + // std::list: the returned pointer must survive later lookups. + handlerRoutes_.push_back( + Route{request->url().c_str(), request->method(), + [target](AsyncWebServerRequest* r) { target->handleRequest(r); }, + target->isRequestHandlerTrivial() + ? ArBodyHandlerFunction() + : [target](AsyncWebServerRequest* r, uint8_t* data, size_t len, size_t index, + size_t total) { target->handleBody(r, data, len, index, total); }}); + return &handlerRoutes_.back(); + } return nullptr; } + // Convenience for tests that only care about (uri, method) routing. + const Route* findRoute(const char* uri, int method) const { + AsyncWebServerRequest probe; + probe.setUrl(uri); + probe.setMethod(method); + return findRoute(&probe); + } + uint16_t port; bool started = false; std::vector routes; + std::vector> handlers; ArRequestHandlerFunction notFound; + +private: + mutable std::list handlerRoutes_; }; diff --git a/test/test_http_server/test_http_server.cpp b/test/test_http_server/test_http_server.cpp index b69e68f..d70f7d9 100644 --- a/test/test_http_server/test_http_server.cpp +++ b/test/test_http_server/test_http_server.cpp @@ -72,9 +72,11 @@ struct TestServer { }; /* Feeds `body` through the POST route in `chunkSize`-byte pieces and finishes - * the request. chunkSize 0 = one chunk. */ + * the request. chunkSize 0 = one chunk. The route is resolved from the request + * itself, so header-dependent routing (an SSE-flavoured Accept) is exercised + * rather than assumed away. */ void drivePost(TestServer& srv, AsyncWebServerRequest& req, const std::string& body, size_t chunkSize = 0) { - const AsyncWebServer::Route* route = srv.postRoute(); + const AsyncWebServer::Route* route = srv.web->findRoute(&req); TEST_ASSERT_NOT_NULL(route); req.setContentLength(body.size()); @@ -558,12 +560,44 @@ void test_ping_over_http(void) { TEST_ASSERT_NOT_NULL(strstr(req.lastBody.c_str(), "\"result\":{}")); } +void test_streamable_http_accept_header_still_routes_to_endpoint(void) { + /* MCP's Streamable HTTP transport requires this Accept on every client + * POST. ESPAsyncWebServer reads the text/event-stream part as an SSE + * subscription and refuses the request from any server->on() route, which + * sent every conforming client to the catch-all 404 while a bare curl + * worked. The endpoint therefore matches on path alone. */ + TestServer srv; + AsyncWebServerRequest req; + req.setHeader("Accept", "application/json, text/event-stream"); + drivePost(srv, req, R"({"jsonrpc":"2.0","id":1,"method":"ping"})"); + + TEST_ASSERT_EQUAL_INT(200, req.lastCode); + TEST_ASSERT_NOT_NULL(strstr(req.lastBody.c_str(), "\"result\":{}")); +} + +void test_streamable_http_accept_header_get_stream_probe_gets_405(void) { + // The client's server-initiated-stream probe. This server offers no stream; + // MCP prescribes 405 for that, and it must not fall through to the 404. + TestServer srv; + AsyncWebServerRequest req; + req.setMethod(HTTP_GET); + req.setHeader("Accept", "text/event-stream"); + + const AsyncWebServer::Route* route = srv.web->findRoute(&req); + TEST_ASSERT_NOT_NULL(route); + route->onRequest(&req); + + TEST_ASSERT_EQUAL_INT(405, req.lastCode); + TEST_ASSERT_EQUAL_STRING("POST", req.lastHeaders["Allow"].c_str()); +} + void test_get_mcp_returns_405(void) { TestServer srv; const AsyncWebServer::Route* route = srv.web->findRoute("/mcp", HTTP_GET); TEST_ASSERT_NOT_NULL(route); AsyncWebServerRequest req; + req.setMethod(HTTP_GET); route->onRequest(&req); TEST_ASSERT_EQUAL_INT(405, req.lastCode); @@ -575,6 +609,7 @@ void test_get_mcp_rejects_invalid_origin_before_method_check(void) { TEST_ASSERT_NOT_NULL(route); AsyncWebServerRequest req; + req.setMethod(HTTP_GET); req.setHeader("Origin", "http://evil.example:3000"); req.setHeader("Host", "evil.example:3000"); route->onRequest(&req); @@ -590,6 +625,7 @@ void test_delete_mcp_returns_405_with_allow(void) { TEST_ASSERT_NOT_NULL(route); AsyncWebServerRequest req; + req.setMethod(HTTP_DELETE); route->onRequest(&req); TEST_ASSERT_EQUAL_INT(405, req.lastCode); @@ -671,6 +707,8 @@ int main(int argc, char** argv) { RUN_TEST(test_advertised_mdns_origin_is_accepted_case_insensitively); RUN_TEST(test_notification_returns_202_without_body); RUN_TEST(test_ping_over_http); + RUN_TEST(test_streamable_http_accept_header_still_routes_to_endpoint); + RUN_TEST(test_streamable_http_accept_header_get_stream_probe_gets_405); RUN_TEST(test_get_mcp_returns_405); RUN_TEST(test_get_mcp_rejects_invalid_origin_before_method_check); RUN_TEST(test_delete_mcp_returns_405_with_allow); From 48dfa05ecfd17c36ba05403c14f2959738dcab9a Mon Sep 17 00:00:00 2001 From: lyf Date: Sun, 16 Aug 2026 15:06:34 +0800 Subject: [PATCH 2/2] bump: 0.2.1 * ship the /mcp routing fix for clients sending the Streamable HTTP Accept header; every consumer on 0.2.0 is unreachable by a conformant MCP client --- library.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library.json b/library.json index 6172f66..8a24220 100644 --- a/library.json +++ b/library.json @@ -1,6 +1,6 @@ { "name": "ESP32-MCPServer", - "version": "0.2.0", + "version": "0.2.1", "description": "Model Context Protocol (MCP) Server library for ESP32. Implements MCP server functionality over HTTP/JSON-RPC, with tool calls executed on a worker task so blocking handlers never stall the async TCP task.", "keywords": "mcp, server, esp32, async, llm, ai", "repository": {