Added new Acceptor design pattern and fixed static analyzer warnings

Author: rianna-parla

Dockerfile.demo

@@ -22,8 +22,11 @@ WORKDIR /src
 COPY . .
 
 # Configure and build (Release + tests)
+# Show Ninja progress and limit parallelism to reduce resource contention inside Docker
+ENV NINJA_STATUS="[%f/%t %o/sec %es] "
+ENV CMAKE_BUILD_PARALLEL_LEVEL=6
 RUN cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DMCP_BUILD_TESTS=ON \
- && cmake --build build -j
+ && cmake --build build -j -- -v
 
 # 1b) Test stage: run unit tests inside the build environment
 FROM build AS test
@@ -32,7 +35,9 @@ ENV MCP_STDIOTRANSPORT_TIMEOUT_MS=${MCP_STDIOTRANSPORT_TIMEOUT_MS}
 ENV GTEST_COLOR=yes
 ENV CTEST_OUTPUT_ON_FAILURE=1
 WORKDIR /src
-RUN ctest --test-dir build -VV --progress
+# Set a per-test timeout to avoid indefinite hangs and show verbose progress
+ENV CTEST_PARALLEL_LEVEL=4
+RUN ctest --test-dir build -VV --progress --timeout 180
 
 # 2) Demo runtime stage
 FROM ubuntu:24.04 AS demo

include/mcp/HTTPServer.hpp

@@ -11,13 +11,14 @@
 #include <future>
 #include <functional>
 #include <memory>
-
+#include "mcp/Transport.h"
 #include "mcp/JSONRPCTypes.h"
 
 namespace mcp {
 
-class HTTPServer {
-public:
+  // Note: HTTPServer implements the server-side acceptor role (ITransportAcceptor)
+  class HTTPServer : public ITransportAcceptor {
+  public:
     //==========================================================================================================
     // Options
     // Purpose: Configuration for bind address/port, JSON-RPC paths, and TLS files.
@@ -39,10 +40,6 @@ class HTTPServer {
         std::string keyFile;  // PEM (required for https)
     };
 
-    using RequestHandler = std::function<std::unique_ptr<JSONRPCResponse>(const JSONRPCRequest&)>;
-    using NotificationHandler = std::function<void(std::unique_ptr<JSONRPCNotification>)>;
-    using ErrorHandler   = std::function<void(const std::string&)>;
-
     explicit HTTPServer(const Options& opts);
     ~HTTPServer();
 
@@ -51,39 +48,52 @@ class HTTPServer {
     // Returns:
     //   Future that becomes ready once the I/O context is running.
     //==========================================================================================================
-    std::future<void> Start();
+    std::future<void> Start() override;
 
     //==========================================================================================================
     // Stops the server: closes acceptor, stops I/O context, and joins background thread.
     // Returns:
     //   Future that completes when shutdown has finished.
     //==========================================================================================================
-    std::future<void> Stop();
+    std::future<void> Stop() override;
 
     //==========================================================================================================
     // Sets the request handler (JSON-RPC request -> response).
     // Args:
     //   handler: Callback invoked per request; may return an error response.
     //==========================================================================================================
-    void SetRequestHandler(RequestHandler handler);
+    void SetRequestHandler(ITransport::RequestHandler handler) override;
 
     //==========================================================================================================
     // Sets the notification handler (no response expected).
     // Args:
     //   handler: Callback invoked per notification with ownership of the notification object.
     //==========================================================================================================
-    void SetNotificationHandler(NotificationHandler handler);
+    void SetNotificationHandler(ITransport::NotificationHandler handler) override;
 
     //==========================================================================================================
     // Sets the error handler for transport/server errors.
     // Args:
     //   handler: Callback invoked with error strings.
     //==========================================================================================================
-    void SetErrorHandler(ErrorHandler handler);
+    void SetErrorHandler(ITransport::ErrorHandler handler) override;
 
 private:
     class Impl;
     std::unique_ptr<Impl> pImpl;
-};
+  };
+
+  //==========================================================================================================
+  // HTTPServerFactory
+  // Purpose: Factory for creating HTTP/HTTPS server acceptors (ITransportAcceptor) from a configuration
+  //          string. The configuration format is uri format, intentionally simple and stable:
+  //            - "http://<address>:<port>" (e.g., http://127.0.0.1:0)
+  //            - "https://<address>:<port>?cert=<pem>&key=<pem>"
+  //          Unknown parameters are ignored. If scheme is omitted, defaults to http.
+  //==========================================================================================================
+  class HTTPServerFactory : public ITransportAcceptorFactory {
+  public:
+    std::unique_ptr<ITransportAcceptor> CreateTransportAcceptor(const std::string& config) override;
+  };
 
 } // namespace mcp

include/mcp/HTTPTransport.hpp

@@ -98,6 +98,10 @@ class HTTPTransport : public ITransport {
     std::unique_ptr<Impl> pImpl;
 };
 
+//==========================================================================================================
+// HTTPTransportFactory
+// Purpose: Factory for creating HTTP/HTTPS transports.
+//==========================================================================================================
 class HTTPTransportFactory : public ITransportFactory {
 public:
     std::unique_ptr<ITransport> CreateTransport(const std::string& config) override;

include/mcp/InMemoryTransport.hpp

@@ -79,6 +79,10 @@ class InMemoryTransport : public ITransport {
     std::unique_ptr<Impl> pImpl;
 };
 
+//==========================================================================================================
+// InMemoryTransportFactory
+// Purpose: Factory for creating in-memory transports.
+//==========================================================================================================
 class InMemoryTransportFactory : public ITransportFactory {
 public:
     std::unique_ptr<ITransport> CreateTransport(const std::string& config) override;

include/mcp/StdioTransport.hpp

@@ -102,6 +102,10 @@ class StdioTransport : public ITransport {
     std::unique_ptr<Impl> pImpl;
 };
 
+//==========================================================================================================
+// StdioTransportFactory
+// Purpose: Factory for creating stdio transports.
+//==========================================================================================================
 class StdioTransportFactory : public ITransportFactory {
 public:
     std::unique_ptr<ITransport> CreateTransport(const std::string& config) override;

include/mcp/Transport.h

@@ -124,6 +124,8 @@ class ITransport {
 // Concrete transports are declared in their respective headers:
 //  - mcp/StdioTransport.hpp
 //  - mcp/InMemoryTransport.hpp
+//  - mcp/HTTPTransport.hpp
+//  - mcp/HTTPServer.hpp (implements ITransportAcceptor)
 
 //==========================================================================================================
 // Transport factory interface
@@ -143,4 +145,79 @@ class ITransportFactory {
     virtual std::unique_ptr<ITransport> CreateTransport(const std::string& config) = 0;
 };
 
+//==========================================================================================================
+// ITransportAcceptor
+// Purpose: Server-side acceptor interface which owns the listen lifecycle and dispatches incoming
+//          JSON-RPC requests/notifications to registered handlers.
+// Notes:
+//   - Keeps the multi-client accept/listen role separate from the single-session ITransport.
+//   - Implementations should bind/listen in Start(), stop/teardown in Stop(), and invoke the
+//     registered handlers upon incoming messages.
+//   - This interface mirrors the handler signatures of ITransport for consistency.
+//==========================================================================================================
+class ITransportAcceptor {
+public:
+    virtual ~ITransportAcceptor() = default;
+
+    //==========================================================================================================
+    // Starts the acceptor (binds/listens/spawns accept loop as needed).
+    // Args:
+    //   (none)
+    // Returns:
+    //   Future that completes when the accept loop is running.
+    //==========================================================================================================
+    virtual std::future<void> Start() = 0;
+
+    //==========================================================================================================
+    // Stops the acceptor and releases resources (closes listener and active sessions gracefully).
+    // Args:
+    //   (none)
+    // Returns:
+    //   Future that completes when the acceptor has stopped.
+    //==========================================================================================================
+    virtual std::future<void> Stop() = 0;
+
+    //==========================================================================================================
+    // Registers the server-side JSON-RPC request handler. Invoked per request, must return a response.
+    // Args:
+    //   handler: Callback taking const JSONRPCRequest& and returning unique_ptr<JSONRPCResponse>.
+    // Returns:
+    //   (none)
+    //==========================================================================================================
+    virtual void SetRequestHandler(ITransport::RequestHandler handler) = 0;
+
+    //==========================================================================================================
+    // Registers the JSON-RPC notification handler (no response expected).
+    // Args:
+    //   handler: Callback invoked with ownership of the incoming notification object.
+    // Returns:
+    //   (none)
+    //==========================================================================================================
+    virtual void SetNotificationHandler(ITransport::NotificationHandler handler) = 0;
+
+    //==========================================================================================================
+    // Registers an error handler to receive acceptor/transport errors.
+    // Args:
+    //   handler: Callback receiving error strings.
+    virtual void SetErrorHandler(ITransport::ErrorHandler handler) = 0;
+};
+
+//==========================================================================================================
+// Transport acceptor factory interface
+// Purpose: Factory for creating server-side acceptors from configuration strings.
+//==========================================================================================================
+  class ITransportAcceptorFactory {
+  public:
+    virtual ~ITransportAcceptorFactory() = default;
+
+    //==========================================================================================================
+    // Creates a server-side acceptor instance using the provided configuration.
+    // Args:
+    //   config: Transport-specific configuration string (e.g., "http://127.0.0.1:9443").
+    // Returns:
+    //   A unique_ptr to a newly created ITransportAcceptor.
+    //==========================================================================================================
+    virtual std::unique_ptr<ITransportAcceptor> CreateTransportAcceptor(const std::string& config) = 0;
+  };
+
 } // namespace mcp

src/mcp/HTTPServer.cpp

@@ -9,6 +9,8 @@
 #include <thread>
 #include <atomic>
 #include <sstream>
+#include <algorithm>
+#include <cctype>
 
 #include <boost/asio.hpp>
 #include <boost/asio/ssl.hpp>
@@ -41,9 +43,9 @@ class HTTPServer::Impl {
     std::unique_ptr<ssl::context> sslCtx; // present when scheme==https
     std::thread ioThread;
 
-    HTTPServer::RequestHandler requestHandler;
-    HTTPServer::NotificationHandler notificationHandler;
-    HTTPServer::ErrorHandler errorHandler;
+    ITransport::RequestHandler requestHandler;
+    ITransport::NotificationHandler notificationHandler;
+    ITransport::ErrorHandler errorHandler;
 
     explicit Impl(const HTTPServer::Options& o) : opts(o) {
         if (opts.scheme == "https") {
@@ -206,6 +208,27 @@ class HTTPServer::Impl {
 
     net::awaitable<void> acceptLoop() {
         try {
+            // Validate port strictly: numeric and within [0, 65535]
+            if (opts.port.empty()) {
+                setError("HTTPServer invalid port: empty");
+                co_return;
+            }
+            bool allDigits = std::all_of(opts.port.begin(), opts.port.end(), [](unsigned char ch){ return std::isdigit(ch) != 0; });
+            if (!allDigits) {
+                setError(std::string("HTTPServer invalid port (non-numeric): ") + opts.port);
+                co_return;
+            }
+            unsigned long portNum = 0;
+            try {
+                portNum = std::stoul(opts.port);
+            } catch (...) {
+                setError(std::string("HTTPServer invalid port (parse failure): ") + opts.port);
+                co_return;
+            }
+            if (portNum > 65535ul) {
+                setError(std::string("HTTPServer invalid port (out of range): ") + opts.port);
+                co_return;
+            }
             tcp::resolver resolver(co_await net::this_coro::executor);
             auto r = resolver.resolve(opts.address, opts.port);
             tcp::endpoint ep = *r.begin();
@@ -284,14 +307,97 @@ std::future<void> HTTPServer::Stop() {
     return fut;
 }
 
-void HTTPServer::SetRequestHandler(RequestHandler handler) { 
+void HTTPServer::SetRequestHandler(ITransport::RequestHandler handler) { 
     pImpl->requestHandler = std::move(handler);
 }
-void HTTPServer::SetNotificationHandler(NotificationHandler handler) { 
+void HTTPServer::SetNotificationHandler(ITransport::NotificationHandler handler) { 
     pImpl->notificationHandler = std::move(handler);
 }
-void HTTPServer::SetErrorHandler(ErrorHandler handler) {
-    pImpl->errorHandler = std::move(handler);
+  
+void HTTPServer::SetErrorHandler(ITransport::ErrorHandler handler) {
+      pImpl->errorHandler = std::move(handler);
+ }
+
+std::unique_ptr<ITransportAcceptor> HTTPServerFactory::CreateTransportAcceptor(const std::string& config) {
+    HTTPServer::Options opts;
+    // Factory default: http if scheme omitted
+    opts.scheme = "http";
+
+    std::string cfg = config;
+    // Trim leading/trailing spaces
+    auto trim = [](std::string& s){
+        auto notSpace = [](unsigned char c){ return !std::isspace(c); };
+        s.erase(s.begin(), std::find_if(s.begin(), s.end(), notSpace));
+        s.erase(std::find_if(s.rbegin(), s.rend(), notSpace).base(), s.end());
+    };
+    trim(cfg);
+
+    // Detect scheme
+    auto startsWith = [](const std::string& s, const char* pfx){ return s.rfind(pfx, 0) == 0; };
+    if (startsWith(cfg, "http://")) {
+        opts.scheme = "http";
+        cfg = cfg.substr(7);
+    } else if (startsWith(cfg, "https://")) {
+        opts.scheme = "https";
+        cfg = cfg.substr(8);
+    }
+
+    // Split query params
+    std::string hostPortPath = cfg;
+    std::string query;
+    auto qpos = cfg.find('?');
+    if (qpos != std::string::npos) {
+        hostPortPath = cfg.substr(0, qpos);
+        query = cfg.substr(qpos + 1);
+    }
+
+    // Strip path component if present
+    std::string hostPort = hostPortPath;
+    auto slash = hostPortPath.find('/');
+    if (slash != std::string::npos) {
+        hostPort = hostPortPath.substr(0, slash);
+    }
+    trim(hostPort);
+
+    // Parse host[:port] including IPv4/IPv6 in [addr]:port form
+    if (!hostPort.empty()) {
+        if (hostPort.front() == '[') {
+            auto rb = hostPort.find(']');
+            if (rb != std::string::npos) {
+                opts.address = hostPort.substr(1, rb - 1);
+                if (rb + 1 < hostPort.size() && hostPort[rb + 1] == ':') {
+                    opts.port = hostPort.substr(rb + 2);
+                }
+            }
+        } else {
+            auto colon = hostPort.rfind(':');
+            if (colon != std::string::npos) {
+                opts.address = hostPort.substr(0, colon);
+                opts.port = hostPort.substr(colon + 1);
+            } else {
+                opts.address = hostPort;
+            }
+        }
+        trim(opts.address);
+        trim(opts.port);
+        if (opts.port.empty()) opts.port = "9443"; // default
+    }
+
+    // Parse query parameters: cert, key
+    if (!query.empty()) {
+        std::stringstream ss(query);
+        std::string kv;
+        while (std::getline(ss, kv, '&')) {
+            auto eq = kv.find('=');
+            std::string key = (eq == std::string::npos) ? kv : kv.substr(0, eq);
+            std::string val = (eq == std::string::npos) ? std::string() : kv.substr(eq + 1);
+            if (key == "cert") opts.certFile = val;
+            else if (key == "key") opts.keyFile = val;
+        }
+    }
+
+    // Return server acceptor
+    return std::unique_ptr<ITransportAcceptor>(new HTTPServer(opts));
 }
 
 } // namespace mcp