From: Joe Orton Date: Fri, 4 Oct 2019 09:20:33 +0000 (+0000) Subject: Move common (and near-identical) code for CGI response output handling X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=54c71d419a4a2e0063f5c6100eff55127ae054a9;p=apache Move common (and near-identical) code for CGI response output handling to cgi_common.h; the diff between the modules for this code was as follows: https://people.apache.org/~jorton/mod_cgi-to-cgid-handler.diff Change from previous: mod_cgi will now explicitly discard output when returning HTTP_MOVED_TEMPORARILY for relative redirects (should not be functionally different), TRACE1 logging of ap_pass_brigade failures for mod_cgid is dropped. * modules/generators/cgi_common.h (cgi_handle_response): New function, factored out from mod_cgid. (discard_script_output): Copied function from mod_cgi/d unchanged. * modules/generator/mod_cgid.c (cgid_handler), modules/generator/mod_cgi.c (cgi_handler): Use cgi_handle_response. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1867968 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/modules/generators/cgi_common.h b/modules/generators/cgi_common.h index c60f24289b..6b24950e89 100644 --- a/modules/generators/cgi_common.h +++ b/modules/generators/cgi_common.h @@ -27,6 +27,27 @@ #include "httpd.h" #include "util_filter.h" +static void discard_script_output(apr_bucket_brigade *bb) +{ + apr_bucket *e; + const char *buf; + apr_size_t len; + apr_status_t rv; + + for (e = APR_BRIGADE_FIRST(bb); + e != APR_BRIGADE_SENTINEL(bb); + e = APR_BUCKET_NEXT(e)) + { + if (APR_BUCKET_IS_EOS(e)) { + break; + } + rv = apr_bucket_read(e, &buf, &len, APR_BLOCK_READ); + if (rv != APR_SUCCESS) { + break; + } + } +} + /* A CGI bucket type is needed to catch any output to stderr from the * script; see PR 22030. */ static const apr_bucket_type_t bucket_type_cgi; @@ -218,3 +239,122 @@ static const apr_bucket_type_t bucket_type_cgi = { apr_bucket_copy_notimpl }; +/* Handle the CGI response output, having set up the brigade with the + * CGI or PIPE bucket as appropriate. */ +static int cgi_handle_response(request_rec *r, int nph, apr_bucket_brigade *bb, + apr_interval_time_t timeout, cgi_server_conf *conf, + char *logdata, apr_file_t *script_err) +{ + apr_status_t rv; + + /* Handle script return... */ + if (!nph) { + const char *location; + char sbuf[MAX_STRING_LEN]; + int ret; + + if ((ret = ap_scan_script_header_err_brigade_ex(r, bb, sbuf, + APLOG_MODULE_INDEX))) + { + ret = log_script(r, conf, ret, logdata, sbuf, bb, script_err); + + /* + * ret could be HTTP_NOT_MODIFIED in the case that the CGI script + * does not set an explicit status and ap_meets_conditions, which + * is called by ap_scan_script_header_err_brigade, detects that + * the conditions of the requests are met and the response is + * not modified. + * In this case set r->status and return OK in order to prevent + * running through the error processing stack as this would + * break with mod_cache, if the conditions had been set by + * mod_cache itself to validate a stale entity. + * BTW: We circumvent the error processing stack anyway if the + * CGI script set an explicit status code (whatever it is) and + * the only possible values for ret here are: + * + * HTTP_NOT_MODIFIED (set by ap_meets_conditions) + * HTTP_PRECONDITION_FAILED (set by ap_meets_conditions) + * HTTP_INTERNAL_SERVER_ERROR (if something went wrong during the + * processing of the response of the CGI script, e.g broken headers + * or a crashed CGI process). + */ + if (ret == HTTP_NOT_MODIFIED) { + r->status = ret; + return OK; + } + + return ret; + } + + location = apr_table_get(r->headers_out, "Location"); + + if (location && r->status == 200) { + /* For a redirect whether internal or not, discard any + * remaining stdout from the script, and log any remaining + * stderr output, as normal. */ + discard_script_output(bb); + apr_brigade_destroy(bb); + + if (script_err) { + apr_file_pipe_timeout_set(script_err, timeout); + log_script_err(r, script_err); + } + } + + if (location && location[0] == '/' && r->status == 200) { + /* This redirect needs to be a GET no matter what the original + * method was. + */ + r->method = "GET"; + r->method_number = M_GET; + + /* We already read the message body (if any), so don't allow + * the redirected request to think it has one. We can ignore + * Transfer-Encoding, since we used REQUEST_CHUNKED_ERROR. + */ + apr_table_unset(r->headers_in, "Content-Length"); + + ap_internal_redirect_handler(location, r); + return OK; + } + else if (location && r->status == 200) { + /* XXX: Note that if a script wants to produce its own Redirect + * body, it now has to explicitly *say* "Status: 302" + */ + discard_script_output(bb); + apr_brigade_destroy(bb); + return HTTP_MOVED_TEMPORARILY; + } + + rv = ap_pass_brigade(r->output_filters, bb); + } + else /* nph */ { + struct ap_filter_t *cur; + + /* get rid of all filters up through protocol... since we + * haven't parsed off the headers, there is no way they can + * work + */ + + cur = r->proto_output_filters; + while (cur && cur->frec->ftype < AP_FTYPE_CONNECTION) { + cur = cur->next; + } + r->output_filters = r->proto_output_filters = cur; + + rv = ap_pass_brigade(r->output_filters, bb); + } + + /* don't soak up script output if errors occurred writing it + * out... otherwise, we prolong the life of the script when the + * connection drops or we stopped sending output for some other + * reason */ + if (script_err && rv == APR_SUCCESS && !r->connection->aborted) { + apr_file_pipe_timeout_set(script_err, timeout); + log_script_err(r, script_err); + } + + if (script_err) apr_file_close(script_err); + + return OK; /* NOT r->status, even if it has changed. */ +} diff --git a/modules/generators/mod_cgi.c b/modules/generators/mod_cgi.c index f135912162..5708aa0be6 100644 --- a/modules/generators/mod_cgi.c +++ b/modules/generators/mod_cgi.c @@ -568,27 +568,6 @@ static apr_status_t default_build_command(const char **cmd, const char ***argv, return APR_SUCCESS; } -static void discard_script_output(apr_bucket_brigade *bb) -{ - apr_bucket *e; - const char *buf; - apr_size_t len; - apr_status_t rv; - - for (e = APR_BRIGADE_FIRST(bb); - e != APR_BRIGADE_SENTINEL(bb); - e = APR_BUCKET_NEXT(e)) - { - if (APR_BUCKET_IS_EOS(e)) { - break; - } - rv = apr_bucket_read(e, &buf, &len, APR_BLOCK_READ); - if (rv != APR_SUCCESS) { - break; - } - } -} - #if APR_FILES_AS_SOCKETS #include "cgi_common.h" #endif @@ -782,111 +761,7 @@ static int cgi_handler(request_rec *r) b = apr_bucket_eos_create(c->bucket_alloc); APR_BRIGADE_INSERT_TAIL(bb, b); - /* Handle script return... */ - if (!nph) { - const char *location; - char sbuf[MAX_STRING_LEN]; - int ret; - - if ((ret = ap_scan_script_header_err_brigade_ex(r, bb, sbuf, - APLOG_MODULE_INDEX))) - { - ret = log_script(r, conf, ret, dbuf, sbuf, bb, script_err); - - /* - * ret could be HTTP_NOT_MODIFIED in the case that the CGI script - * does not set an explicit status and ap_meets_conditions, which - * is called by ap_scan_script_header_err_brigade, detects that - * the conditions of the requests are met and the response is - * not modified. - * In this case set r->status and return OK in order to prevent - * running through the error processing stack as this would - * break with mod_cache, if the conditions had been set by - * mod_cache itself to validate a stale entity. - * BTW: We circumvent the error processing stack anyway if the - * CGI script set an explicit status code (whatever it is) and - * the only possible values for ret here are: - * - * HTTP_NOT_MODIFIED (set by ap_meets_conditions) - * HTTP_PRECONDITION_FAILED (set by ap_meets_conditions) - * HTTP_INTERNAL_SERVER_ERROR (if something went wrong during the - * processing of the response of the CGI script, e.g broken headers - * or a crashed CGI process). - */ - if (ret == HTTP_NOT_MODIFIED) { - r->status = ret; - return OK; - } - - return ret; - } - - location = apr_table_get(r->headers_out, "Location"); - - if (location && r->status == 200) { - /* For a redirect whether internal or not, discard any - * remaining stdout from the script, and log any remaining - * stderr output, as normal. */ - discard_script_output(bb); - apr_brigade_destroy(bb); - apr_file_pipe_timeout_set(script_err, timeout); - log_script_err(r, script_err); - } - - if (location && location[0] == '/' && r->status == 200) { - /* This redirect needs to be a GET no matter what the original - * method was. - */ - r->method = "GET"; - r->method_number = M_GET; - - /* We already read the message body (if any), so don't allow - * the redirected request to think it has one. We can ignore - * Transfer-Encoding, since we used REQUEST_CHUNKED_ERROR. - */ - apr_table_unset(r->headers_in, "Content-Length"); - - ap_internal_redirect_handler(location, r); - return OK; - } - else if (location && r->status == 200) { - /* XXX: Note that if a script wants to produce its own Redirect - * body, it now has to explicitly *say* "Status: 302" - */ - return HTTP_MOVED_TEMPORARILY; - } - - rv = ap_pass_brigade(r->output_filters, bb); - } - else /* nph */ { - struct ap_filter_t *cur; - - /* get rid of all filters up through protocol... since we - * haven't parsed off the headers, there is no way they can - * work - */ - - cur = r->proto_output_filters; - while (cur && cur->frec->ftype < AP_FTYPE_CONNECTION) { - cur = cur->next; - } - r->output_filters = r->proto_output_filters = cur; - - rv = ap_pass_brigade(r->output_filters, bb); - } - - /* don't soak up script output if errors occurred writing it - * out... otherwise, we prolong the life of the script when the - * connection drops or we stopped sending output for some other - * reason */ - if (rv == APR_SUCCESS && !r->connection->aborted) { - apr_file_pipe_timeout_set(script_err, timeout); - log_script_err(r, script_err); - } - - apr_file_close(script_err); - - return OK; /* NOT r->status, even if it has changed. */ + return cgi_handle_response(r, nph, bb, timeout, conf, dbuf, script_err); } /*============================================================================ diff --git a/modules/generators/mod_cgid.c b/modules/generators/mod_cgid.c index 4354499b4f..90cd529735 100644 --- a/modules/generators/mod_cgid.c +++ b/modules/generators/mod_cgid.c @@ -1351,6 +1351,7 @@ static int log_script(request_rec *r, cgid_server_conf * conf, int ret, #ifdef HAVE_CGID_FDPASSING /* Pull in CGI bucket implementation. */ +#define cgi_server_conf cgid_server_conf #include "cgi_common.h" #endif @@ -1420,27 +1421,6 @@ static int connect_to_daemon(int *sdptr, request_rec *r, return OK; } -static void discard_script_output(apr_bucket_brigade *bb) -{ - apr_bucket *e; - const char *buf; - apr_size_t len; - apr_status_t rv; - - for (e = APR_BRIGADE_FIRST(bb); - e != APR_BRIGADE_SENTINEL(bb); - e = APR_BUCKET_NEXT(e)) - { - if (APR_BUCKET_IS_EOS(e)) { - break; - } - rv = apr_bucket_read(e, &buf, &len, APR_BLOCK_READ); - if (rv != APR_SUCCESS) { - break; - } - } -} - /**************************************************************** * * Actual cgid handling... @@ -1779,117 +1759,7 @@ static int cgid_handler(request_rec *r) b = apr_bucket_eos_create(c->bucket_alloc); APR_BRIGADE_INSERT_TAIL(bb, b); - /* Handle script return... */ - if (!nph) { - const char *location; - char sbuf[MAX_STRING_LEN]; - int ret; - - if ((ret = ap_scan_script_header_err_brigade_ex(r, bb, sbuf, - APLOG_MODULE_INDEX))) - { - ret = log_script(r, conf, ret, dbuf, sbuf, bb, script_err); - - /* - * ret could be HTTP_NOT_MODIFIED in the case that the CGI script - * does not set an explicit status and ap_meets_conditions, which - * is called by ap_scan_script_header_err_brigade, detects that - * the conditions of the requests are met and the response is - * not modified. - * In this case set r->status and return OK in order to prevent - * running through the error processing stack as this would - * break with mod_cache, if the conditions had been set by - * mod_cache itself to validate a stale entity. - * BTW: We circumvent the error processing stack anyway if the - * CGI script set an explicit status code (whatever it is) and - * the only possible values for ret here are: - * - * HTTP_NOT_MODIFIED (set by ap_meets_conditions) - * HTTP_PRECONDITION_FAILED (set by ap_meets_conditions) - * HTTP_INTERNAL_SERVER_ERROR (if something went wrong during the - * processing of the response of the CGI script, e.g broken headers - * or a crashed CGI process). - */ - if (ret == HTTP_NOT_MODIFIED) { - r->status = ret; - return OK; - } - - return ret; - } - - location = apr_table_get(r->headers_out, "Location"); - - if (location && location[0] == '/' && r->status == 200) { - - /* Soak up all the script output */ - discard_script_output(bb); - apr_brigade_destroy(bb); - if (script_err) { - apr_file_pipe_timeout_set(script_err, timeout); - log_script_err(r, script_err); - } - - /* This redirect needs to be a GET no matter what the original - * method was. - */ - r->method = "GET"; - r->method_number = M_GET; - - /* We already read the message body (if any), so don't allow - * the redirected request to think it has one. We can ignore - * Transfer-Encoding, since we used REQUEST_CHUNKED_ERROR. - */ - apr_table_unset(r->headers_in, "Content-Length"); - - ap_internal_redirect_handler(location, r); - return OK; - } - else if (location && r->status == 200) { - /* XXX: Note that if a script wants to produce its own Redirect - * body, it now has to explicitly *say* "Status: 302" - */ - discard_script_output(bb); - apr_brigade_destroy(bb); - return HTTP_MOVED_TEMPORARILY; - } - - rv = ap_pass_brigade(r->output_filters, bb); - if (rv != APR_SUCCESS) { - ap_log_rerror(APLOG_MARK, APLOG_TRACE1, rv, r, - "Failed to flush CGI output to client"); - } - } - - if (nph) { - struct ap_filter_t *cur; - - /* get rid of all filters up through protocol... since we - * haven't parsed off the headers, there is no way they can - * work - */ - - cur = r->proto_output_filters; - while (cur && cur->frec->ftype < AP_FTYPE_CONNECTION) { - cur = cur->next; - } - r->output_filters = r->proto_output_filters = cur; - - rv = ap_pass_brigade(r->output_filters, bb); - } - - /* don't soak up script output if errors occurred writing it - * out... otherwise, we prolong the life of the script when the - * connection drops or we stopped sending output for some other - * reason */ - if (script_err && rv == APR_SUCCESS && !r->connection->aborted) { - apr_file_pipe_timeout_set(script_err, timeout); - log_script_err(r, script_err); - } - - if (script_err) apr_file_close(script_err); - - return OK; /* NOT r->status, even if it has changed. */ + return cgi_handle_response(r, nph, bb, timeout, conf, dbuf, script_err); }