Parcourir la source

refactor: make the request body cap configurable, and raise it to 32 MB

HttpServer already bounded every request body: set_payload_max_length()
was called with a hardcoded 16 MB literal where it constructs
httplib::Server. That cap was real but unconfigurable, and it disagreed
with a second, lower one - the runner's own gRPC server accepted at
most grpc++'s default 4 MB per message, so a body already past the
HTTP cap could still be rejected one hop later when the webserver
forwarded it to a runner.

Add server.max_upload_mb (default 32) to webserver.json, thread it
through WebServerServiceConfig and HttpServerConfig, and apply it via
set_payload_max_length() in place of the old literal. On the runner
side, apply the existing (previously database-client-only)
max_message_size_mb to the runner's own grpc::ServerBuilder so the two
hops agree.

The default rises from 16 MB to 32 MB because an upload node needs to
carry up to a 16 MB image field, and that field cannot fit inside a
16 MB total body once multipart framing is added on top.

Add bodyPadBytes support to verify-node.py's run_http_case and two
tests that bracket the new cap: a 34 MB body, over both the old and
new limits, expects 413; a 20 MB body, over the old 16 MB limit but
under the new 32 MB one, expects 200 - the second is the one that
actually fails if the old literal were still in force or either
config were not wired through.
fszontagh il y a 1 mois
Parent
commit
094cfbef19

+ 3 - 0
config/webserver.json

@@ -5,6 +5,9 @@
   "static_files_path": "${WEBUI_PATH:./webui/dist}",
   "database_address": "${DATABASE_ADDRESS:zeus.fsociety.hu:9004}",
   "database_project": "${DATABASE_PROJECT:smartbotic-automation}",
+  "server": {
+    "max_upload_mb": 32
+  },
   "runners": {
     "load_balancing": "least-connections",
     "heartbeat_timeout_sec": 30,

+ 6 - 1
scripts/verify-node.py

@@ -60,7 +60,12 @@ def run_http_case(case, token, workflow_id):
     """A webhook case asserts on the HTTP response, not on node outputs."""
     spec = case["http"]
     url = BASE.replace("/api/v1", "") + spec["path"].replace("{workflowId}", workflow_id)
-    data = json.dumps(spec.get("body", {})).encode()
+    body = spec.get("body", {})
+    pad = spec.get("bodyPadBytes", 0)
+    if pad:
+        body = dict(body)
+        body["_pad"] = "x" * pad
+    data = json.dumps(body).encode()
     req = urllib.request.Request(url, data=data, method=spec.get("method", "POST"))
     req.add_header("Content-Type", "application/json")
 

+ 12 - 0
src/runner/runner_service.cpp

@@ -748,6 +748,18 @@ void RunnerService::start() {
                             grpc::InsecureServerCredentials());
     builder.RegisterService(service_impl_.get());
 
+    // grpc++ defaults an unset receive limit to 4 MB, well under a webhook
+    // body the webserver now accepts up to server.max_upload_mb (32 MB by
+    // default) for. ExecuteWorkflow carries that body from the webserver to
+    // this server as part of the request, so without raising this the
+    // gRPC hop silently re-imposes a lower cap than the HTTP one already
+    // passed. Reuse max_message_size_mb - already read from config above
+    // and already applied to the database client - instead of adding a
+    // second knob for the same idea.
+    const int max_message_bytes = config_.max_message_size_mb * 1024 * 1024;
+    builder.SetMaxReceiveMessageSize(max_message_bytes);
+    builder.SetMaxSendMessageSize(max_message_bytes);
+
     server_ = builder.BuildAndStart();
     LOG_INFO("Runner gRPC server listening on port {}", config_.grpc_port);
 

+ 11 - 1
src/webserver/http_server.cpp

@@ -8,7 +8,17 @@ HttpServer::HttpServer(const HttpServerConfig& config)
     : config_(config) {
 
     // Configure server
-    server_.set_payload_max_length(1024 * 1024 * 16);  // 16MB max payload
+    //
+    // This line already bounded every request body before this change - it
+    // used to be a hardcoded 16 MB literal. The bound was real, just not
+    // configurable, and it disagreed with the (previously unused) SIZE_MAX
+    // default that httplib itself falls back to when nothing sets this at
+    // all. Reading the limit from config makes the two paths agree and lets
+    // it be raised deliberately: an upload node (see docs/nodes.md) needs to
+    // carry up to a 16 MB image field, and that field cannot fit inside a
+    // 16 MB total body once multipart framing is added on top, so the
+    // shipped default here goes up to 32 MB.
+    server_.set_payload_max_length(static_cast<size_t>(config_.max_upload_mb) * 1024 * 1024);
 
     // Compression is auto-enabled when supported by client
 

+ 1 - 0
src/webserver/http_server.hpp

@@ -12,6 +12,7 @@ struct HttpServerConfig {
     int port = 8080;
     std::string static_files_path = "./webui/dist";
     int thread_pool_size = 8;
+    int max_upload_mb = 32;
 };
 
 class HttpServer {

+ 10 - 0
src/webserver/webserver_service.cpp

@@ -156,6 +156,7 @@ WebServerService::WebServerService(const WebServerServiceConfig& config)
     HttpServerConfig http_config;
     http_config.port = config_.http_port;
     http_config.static_files_path = config_.static_files_path;
+    http_config.max_upload_mb = config_.max_upload_mb;
     http_server_ = std::make_unique<HttpServer>(http_config);
 
     // Initialize WebSocket server on separate port
@@ -188,6 +189,15 @@ WebServerServiceConfig WebServerService::loadConfig(const std::filesystem::path&
         config.database_address = cfg.getOr<std::string>("database_address", "localhost:9004");
         config.database_project = cfg.getOr<std::string>("database_project", "smartbotic-automation");
 
+        // HttpServer already enforced a body cap before this - a hardcoded
+        // 16 MB literal at the point it constructs httplib::Server - so this
+        // is not closing an open door, it is making that cap configurable
+        // and raising it. The default goes from 16 MB to 32 MB because an
+        // upload node needs to carry up to a 16 MB image field, and that
+        // field cannot fit inside a 16 MB total body once multipart framing
+        // is added on top.
+        config.max_upload_mb = cfg.getOr<int>("server.max_upload_mb", 32);
+
         // Runner config
         config.runner_config.heartbeat_timeout_sec =
             cfg.getOr<int>("runners.heartbeat_timeout_sec", 30);

+ 1 - 0
src/webserver/webserver_service.hpp

@@ -65,6 +65,7 @@ struct WebServerServiceConfig {
     std::string static_files_path = "./webui/dist";
     std::string database_address = "localhost:9004";
     std::string database_project = "smartbotic-automation";
+    int max_upload_mb = 32;
     runners::RunnerRegistryConfig runner_config;
     runners::LoadBalancerConfig load_balancer_config;
     auth::JwtUtils::Config jwt_config;

+ 13 - 0
tests/nodes/webhook-body-too-large.json

@@ -0,0 +1,13 @@
+{
+  "name": "verify-webhook-body-too-large",
+  "nodes": [
+    {"id": "n1", "name": "Webhook", "type": "post-trigger", "position": {"x": 0, "y": 0}, "config": {}}
+  ],
+  "connections": [],
+  "http": {
+    "method": "POST",
+    "path": "/webhook/{workflowId}",
+    "bodyPadBytes": 34000000,
+    "expectStatus": 413
+  }
+}

+ 13 - 0
tests/nodes/webhook-body-within-cap.json

@@ -0,0 +1,13 @@
+{
+  "name": "verify-webhook-body-within-cap",
+  "nodes": [
+    {"id": "n1", "name": "Webhook", "type": "post-trigger", "position": {"x": 0, "y": 0}, "config": {}}
+  ],
+  "connections": [],
+  "http": {
+    "method": "POST",
+    "path": "/webhook/{workflowId}",
+    "bodyPadBytes": 20000000,
+    "expectStatus": 200
+  }
+}