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"),