From 4f7dfe580f1f7db2537eed4b1d1dbc059003de80 Mon Sep 17 00:00:00 2001 From: Brian Pane Date: Sat, 23 Mar 2002 23:19:41 +0000 Subject: [PATCH] Changed mod_cgi to not do single-byte reads to consume the script headers git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@94151 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 2 ++ STATUS | 4 ++- include/util_script.h | 17 +++++++++ modules/generators/mod_cgi.c | 67 ++++++++++++++++++------------------ server/util_script.c | 52 ++++++++++++++++++++++++++++ 5 files changed, 108 insertions(+), 34 deletions(-) diff --git a/CHANGES b/CHANGES index f045f9db9f..7251ef8aee 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,7 @@ Changes with Apache 2.0.34-dev + *) Remove the single-byte socket reads for CGI headers [Brian Pane] + *) When a proxied site was being served, Apache was replacing the original site Server header with it's own, which is not allowed by RFC2616. Fixed. [Graham Leggett] diff --git a/STATUS b/STATUS index 9f9bcd4285..57aa512970 100644 --- a/STATUS +++ b/STATUS @@ -1,5 +1,5 @@ APACHE 2.0 STATUS: -*-text-*- -Last modified at [$Date: 2002/03/22 19:35:47 $] +Last modified at [$Date: 2002/03/23 23:19:41 $] Release: @@ -188,6 +188,8 @@ RELEASE NON-SHOWSTOPPERS BUT WOULD BE REAL NICE TO WRAP THESE UP: to read from. Proposed solution in: Message-ID: <3C36ADAF.60601@cnet.com> + Update 3/23/2002: solution implemented for mod_cgi, mod_cgid remains + to be fixed * Try to get libtool inter-library dependency code working on AIX. Message-ID: diff --git a/include/util_script.h b/include/util_script.h index b1620c9b3b..8229f20cc1 100644 --- a/include/util_script.h +++ b/include/util_script.h @@ -59,6 +59,8 @@ #ifndef APACHE_UTIL_SCRIPT_H #define APACHE_UTIL_SCRIPT_H +#include "apr_buckets.h" + #ifdef __cplusplus extern "C" { #endif @@ -124,6 +126,21 @@ AP_DECLARE(void) ap_add_common_vars(request_rec *r); */ AP_DECLARE(int) ap_scan_script_header_err(request_rec *r, apr_file_t *f, char *buffer); +/** + * Read headers output from a script, ensuring that the output is valid. If + * the output is valid, then the headers are added to the headers out of the + * current request + * @param r The current request + * @param bb The brigade from which to read + * @param buffer Empty when calling the function. On output, if there was an + * error, the string that cause the error is stored here. + * @return HTTP_OK on success, HTTP_INTERNAL_SERVER_ERROR otherwise + * @deffunc int ap_scan_script_header_err_brigade(request_rec *r, apr_bucket_brigade *bb, char *buffer) + */ +AP_DECLARE(int) ap_scan_script_header_err_brigade(request_rec *r, + apr_bucket_brigade *bb, + char *buffer); + /** * Read headers strings from a script, ensuring that the output is valid. If * the output is valid, then the headers are added to the headers out of the diff --git a/modules/generators/mod_cgi.c b/modules/generators/mod_cgi.c index fb48e486cc..7336f0461f 100644 --- a/modules/generators/mod_cgi.c +++ b/modules/generators/mod_cgi.c @@ -261,7 +261,7 @@ static void log_script_err(request_rec *r, apr_file_t *script_err) } static int log_script(request_rec *r, cgi_server_conf * conf, int ret, - char *dbuf, const char *sbuf, apr_file_t *script_in, + char *dbuf, const char *sbuf, apr_bucket_brigade *bb, apr_file_t *script_err) { const apr_array_header_t *hdrs_arr = apr_table_elts(r->headers_in); @@ -280,9 +280,11 @@ static int log_script(request_rec *r, cgi_server_conf * conf, int ret, (apr_file_open(&f, conf->logname, APR_APPEND|APR_WRITE|APR_CREATE, APR_OS_DEFAULT, r->pool) != APR_SUCCESS)) { /* Soak up script output */ +#if LATER while (apr_file_gets(argsbuffer, HUGE_STRING_LEN, script_in) == APR_SUCCESS) continue; +#endif /* LATER */ log_script_err(r, script_err); return ret; @@ -319,6 +321,7 @@ static int log_script(request_rec *r, cgi_server_conf * conf, int ret, if (sbuf && *sbuf) apr_file_printf(f, "%s\n", sbuf); +#if LATER if (apr_file_gets(argsbuffer, HUGE_STRING_LEN, script_in) == APR_SUCCESS) { apr_file_puts("%stdout\n", f); apr_file_puts(argsbuffer, f); @@ -327,6 +330,7 @@ static int log_script(request_rec *r, cgi_server_conf * conf, int ret, apr_file_puts(argsbuffer, f); apr_file_puts("\n", f); } +#endif /* LATER */ if (apr_file_gets(argsbuffer, HUGE_STRING_LEN, script_err) == APR_SUCCESS) { apr_file_puts("%stderr\n", f); @@ -337,7 +341,7 @@ static int log_script(request_rec *r, cgi_server_conf * conf, int ret, apr_file_puts("\n", f); } - apr_file_close(script_in); + apr_brigade_destroy(bb); apr_file_close(script_err); apr_file_close(f); @@ -464,29 +468,12 @@ static apr_status_t run_cgi_child(apr_file_t **script_out, if (rc != APR_SUCCESS) { /* Bad things happened. Everyone should have cleaned up. */ -#if APR_HAS_PROC_INVOKED - if (procnew->invoked) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, rc, r, - "mod_cgi: failed to invoke process: %s", - procnew->invoked); - } - else -#endif ap_log_rerror(APLOG_MARK, APLOG_ERR, rc, r, - "mod_cgi: couldn't create child process: %s", - r->filename); + "couldn't create child process: %d: %s", rc, r->filename); } else { apr_pool_note_subprocess(p, procnew, APR_KILL_AFTER_TIMEOUT); -#if APR_HAS_PROC_INVOKED - if (procnew->invoked) { - ap_log_rerror(APLOG_MARK, APLOG_INFO, rc, r, - "mod_cgi invoked process: %s", - procnew->invoked); - } -#endif - *script_in = procnew->out; if (!*script_in) return APR_EBADF; @@ -559,6 +546,19 @@ 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; + APR_BRIGADE_FOREACH(e, bb) { + rv = apr_bucket_read(e, &buf, &len, APR_BLOCK_READ); + if (!APR_STATUS_IS_SUCCESS(rv)) { + break; + } + } +} static int cgi_handler(request_rec *r) { @@ -705,20 +705,23 @@ static int cgi_handler(request_rec *r) const char *location; char sbuf[MAX_STRING_LEN]; int ret; + conn_rec *c = r->connection; - if ((ret = ap_scan_script_header_err(r, script_in, sbuf))) { - return log_script(r, conf, ret, dbuf, sbuf, script_in, script_err); + bb = apr_brigade_create(r->pool); + b = apr_bucket_pipe_create(script_in); + APR_BRIGADE_INSERT_TAIL(bb, b); + b = apr_bucket_eos_create(); + APR_BRIGADE_INSERT_TAIL(bb, b); + + if ((ret = ap_scan_script_header_err_brigade(r, bb, sbuf))) { + return log_script(r, conf, ret, dbuf, sbuf, bb, script_err); } location = apr_table_get(r->headers_out, "Location"); if (location && location[0] == '/' && r->status == 200) { - - /* Soak up all the script output */ - while (apr_file_gets(argsbuffer, HUGE_STRING_LEN, - script_in) == APR_SUCCESS) { - continue; - } + discard_script_output(bb); + apr_brigade_destroy(bb); log_script_err(r, script_err); /* This redirect needs to be a GET no matter what the original * method was. @@ -739,15 +742,12 @@ static int cgi_handler(request_rec *r) /* XX 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; } if (!r->header_only) { - bb = apr_brigade_create(r->pool); - b = apr_bucket_pipe_create(script_in); - APR_BRIGADE_INSERT_TAIL(bb, b); - b = apr_bucket_eos_create(); - APR_BRIGADE_INSERT_TAIL(bb, b); ap_pass_brigade(r->output_filters, bb); } @@ -756,6 +756,7 @@ static int cgi_handler(request_rec *r) } if (script_in && nph) { + conn_rec *c = r->connection; struct ap_filter_t *cur; /* get rid of all filters up through protocol... since we diff --git a/server/util_script.c b/server/util_script.c index 351b08b5a2..7a790d577d 100644 --- a/server/util_script.c +++ b/server/util_script.c @@ -618,6 +618,58 @@ AP_DECLARE(int) ap_scan_script_header_err(request_rec *r, apr_file_t *f, return ap_scan_script_header_err_core(r, buffer, getsfunc_FILE, f); } +static int getsfunc_BRIGADE(char *buf, int len, void *arg) +{ + apr_bucket_brigade *bb = (apr_bucket_brigade *)arg; + const char *dst_end = buf + len - 1; /* leave room for terminating null */ + char *dst = buf; + apr_bucket *e = APR_BRIGADE_FIRST(bb); + apr_status_t rv; + int done = 0; + + while ((dst < dst_end) && !done && !APR_BUCKET_IS_EOS(e)) { + const char *bucket_data; + apr_size_t bucket_data_len; + const char *src; + const char *src_end; + apr_bucket * next; + + rv = apr_bucket_read(e, &bucket_data, &bucket_data_len, + APR_BLOCK_READ); + if (!APR_STATUS_IS_SUCCESS(rv)) { + return 0; + } + src = bucket_data; + src_end = bucket_data + bucket_data_len; + while ((src < src_end) && (dst < dst_end) && !done) { + if (*src == '\n') { + done = 1; + } + else { + *dst++ = *src; + } + src++; + } + + if (src < src_end) { + apr_bucket_split(e, src - bucket_data); + } + next = APR_BUCKET_NEXT(e); + APR_BUCKET_REMOVE(e); + apr_bucket_destroy(e); + e = next; + } + *dst = 0; + return 1; +} + +AP_DECLARE(int) ap_scan_script_header_err_brigade(request_rec *r, + apr_bucket_brigade *bb, + char *buffer) +{ + return ap_scan_script_header_err_core(r, buffer, getsfunc_BRIGADE, bb); +} + struct vastrs { va_list args; int arg; -- 2.40.0