On Fri, Dec 27, 2013 at 8:49 AM, Justin Erenkrantz
<jus...@erenkrantz.com> wrote:
> Anyway, I'm diving into the code a bit - but, I figured it might be useful
> to see if anyone else has any thoughts about how to handle large request
> bodies with mod_fcgid.

Here's a first-cut patch that compiles at least.  Cheers.  -- justin

Add FcgidStreamRequestBody directive to send request body as it arrives without
storing it in memory or on disk.

* modules/fcgid/mod_fcgid.c
  (fcgid_cmds): Add FcgidStreamRequestBody directive.
* modules/fcgid/fcgid_conf.h
  (fcgid_server_conf): Add stream_request_body/stream_request_body_set flags.
* modules/fcgid/fcgid_conf.c
  (DEFAULT_STREAM_REQUEST_BODY): Set to 0.
  (create_fcgid_server_config): Init stream_request_body flag.
  (merge_fcgid_server_config): Merge new stream_request_body flag.
  (set_stream_request_body): New config helper.
* modules/fcgid/fcgid_bridge.c
  (add_request_body): Add a forward declaration to reduce diff (for now);
  take fcgid_bucket_ctx param; if stream_request_body is set, don't copy
  bucket and then call proc_write_ipc to send out that brigade and clear it
  out before next loop iteration.
  (bridge_request): Delay reading the request body until later.
  (handle_request_ipc): Move add_request_body call to here and append the EOS
  bucket before we do the final write of the request body.

Index: modules/fcgid/fcgid_bridge.c
===================================================================
--- modules/fcgid/fcgid_bridge.c (revision 1553671)
+++ modules/fcgid/fcgid_bridge.c (working copy)
@@ -287,6 +287,10 @@ static int getsfunc_fcgid_BRIGADE(char *buf, int l
     return done;
 }

+static int add_request_body(request_rec *r, apr_pool_t *request_pool,
+                            apr_bucket_brigade *output_brigade,
+                            fcgid_bucket_ctx *bucket_ctx);
+
 static int
 handle_request_ipc(request_rec *r, int role,
                    apr_bucket_brigade *output_brigade,
@@ -295,9 +299,21 @@ handle_request_ipc(request_rec *r, int role,
     int cond_status;
     apr_status_t rv;
     apr_bucket_brigade *brigade_stdout;
+    apr_bucket *bucket_eos;
     char sbuf[MAX_STRING_LEN];
     const char *location;

+    if (role == FCGI_RESPONDER) {
+        rv = add_request_body(r, r->pool, output_brigade, bucket_ctx);
+        if (rv) {
+            return rv;
+        }
+    }
+
+    /* The eos bucket now */
+    bucket_eos = apr_bucket_eos_create(r->connection->bucket_alloc);
+    APR_BRIGADE_INSERT_TAIL(output_brigade, bucket_eos);
+
     /* Write output_brigade to fastcgi server */
     if ((rv = proc_write_ipc(&bucket_ctx->ipc,
                              output_brigade)) != APR_SUCCESS) {
@@ -304,6 +320,7 @@ handle_request_ipc(request_rec *r, int role,
         bucket_ctx->has_error = 1;
         return HTTP_INTERNAL_SERVER_ERROR;
     }
+    apr_brigade_cleanup(output_brigade);

     /* Create brigade */
     brigade_stdout =
@@ -522,7 +539,8 @@ handle_request(request_rec * r, int role, fcgid_cm
 }

 static int add_request_body(request_rec *r, apr_pool_t *request_pool,
-                            apr_bucket_brigade *output_brigade)
+                            apr_bucket_brigade *output_brigade,
+                            fcgid_bucket_ctx *bucket_ctx)
 {
     apr_bucket *bucket_input, *bucket_header;
     apr_file_t *fd = NULL;
@@ -563,8 +581,6 @@ static int add_request_body(request_rec *r, apr_po
             return HTTP_INTERNAL_SERVER_ERROR;
         }

-
-
         while ((bucket_input = APR_BRIGADE_FIRST(input_brigade)) !=
APR_BRIGADE_SENTINEL(input_brigade)) {
             const char *data;
             apr_size_t len;
@@ -615,7 +631,9 @@ static int add_request_body(request_rec *r, apr_po
                 return HTTP_INTERNAL_SERVER_ERROR;
             }

-            if (request_size > sconf->max_mem_request_len) {
+            if (sconf->stream_request_body) {
+                bucket_stdin = bucket_input;
+            } else if (request_size > sconf->max_mem_request_len) {
                 apr_size_t wrote_len;
                 static const char *fd_key = "fcgid_fd";

@@ -701,6 +719,16 @@ static int add_request_body(request_rec *r, apr_po
             }
             APR_BRIGADE_INSERT_TAIL(output_brigade, bucket_header);
             APR_BRIGADE_INSERT_TAIL(output_brigade, bucket_stdin);
+
+            if (sconf->stream_request_body) {
+                /* Write output_brigade to fastcgi server */
+                if ((rv = proc_write_ipc(&bucket_ctx->ipc,
+                                         output_brigade)) != APR_SUCCESS) {
+                    bucket_ctx->has_error = 1;
+                    return HTTP_INTERNAL_SERVER_ERROR;
+                }
+                apr_brigade_cleanup(output_brigade);
+            }
         }

         apr_brigade_cleanup(input_brigade);
@@ -731,7 +759,6 @@ static int add_request_body(request_rec *r, apr_po
 int bridge_request(request_rec * r, int role, fcgid_cmd_conf *cmd_conf)
 {
     apr_bucket_brigade *output_brigade;
-    apr_bucket *bucket_eos;
     char **envp = ap_create_environment(r->pool,
                                         r->subprocess_env);
     int rc;
@@ -750,17 +777,6 @@ int bridge_request(request_rec * r, int role, fcgi
         return HTTP_INTERNAL_SERVER_ERROR;
     }

-    if (role == FCGI_RESPONDER) {
-        rc = add_request_body(r, r->pool, output_brigade);
-        if (rc) {
-            return rc;
-        }
-    }
-
-    /* The eos bucket now */
-    bucket_eos = apr_bucket_eos_create(r->connection->bucket_alloc);
-    APR_BRIGADE_INSERT_TAIL(output_brigade, bucket_eos);
-
     /* Bridge the request */
     return handle_request(r, role, cmd_conf, output_brigade);
 }
Index: modules/fcgid/fcgid_conf.c
===================================================================
--- modules/fcgid/fcgid_conf.c (revision 1553671)
+++ modules/fcgid/fcgid_conf.c (working copy)
@@ -57,6 +57,7 @@
  */
 #define DEFAULT_MAX_REQUEST_LEN (1024*128)
 #define DEFAULT_MAX_MEM_REQUEST_LEN (1024*64)
+#define DEFAULT_STREAM_REQUEST_BODY 0
 #define DEFAULT_WRAPPER_KEY "ALL"
 #define WRAPPER_FLAG_VIRTUAL "virtual"

@@ -97,6 +98,7 @@ void *create_fcgid_server_config(apr_pool_t * p, s
     config->ipc_connect_timeout = DEFAULT_IPC_CONNECT_TIMEOUT;
     config->max_mem_request_len = DEFAULT_MAX_MEM_REQUEST_LEN;
     config->max_request_len = DEFAULT_MAX_REQUEST_LEN;
+    config->stream_request_body = DEFAULT_STREAM_REQUEST_BODY;
     config->max_requests_per_process = DEFAULT_MAX_REQUESTS_PER_PROCESS;
     config->output_buffersize = DEFAULT_OUTPUT_BUFFERSIZE;
     config->max_class_process_count = DEFAULT_MAX_CLASS_PROCESS_COUNT;
@@ -158,6 +160,7 @@ void *merge_fcgid_server_config(apr_pool_t * p, vo
     MERGE_SCALAR(base, local, merged, ipc_connect_timeout);
     MERGE_SCALAR(base, local, merged, max_mem_request_len);
     MERGE_SCALAR(base, local, merged, max_request_len);
+    MERGE_SCALAR(base, local, merged, stream_request_body);
     MERGE_SCALAR(base, local, merged, max_requests_per_process);
     MERGE_SCALAR(base, local, merged, output_buffersize);
     MERGE_SCALAR(base, local, merged, max_class_process_count);
@@ -407,6 +410,17 @@ const char *set_max_mem_request_len(cmd_parms * cm
     return NULL;
 }

+const char *set_stream_request_body(cmd_parms * cmd, void *dummy, int arg)
+{
+    server_rec *s = cmd->server;
+    fcgid_server_conf *config =
+        ap_get_module_config(s->module_config, &fcgid_module);
+
+    config->stream_request_body = arg;
+    config->stream_request_body_set = 1;
+    return NULL;
+}
+
 const char *set_spawn_score(cmd_parms * cmd, void *dummy, const char *arg)
 {
     server_rec *s = cmd->server;
Index: modules/fcgid/fcgid_conf.h
===================================================================
--- modules/fcgid/fcgid_conf.h (revision 1553671)
+++ modules/fcgid/fcgid_conf.h (working copy)
@@ -91,6 +91,8 @@ typedef struct {
     int ipc_connect_timeout_set;
     int max_mem_request_len;
     int max_mem_request_len_set;
+    int stream_request_body;
+    int stream_request_body_set;
     apr_off_t max_request_len;
     int max_request_len_set;
     int max_requests_per_process;
@@ -259,6 +261,8 @@ const char *set_php_fix_pathinfo_enable(cmd_parms
 const char *set_max_requests_per_process(cmd_parms * cmd, void *dummy,
                                          const char *arg);

+const char *set_stream_request_body(cmd_parms * cmd, void *config, int arg);
+
 #ifdef WIN32
 const char *set_win32_prevent_process_orphans(cmd_parms *cmd, void *dummy,
                                               int arg);
Index: modules/fcgid/mod_fcgid.c
===================================================================
--- modules/fcgid/mod_fcgid.c (revision 1553671)
+++ modules/fcgid/mod_fcgid.c (working copy)
@@ -784,6 +784,10 @@ static const command_rec fcgid_cmds[] = {
     AP_INIT_TAKE1("FcgidMaxRequestsPerProcess", set_max_requests_per_process,
                   NULL, RSRC_CONF,
                   "Max requests handled by each fastcgi application"),
+    AP_INIT_FLAG("FcgidStreamRequestBody",
+                 set_stream_request_body, NULL,
+                 ACCESS_CONF | OR_FILEINFO,
+                 "Set to 'on' to allow request body to be streamed to
FastCGI app"),
     AP_INIT_TAKE1("FcgidOutputBufferSize", set_output_buffersize, NULL,
                   RSRC_CONF,
                   "CGI output buffer size"),

Reply via email to