Move common (and near-identical) code for CGI response output handling
authorJoe Orton <jorton@apache.org>
Fri, 4 Oct 2019 09:20:33 +0000 (09:20 +0000)
committerJoe Orton <jorton@apache.org>
Fri, 4 Oct 2019 09:20:33 +0000 (09:20 +0000)
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

modules/generators/cgi_common.h
modules/generators/mod_cgi.c
modules/generators/mod_cgid.c

index c60f242..6b24950 100644 (file)
 #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. */
+}
index f135912..5708aa0 100644 (file)
@@ -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);
 }
 
 /*============================================================================
index 4354499..90cd529 100644 (file)
@@ -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);
 }