]> granicus.if.org Git - apache/blobdiff - modules/generators/mod_cgi.c
Add lots of unique tags to error log messages
[apache] / modules / generators / mod_cgi.c
index 2d3b5114cd1a1c8fef333838a94351de5177d139..385bb36495fbdb8d78c1874e0c9b4d754438ec58 100644 (file)
@@ -1,8 +1,9 @@
-/* Copyright 2000-2004 Apache Software Foundation
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
+/* Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
  *
  *     http://www.apache.org/licenses/LICENSE-2.0
  *
 
 /*
  * http_script: keeps all script-related ramblings together.
- * 
+ *
  * Compliant to CGI/1.1 spec
- * 
+ *
  * Adapted by rst from original NCSA code by Rob McCool
  *
  * Apache adds some new env vars; REDIRECT_URL and REDIRECT_QUERY_STRING for
  * custom error responses, and DOCUMENT_ROOT because we found it useful.
- * It also adds SERVER_ADMIN - useful for scripts to know who to mail when 
+ * It also adds SERVER_ADMIN - useful for scripts to know who to mail when
  * they fail.
  */
 
 #include "apr_optional.h"
 #include "apr_buckets.h"
 #include "apr_lib.h"
+#include "apr_poll.h"
 
 #define APR_WANT_STRFUNC
 #define APR_WANT_MEMFUNC
 #include "apr_want.h"
 
-#define CORE_PRIVATE
-
 #include "util_filter.h"
 #include "ap_config.h"
 #include "httpd.h"
 #include "mod_core.h"
 #include "mod_cgi.h"
 
+#if APR_HAVE_STRUCT_RLIMIT
+#if defined (RLIMIT_CPU) || defined (RLIMIT_NPROC) || defined (RLIMIT_DATA) || defined(RLIMIT_VMEM) || defined(RLIMIT_AS)
+#define AP_CGI_USE_RLIMIT
+#endif
+#endif
+
 module AP_MODULE_DECLARE_DATA cgi_module;
 
 static APR_OPTIONAL_FN_TYPE(ap_register_include_handler) *cgi_pfn_reg_with_ssi;
@@ -163,7 +169,7 @@ static int log_scripterror(request_rec *r, cgi_server_conf * conf, int ret,
     char time_str[APR_CTIME_LEN];
     int log_flags = rv ? APLOG_ERR : APLOG_ERR;
 
-    ap_log_rerror(APLOG_MARK, log_flags, rv, r, 
+    ap_log_rerror(APLOG_MARK, log_flags, rv, r,
                   "%s: %s", error, r->filename);
 
     /* XXX Very expensive mainline case! Open, then getfileinfo! */
@@ -190,26 +196,29 @@ static int log_scripterror(request_rec *r, cgi_server_conf * conf, int ret,
     return ret;
 }
 
-/* Soak up stderr from a script and redirect it to the error log. 
+/* Soak up stderr from a script and redirect it to the error log.
  */
-static void log_script_err(request_rec *r, apr_file_t *script_err)
+static apr_status_t log_script_err(request_rec *r, apr_file_t *script_err)
 {
     char argsbuffer[HUGE_STRING_LEN];
     char *newline;
+    apr_status_t rv;
 
-    while (apr_file_gets(argsbuffer, HUGE_STRING_LEN,
-                         script_err) == APR_SUCCESS) {
+    while ((rv = apr_file_gets(argsbuffer, HUGE_STRING_LEN,
+                               script_err)) == APR_SUCCESS) {
         newline = strchr(argsbuffer, '\n');
         if (newline) {
             *newline = '\0';
         }
-        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, 
-                      "%s", argsbuffer);            
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01215)
+                      "%s", argsbuffer);
     }
+
+    return rv;
 }
 
 static int log_script(request_rec *r, cgi_server_conf * conf, int ret,
-                      char *dbuf, const char *sbuf, apr_bucket_brigade *bb, 
+                      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);
@@ -348,16 +357,26 @@ static void cgi_child_errfn(apr_pool_t *pool, apr_status_t err,
     char errbuf[200];
 
     apr_file_open_stderr(&stderr_log, pool);
+    /* Escape the logged string because it may be something that
+     * came in over the network.
+     */
     apr_file_printf(stderr_log,
                     "(%d)%s: %s\n",
                     err,
                     apr_strerror(err, errbuf, sizeof(errbuf)),
-                    description);
+#ifndef AP_UNSAFE_ERROR_LOG_UNESCAPED
+                    ap_escape_logitem(pool,
+#endif
+                    description
+#ifndef AP_UNSAFE_ERROR_LOG_UNESCAPED
+                    )
+#endif
+                    );
 }
 
 static apr_status_t run_cgi_child(apr_file_t **script_out,
                                   apr_file_t **script_in,
-                                  apr_file_t **script_err, 
+                                  apr_file_t **script_err,
                                   const char *command,
                                   const char * const argv[],
                                   request_rec *r,
@@ -369,11 +388,8 @@ static apr_status_t run_cgi_child(apr_file_t **script_out,
     apr_proc_t *procnew;
     apr_status_t rc = APR_SUCCESS;
 
-#if defined(RLIMIT_CPU)  || defined(RLIMIT_NPROC) || \
-    defined(RLIMIT_DATA) || defined(RLIMIT_VMEM) || defined (RLIMIT_AS)
-
-    core_dir_config *conf = ap_get_module_config(r->per_dir_config,
-                                                 &core_module);
+#ifdef AP_CGI_USE_RLIMIT
+    core_dir_config *conf = ap_get_core_module_config(r->per_dir_config);
 #endif
 
 #ifdef DEBUG_CGI
@@ -398,6 +414,7 @@ static apr_status_t run_cgi_child(apr_file_t **script_out,
     fprintf(dbg, "Environment: \n");
     for (i = 0; env[i]; ++i)
         fprintf(dbg, "'%s'\n", env[i]);
+    fclose(dbg);
 #endif
 
     /* Transmute ourselves into the script.
@@ -408,18 +425,18 @@ static apr_status_t run_cgi_child(apr_file_t **script_out,
                                    e_info->in_pipe,
                                    e_info->out_pipe,
                                    e_info->err_pipe)) != APR_SUCCESS) ||
-        ((rc = apr_procattr_dir_set(procattr, 
+        ((rc = apr_procattr_dir_set(procattr,
                         ap_make_dirstr_parent(r->pool,
                                               r->filename))) != APR_SUCCESS) ||
-#ifdef RLIMIT_CPU
+#if defined(RLIMIT_CPU) && defined(AP_CGI_USE_RLIMIT)
         ((rc = apr_procattr_limit_set(procattr, APR_LIMIT_CPU,
                                       conf->limit_cpu)) != APR_SUCCESS) ||
 #endif
-#if defined(RLIMIT_DATA) || defined(RLIMIT_VMEM) || defined(RLIMIT_AS)
+#if defined(AP_CGI_USE_RLIMIT) && (defined(RLIMIT_DATA) || defined(RLIMIT_VMEM) || defined(RLIMIT_AS))
         ((rc = apr_procattr_limit_set(procattr, APR_LIMIT_MEM,
                                       conf->limit_mem)) != APR_SUCCESS) ||
 #endif
-#ifdef RLIMIT_NPROC
+#if RLIMIT_NPROC && defined(AP_CGI_USE_RLIMIT)
         ((rc = apr_procattr_limit_set(procattr, APR_LIMIT_NPROC,
                                       conf->limit_nproc)) != APR_SUCCESS) ||
 #endif
@@ -428,16 +445,18 @@ static apr_status_t run_cgi_child(apr_file_t **script_out,
 
         ((rc = apr_procattr_detach_set(procattr,
                                         e_info->detached)) != APR_SUCCESS) ||
+        ((rc = apr_procattr_addrspace_set(procattr,
+                                        e_info->addrspace)) != APR_SUCCESS) ||
         ((rc = apr_procattr_child_errfn_set(procattr, cgi_child_errfn)) != APR_SUCCESS)) {
         /* Something bad happened, tell the world. */
-        ap_log_rerror(APLOG_MARK, APLOG_ERR, rc, r,
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, rc, r, APLOGNO(01216)
                       "couldn't set child process attributes: %s", r->filename);
     }
     else {
         procnew = apr_pcalloc(p, sizeof(*procnew));
         rc = ap_os_create_privileged_process(r, procnew, command, argv, env,
                                              procattr, p);
-    
+
         if (rc != APR_SUCCESS) {
             /* Bad things happened. Everyone should have cleaned up. */
             ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_TOCLIENT, rc, r,
@@ -465,9 +484,6 @@ static apr_status_t run_cgi_child(apr_file_t **script_out,
             }
         }
     }
-#ifdef DEBUG_CGI
-    fclose(dbg);
-#endif
     return (rc);
 }
 
@@ -482,7 +498,7 @@ static apr_status_t default_build_command(const char **cmd, const char ***argv,
 
     if (e_info->process_cgi) {
         *cmd = r->filename;
-        /* Do not process r->args if they contain an '=' assignment 
+        /* Do not process r->args if they contain an '=' assignment
          */
         if (r->args && r->args[0] && !ap_strchr_c(r->args, '=')) {
             args = r->args;
@@ -500,9 +516,9 @@ static apr_status_t default_build_command(const char **cmd, const char ***argv,
             }
         }
     }
-    /* Everything is - 1 to account for the first parameter 
+    /* Everything is - 1 to account for the first parameter
      * which is the program name.
-     */ 
+     */
     if (numwords > APACHE_ARG_MAX - 1) {
         numwords = APACHE_ARG_MAX - 1;    /* Truncate args to prevent overrun */
     }
@@ -539,6 +555,192 @@ static void discard_script_output(apr_bucket_brigade *bb)
     }
 }
 
+#if APR_FILES_AS_SOCKETS
+
+/* 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;
+
+struct cgi_bucket_data {
+    apr_pollset_t *pollset;
+    request_rec *r;
+};
+
+/* Create a CGI bucket using pipes from script stdout 'out'
+ * and stderr 'err', for request 'r'. */
+static apr_bucket *cgi_bucket_create(request_rec *r,
+                                     apr_file_t *out, apr_file_t *err,
+                                     apr_bucket_alloc_t *list)
+{
+    apr_bucket *b = apr_bucket_alloc(sizeof(*b), list);
+    apr_status_t rv;
+    apr_pollfd_t fd;
+    struct cgi_bucket_data *data = apr_palloc(r->pool, sizeof *data);
+
+    APR_BUCKET_INIT(b);
+    b->free = apr_bucket_free;
+    b->list = list;
+    b->type = &bucket_type_cgi;
+    b->length = (apr_size_t)(-1);
+    b->start = -1;
+
+    /* Create the pollset */
+    rv = apr_pollset_create(&data->pollset, 2, r->pool, 0);
+    if (rv != APR_SUCCESS) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01217)
+                     "apr_pollset_create(); check system or user limits");
+        return NULL;
+    }
+
+    fd.desc_type = APR_POLL_FILE;
+    fd.reqevents = APR_POLLIN;
+    fd.p = r->pool;
+    fd.desc.f = out; /* script's stdout */
+    fd.client_data = (void *)1;
+    rv = apr_pollset_add(data->pollset, &fd);
+    if (rv != APR_SUCCESS) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01218)
+                     "apr_pollset_add(); check system or user limits");
+        return NULL;
+    }
+
+    fd.desc.f = err; /* script's stderr */
+    fd.client_data = (void *)2;
+    rv = apr_pollset_add(data->pollset, &fd);
+    if (rv != APR_SUCCESS) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01219)
+                     "apr_pollset_add(); check system or user limits");
+        return NULL;
+    }
+
+    data->r = r;
+    b->data = data;
+    return b;
+}
+
+/* Create a duplicate CGI bucket using given bucket data */
+static apr_bucket *cgi_bucket_dup(struct cgi_bucket_data *data,
+                                  apr_bucket_alloc_t *list)
+{
+    apr_bucket *b = apr_bucket_alloc(sizeof(*b), list);
+    APR_BUCKET_INIT(b);
+    b->free = apr_bucket_free;
+    b->list = list;
+    b->type = &bucket_type_cgi;
+    b->length = (apr_size_t)(-1);
+    b->start = -1;
+    b->data = data;
+    return b;
+}
+
+/* Handle stdout from CGI child.  Duplicate of logic from the _read
+ * method of the real APR pipe bucket implementation. */
+static apr_status_t cgi_read_stdout(apr_bucket *a, apr_file_t *out,
+                                    const char **str, apr_size_t *len)
+{
+    char *buf;
+    apr_status_t rv;
+
+    *str = NULL;
+    *len = APR_BUCKET_BUFF_SIZE;
+    buf = apr_bucket_alloc(*len, a->list); /* XXX: check for failure? */
+
+    rv = apr_file_read(out, buf, len);
+
+    if (rv != APR_SUCCESS && rv != APR_EOF) {
+        apr_bucket_free(buf);
+        return rv;
+    }
+
+    if (*len > 0) {
+        struct cgi_bucket_data *data = a->data;
+        apr_bucket_heap *h;
+
+        /* Change the current bucket to refer to what we read */
+        a = apr_bucket_heap_make(a, buf, *len, apr_bucket_free);
+        h = a->data;
+        h->alloc_len = APR_BUCKET_BUFF_SIZE; /* note the real buffer size */
+        *str = buf;
+        APR_BUCKET_INSERT_AFTER(a, cgi_bucket_dup(data, a->list));
+    }
+    else {
+        apr_bucket_free(buf);
+        a = apr_bucket_immortal_make(a, "", 0);
+        *str = a->data;
+    }
+    return rv;
+}
+
+/* Read method of CGI bucket: polls on stderr and stdout of the child,
+ * sending any stderr output immediately away to the error log. */
+static apr_status_t cgi_bucket_read(apr_bucket *b, const char **str,
+                                    apr_size_t *len, apr_read_type_e block)
+{
+    struct cgi_bucket_data *data = b->data;
+    apr_interval_time_t timeout;
+    apr_status_t rv;
+    int gotdata = 0;
+
+    timeout = block == APR_NONBLOCK_READ ? 0 : data->r->server->timeout;
+
+    do {
+        const apr_pollfd_t *results;
+        apr_int32_t num;
+
+        rv = apr_pollset_poll(data->pollset, timeout, &num, &results);
+        if (APR_STATUS_IS_TIMEUP(rv)) {
+            if (timeout) {
+                ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, data->r, APLOGNO(01220)
+                              "Timeout waiting for output from CGI script %s",
+                              data->r->filename);
+                return rv;
+            }
+            else {
+                return APR_EAGAIN;
+            }
+        }
+        else if (APR_STATUS_IS_EINTR(rv)) {
+            continue;
+        }
+        else if (rv != APR_SUCCESS) {
+            ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, data->r, APLOGNO(01221)
+                          "poll failed waiting for CGI child");
+            return rv;
+        }
+
+        for (; num; num--, results++) {
+            if (results[0].client_data == (void *)1) {
+                /* stdout */
+                rv = cgi_read_stdout(b, results[0].desc.f, str, len);
+                if (APR_STATUS_IS_EOF(rv)) {
+                    rv = APR_SUCCESS;
+                }
+                gotdata = 1;
+            } else {
+                /* stderr */
+                apr_status_t rv2 = log_script_err(data->r, results[0].desc.f);
+                if (APR_STATUS_IS_EOF(rv2)) {
+                    apr_pollset_remove(data->pollset, &results[0]);
+                }
+            }
+        }
+
+    } while (!gotdata);
+
+    return rv;
+}
+
+static const apr_bucket_type_t bucket_type_cgi = {
+    "CGI", 5, APR_BUCKET_DATA,
+    apr_bucket_destroy_noop,
+    cgi_bucket_read,
+    apr_bucket_setaside_notimpl,
+    apr_bucket_split_notimpl,
+    apr_bucket_copy_notimpl
+};
+
+#endif
+
 static int cgi_handler(request_rec *r)
 {
     int nph;
@@ -556,21 +758,18 @@ static int cgi_handler(request_rec *r)
     cgi_server_conf *conf;
     apr_status_t rv;
     cgi_exec_info_t e_info;
+    conn_rec *c;
 
-    if(strcmp(r->handler, CGI_MAGIC_TYPE) && strcmp(r->handler, "cgi-script"))
+    if (strcmp(r->handler, CGI_MAGIC_TYPE) && strcmp(r->handler, "cgi-script")) {
         return DECLINED;
+    }
+
+    c = r->connection;
 
     is_included = !strcmp(r->protocol, "INCLUDED");
 
     p = r->main ? r->main->pool : r->pool;
 
-    if (r->method_number == M_OPTIONS) {
-        /* 99 out of 100 CGI scripts, this is all they support */
-        r->allowed |= (AP_METHOD_BIT << M_GET);
-        r->allowed |= (AP_METHOD_BIT << M_POST);
-        return DECLINED;
-    }
-
     argv0 = apr_filepath_name_get(r->filename);
     nph = !(strncmp(argv0, "nph-", 4));
     conf = ap_get_module_config(r->server->module_config, &cgi_module);
@@ -582,7 +781,7 @@ static int cgi_handler(request_rec *r)
         return log_scripterror(r, conf, HTTP_FORBIDDEN, 0,
                                "attempt to include NPH CGI script");
 
-    if (r->finfo.filetype == 0)
+    if (r->finfo.filetype == APR_NOFILE)
         return log_scripterror(r, conf, HTTP_NOT_FOUND, 0,
                                "script not found or unable to stat");
     if (r->finfo.filetype == APR_DIR)
@@ -617,11 +816,12 @@ static int cgi_handler(request_rec *r)
     e_info.bb          = NULL;
     e_info.ctx         = NULL;
     e_info.next        = NULL;
+    e_info.addrspace   = 0;
 
     /* build the command line */
     if ((rv = cgi_build_command(&command, &argv, r, p, &e_info)) != APR_SUCCESS) {
-        ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
-                      "don't know how to spawn child process: %s", 
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01222)
+                      "don't know how to spawn child process: %s",
                       r->filename);
         return HTTP_INTERNAL_SERVER_ERROR;
     }
@@ -629,7 +829,7 @@ static int cgi_handler(request_rec *r)
     /* run the script in its own process */
     if ((rv = run_cgi_child(&script_out, &script_in, &script_err,
                             command, argv, r, p, &e_info)) != APR_SUCCESS) {
-        ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01223)
                       "couldn't spawn child process: %s", r->filename);
         return HTTP_INTERNAL_SERVER_ERROR;
     }
@@ -637,7 +837,7 @@ static int cgi_handler(request_rec *r)
     /* Transfer any put/post args, CERN style...
      * Note that we already ignore SIGPIPE in the core server.
      */
-    bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
+    bb = apr_brigade_create(r->pool, c->bucket_alloc);
     seen_eos = 0;
     child_stopped_reading = 0;
     if (conf->logname) {
@@ -649,9 +849,16 @@ static int cgi_handler(request_rec *r)
 
         rv = ap_get_brigade(r->input_filters, bb, AP_MODE_READBYTES,
                             APR_BLOCK_READ, HUGE_STRING_LEN);
-       
+
         if (rv != APR_SUCCESS) {
-            return rv;
+            if (APR_STATUS_IS_TIMEUP(rv)) {
+                ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01224)
+                              "Timeout during reading request entity data");
+                return HTTP_REQUEST_TIME_OUT;
+            }
+            ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01225)
+                          "Error reading request entity data");
+            return HTTP_INTERNAL_SERVER_ERROR;
         }
 
         for (bucket = APR_BRIGADE_FIRST(bb);
@@ -674,11 +881,11 @@ static int cgi_handler(request_rec *r)
             /* If the child stopped, we still must read to EOS. */
             if (child_stopped_reading) {
                 continue;
-            } 
+            }
 
             /* read */
             apr_bucket_read(bucket, &data, &len, APR_BLOCK_READ);
-            
+
             if (conf->logname && dbpos < conf->bufbytes) {
                 int cursize;
 
@@ -713,28 +920,76 @@ static int cgi_handler(request_rec *r)
     apr_file_flush(script_out);
     apr_file_close(script_out);
 
+    AP_DEBUG_ASSERT(script_in != NULL);
+
+    apr_brigade_cleanup(bb);
+
+#if APR_FILES_AS_SOCKETS
+    apr_file_pipe_timeout_set(script_in, 0);
+    apr_file_pipe_timeout_set(script_err, 0);
+
+    b = cgi_bucket_create(r, script_in, script_err, c->bucket_alloc);
+    if (b == NULL)
+        return HTTP_INTERNAL_SERVER_ERROR;
+#else
+    b = apr_bucket_pipe_create(script_in, c->bucket_alloc);
+#endif
+    APR_BRIGADE_INSERT_TAIL(bb, b);
+    b = apr_bucket_eos_create(c->bucket_alloc);
+    APR_BRIGADE_INSERT_TAIL(bb, b);
+
     /* Handle script return... */
-    if (script_in && !nph) {
-        conn_rec *c = r->connection;
+    if (!nph) {
         const char *location;
         char sbuf[MAX_STRING_LEN];
         int ret;
 
-        b = apr_bucket_pipe_create(script_in, c->bucket_alloc);
-        APR_BRIGADE_INSERT_TAIL(bb, b);
-        b = apr_bucket_eos_create(c->bucket_alloc);
-        APR_BRIGADE_INSERT_TAIL(bb, b);
+        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;
+            }
 
-        if ((ret = ap_scan_script_header_err_brigade(r, bb, sbuf))) {
-            return log_script(r, conf, ret, dbuf, sbuf, bb, script_err);
+            return ret;
         }
 
         location = apr_table_get(r->headers_out, "Location");
 
-        if (location && location[0] == '/' && r->status == 200) {
+        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, r->server->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.
              */
@@ -742,7 +997,7 @@ static int cgi_handler(request_rec *r)
             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 
+             * 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");
@@ -751,33 +1006,17 @@ static int cgi_handler(request_rec *r)
             return OK;
         }
         else if (location && r->status == 200) {
-            /* XX Note that if a script wants to produce its own Redirect
+            /* 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);
-
-        /* 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) {
-            log_script_err(r, script_err);
-        }
-
-        apr_file_close(script_err);
     }
-
-    if (script_in && nph) {
-        conn_rec *c = r->connection;
+    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
@@ -789,14 +1028,20 @@ static int cgi_handler(request_rec *r)
         }
         r->output_filters = r->proto_output_filters = cur;
 
-        bb = apr_brigade_create(r->pool, c->bucket_alloc);
-        b = apr_bucket_pipe_create(script_in, c->bucket_alloc);
-        APR_BRIGADE_INSERT_TAIL(bb, b);
-        b = apr_bucket_eos_create(c->bucket_alloc);
-        APR_BRIGADE_INSERT_TAIL(bb, b);
-        ap_pass_brigade(r->output_filters, bb);
+        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, r->server->timeout);
+        log_script_err(r, script_err);
     }
 
+    apr_file_close(script_err);
+
     return OK;                      /* NOT r->status, even if it has changed. */
 }
 
@@ -881,11 +1126,12 @@ static apr_status_t include_cmd(include_ctx_t *ctx, ap_filter_t *f,
     e_info.bb          = &bb;
     e_info.ctx         = ctx;
     e_info.next        = f->next;
+    e_info.addrspace   = 0;
 
     if ((rv = cgi_build_command(&command, &argv, r, r->pool,
                                 &e_info)) != APR_SUCCESS) {
-        ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
-                      "don't know how to spawn cmd child process: %s", 
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01226)
+                      "don't know how to spawn cmd child process: %s",
                       r->filename);
         return rv;
     }
@@ -894,7 +1140,7 @@ static apr_status_t include_cmd(include_ctx_t *ctx, ap_filter_t *f,
     if ((rv = run_cgi_child(&script_out, &script_in, &script_err,
                             command, argv, r, r->pool,
                             &e_info)) != APR_SUCCESS) {
-        ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01227)
                       "couldn't spawn child process: %s", r->filename);
         return rv;
     }
@@ -937,7 +1183,7 @@ static apr_status_t handle_exec(include_ctx_t *ctx, ap_filter_t *f,
     }
 
     if (ctx->flags & SSI_FLAG_NO_EXEC) {
-        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "exec used but not allowed "
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01228) "exec used but not allowed "
                       "in %s", r->filename);
         SSI_CREATE_ERROR_BUCKET(ctx, f, bb);
         return APR_SUCCESS;
@@ -956,8 +1202,8 @@ static apr_status_t handle_exec(include_ctx_t *ctx, ap_filter_t *f,
                        SSI_EXPAND_LEAVE_NAME);
 
             rv = include_cmd(ctx, f, bb, parsed_string);
-            if (!APR_STATUS_IS_SUCCESS(rv)) {
-                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "execution failure "
+            if (rv != APR_SUCCESS) {
+                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01229) "execution failure "
                               "for parameter \"%s\" to tag exec in file %s",
                               tag, r->filename);
                 SSI_CREATE_ERROR_BUCKET(ctx, f, bb);
@@ -971,15 +1217,15 @@ static apr_status_t handle_exec(include_ctx_t *ctx, ap_filter_t *f,
                        SSI_EXPAND_DROP_NAME);
 
             rv = include_cgi(ctx, f, bb, parsed_string);
-            if (!APR_STATUS_IS_SUCCESS(rv)) {
-                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "invalid CGI ref "
+            if (rv != APR_SUCCESS) {
+                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01230) "invalid CGI ref "
                               "\"%s\" in %s", tag_val, file);
                 SSI_CREATE_ERROR_BUCKET(ctx, f, bb);
                 break;
             }
         }
         else {
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "unknown parameter "
+            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01231) "unknown parameter "
                           "\"%s\" to tag exec in %s", tag, file);
             SSI_CREATE_ERROR_BUCKET(ctx, f, bb);
             break;
@@ -1028,7 +1274,7 @@ static void register_hooks(apr_pool_t *p)
     ap_hook_post_config(cgi_post_config, aszPre, NULL, APR_HOOK_REALLY_FIRST);
 }
 
-module AP_MODULE_DECLARE_DATA cgi_module =
+AP_DECLARE_MODULE(cgi) =
 {
     STANDARD20_MODULE_STUFF,
     NULL,                        /* dir config creater */