[patch] Diff for mod_cgi.c

mod_cgi.c.diff (text/plain), 23.58 KB, created by Thierry Carrez (RETIRED) on 2004-05-07 06:14:26 UTC

===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/generators/mod_cgi.c,v
retrieving revision 1.148.2.7
retrieving revision 1.163
diff -u -r1.148.2.7 -r1.163
--- httpd-2.0/modules/generators/mod_cgi.c	2004/02/09 20:53:17	1.148.2.7
+++ httpd-2.0/modules/generators/mod_cgi.c	2004/05/05 15:30:53	1.163
@@ -32,8 +32,10 @@
 #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
@@ -191,13 +193,14 @@
 
 /* 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';
@@ -205,6 +208,8 @@
         ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, 
                       "%s", argsbuffer);            
     }
+
+    return rv;
 }
 
 static int log_script(request_rec *r, cgi_server_conf * conf, int ret,
@@ -270,7 +275,10 @@
         apr_file_printf(f, "%s\n", sbuf);
 
     first = 1;
-    APR_BRIGADE_FOREACH(e, bb) {
+    for (e = APR_BRIGADE_FIRST(bb);
+         e != APR_BRIGADE_SENTINEL(bb);
+         e = APR_BUCKET_NEXT(e))
+    {
         if (APR_BUCKET_IS_EOS(e)) {
             break;
         }
@@ -431,14 +439,6 @@
     }
     else {
         procnew = apr_pcalloc(p, sizeof(*procnew));
-        if (e_info->prog_type == RUN_AS_SSI) {
-            SPLIT_AND_PASS_PRETAG_BUCKETS(*(e_info->bb), e_info->ctx,
-                                          e_info->next, rc);
-            if (rc != APR_SUCCESS) {
-                return rc;
-            }
-        }
-
         rc = ap_os_create_privileged_process(r, procnew, command, argv, env,
                                              procattr, p);
     
@@ -446,7 +446,7 @@
             /* Bad things happened. Everyone should have cleaned up. */
             ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_TOCLIENT, rc, r,
                           "couldn't create child process: %d: %s", rc,
-                          apr_filename_of_pathname(r->filename));
+                          apr_filepath_name_get(r->filename));
         }
         else {
             apr_pool_note_subprocess(p, procnew, APR_KILL_AFTER_TIMEOUT);
@@ -528,7 +528,11 @@
     const char *buf;
     apr_size_t len;
     apr_status_t rv;
-    APR_BRIGADE_FOREACH(e, bb) {
+
+    for (e = APR_BRIGADE_FIRST(bb);
+         e != APR_BRIGADE_SENTINEL(bb);
+         e = APR_BUCKET_NEXT(e))
+    {
         if (APR_BUCKET_IS_EOS(e)) {
             break;
         }
@@ -539,6 +543,172 @@
     }
 }
 
+#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);
+    AP_DEBUG_ASSERT(rv == APR_SUCCESS);
+
+    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);
+    AP_DEBUG_ASSERT(rv == APR_SUCCESS);
+    
+    fd.desc.f = err; /* script's stderr */
+    fd.client_data = (void *)2;
+    rv = apr_pollset_add(data->pollset, &fd);
+    AP_DEBUG_ASSERT(rv == APR_SUCCESS);
+    
+    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)) {
+            return timeout == 0 ? APR_EAGAIN : rv;
+        }
+        else if (APR_STATUS_IS_EINTR(rv)) {
+            continue;
+        }
+        else if (rv != APR_SUCCESS) {
+            ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, data->r,
+                          "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,6 +726,7 @@
     cgi_server_conf *conf;
     apr_status_t rv;
     cgi_exec_info_t e_info;
+    conn_rec *c = r->connection;
 
     if(strcmp(r->handler, CGI_MAGIC_TYPE) && strcmp(r->handler, "cgi-script"))
         return DECLINED;
@@ -571,7 +742,7 @@
         return DECLINED;
     }
 
-    argv0 = apr_filename_of_pathname(r->filename);
+    argv0 = apr_filepath_name_get(r->filename);
     nph = !(strncmp(argv0, "nph-", 4));
     conf = ap_get_module_config(r->server->module_config, &cgi_module);
 
@@ -637,7 +808,7 @@
     /* 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) {
@@ -654,7 +825,10 @@
             return rv;
         }
 
-        APR_BRIGADE_FOREACH(bucket, bb) {
+        for (bucket = APR_BRIGADE_FIRST(bb);
+             bucket != APR_BRIGADE_SENTINEL(bb);
+             bucket = APR_BUCKET_NEXT(bucket))
+        {
             const char *data;
             apr_size_t len;
 
@@ -710,18 +884,28 @@
     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);
+#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(r, bb, sbuf))) {
             return log_script(r, conf, ret, dbuf, sbuf, bb, script_err);
         }
@@ -731,6 +915,7 @@
         if (location && location[0] == '/' && r->status == 200) {
             discard_script_output(bb);
             apr_brigade_destroy(bb);
+            apr_file_pipe_timeout_set(script_err, r->server->timeout);
             log_script_err(r, script_err);
             /* This redirect needs to be a GET no matter what the original
              * method was.
@@ -757,22 +942,8 @@
         }
 
         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
@@ -786,14 +957,20 @@
         }
         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. */
 }
 
@@ -803,92 +980,68 @@
  *   is the code required to handle the "exec" SSI directive.
  *============================================================================
  *============================================================================*/
-static int include_cgi(char *s, request_rec *r, ap_filter_t *next,
-                       apr_bucket *head_ptr, apr_bucket **inserted_head)
+static apr_status_t include_cgi(include_ctx_t *ctx, ap_filter_t *f,
+                                apr_bucket_brigade *bb, char *s)
 {
-    request_rec *rr = ap_sub_req_lookup_uri(s, r, next);
+    request_rec *r = f->r;
+    request_rec *rr = ap_sub_req_lookup_uri(s, r, f->next);
     int rr_status;
-    apr_bucket  *tmp_buck, *tmp2_buck;
 
     if (rr->status != HTTP_OK) {
         ap_destroy_sub_req(rr);
-        return -1;
+        return APR_EGENERAL;
     }
 
     /* No hardwired path info or query allowed */
-
     if ((rr->path_info && rr->path_info[0]) || rr->args) {
         ap_destroy_sub_req(rr);
-        return -1;
+        return APR_EGENERAL;
     }
     if (rr->finfo.filetype != APR_REG) {
         ap_destroy_sub_req(rr);
-        return -1;
+        return APR_EGENERAL;
     }
 
     /* Script gets parameters of the *document*, for back compatibility */
-
     rr->path_info = r->path_info;       /* hard to get right; see mod_cgi.c */
     rr->args = r->args;
 
     /* Force sub_req to be treated as a CGI request, even if ordinary
      * typing rules would have called it something else.
      */
-
     ap_set_content_type(rr, CGI_MAGIC_TYPE);
 
     /* Run it. */
-
     rr_status = ap_run_sub_req(rr);
     if (ap_is_HTTP_REDIRECT(rr_status)) {
-        apr_size_t len_loc;
         const char *location = apr_table_get(rr->headers_out, "Location");
-        conn_rec *c = r->connection;
 
-        location = ap_escape_html(rr->pool, location);
-        len_loc = strlen(location);
+        if (location) {
+            char *buffer;
 
-        /* XXX: if most of this stuff is going to get copied anyway,
-         * it'd be more efficient to pstrcat it into a single pool buffer
-         * and a single pool bucket */
-
-        tmp_buck = apr_bucket_immortal_create("<A HREF=\"",
-                                              sizeof("<A HREF=\"") - 1,
-                                              c->bucket_alloc);
-        APR_BUCKET_INSERT_BEFORE(head_ptr, tmp_buck);
-        tmp2_buck = apr_bucket_heap_create(location, len_loc, NULL,
-                                           c->bucket_alloc);
-        APR_BUCKET_INSERT_BEFORE(head_ptr, tmp2_buck);
-        tmp2_buck = apr_bucket_immortal_create("\">", sizeof("\">") - 1,
-                                               c->bucket_alloc);
-        APR_BUCKET_INSERT_BEFORE(head_ptr, tmp2_buck);
-        tmp2_buck = apr_bucket_heap_create(location, len_loc, NULL,
-                                           c->bucket_alloc);
-        APR_BUCKET_INSERT_BEFORE(head_ptr, tmp2_buck);
-        tmp2_buck = apr_bucket_immortal_create("</A>", sizeof("</A>") - 1,
-                                               c->bucket_alloc);
-        APR_BUCKET_INSERT_BEFORE(head_ptr, tmp2_buck);
-
-        if (*inserted_head == NULL) {
-            *inserted_head = tmp_buck;
+            location = ap_escape_html(rr->pool, location);
+            buffer = apr_pstrcat(ctx->pool, "<a href=\"", location, "\">",
+                                 location, "</a>", NULL);
+
+            APR_BRIGADE_INSERT_TAIL(bb, apr_bucket_pool_create(buffer,
+                                    strlen(buffer), ctx->pool,
+                                    f->c->bucket_alloc));
         }
     }
 
     ap_destroy_sub_req(rr);
 
-    return 0;
+    return APR_SUCCESS;
 }
 
-
-static int include_cmd(include_ctx_t *ctx, apr_bucket_brigade **bb,
-                       const char *command, request_rec *r, ap_filter_t *f)
+static apr_status_t include_cmd(include_ctx_t *ctx, ap_filter_t *f,
+                                apr_bucket_brigade *bb, const char *command)
 {
     cgi_exec_info_t  e_info;
-    const char   **argv;
-    apr_file_t    *script_out = NULL, *script_in = NULL, *script_err = NULL;
-    apr_bucket_brigade *bcgi;
-    apr_bucket *b;
+    const char **argv;
+    apr_file_t *script_out = NULL, *script_in = NULL, *script_err = NULL;
     apr_status_t rv;
+    request_rec *r = f->r;
 
     add_ssi_vars(r);
 
@@ -899,15 +1052,16 @@
     e_info.out_pipe    = APR_FULL_BLOCK;
     e_info.err_pipe    = APR_NO_PIPE;
     e_info.prog_type   = RUN_AS_SSI;
-    e_info.bb          = bb;
+    e_info.bb          = &bb;
     e_info.ctx         = ctx;
     e_info.next        = f->next;
 
-    if ((rv = cgi_build_command(&command, &argv, r, r->pool, &e_info)) != APR_SUCCESS) {
+    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", 
                       r->filename);
-        return HTTP_INTERNAL_SERVER_ERROR;
+        return rv;
     }
 
     /* run the script in its own process */
@@ -916,92 +1070,97 @@
                             &e_info)) != APR_SUCCESS) {
         ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
                       "couldn't spawn child process: %s", r->filename);
-        return HTTP_INTERNAL_SERVER_ERROR;
+        return rv;
     }
 
-    bcgi = apr_brigade_create(r->pool, f->c->bucket_alloc);
-    b = apr_bucket_pipe_create(script_in, f->c->bucket_alloc);
-    APR_BRIGADE_INSERT_TAIL(bcgi, b);
-    ap_pass_brigade(f->next, bcgi);
+    APR_BRIGADE_INSERT_TAIL(bb, apr_bucket_pipe_create(script_in,
+                            f->c->bucket_alloc));
+    ctx->flush_now = 1;
 
     /* We can't close the pipe here, because we may return before the
      * full CGI has been sent to the network.  That's okay though,
      * because we can rely on the pool to close the pipe for us.
      */
-
-    return 0;
+    return APR_SUCCESS;
 }
 
-static int handle_exec(include_ctx_t *ctx, apr_bucket_brigade **bb,
-                       request_rec *r, ap_filter_t *f, apr_bucket *head_ptr,
-                       apr_bucket **inserted_head)
+static apr_status_t handle_exec(include_ctx_t *ctx, ap_filter_t *f,
+                                apr_bucket_brigade *bb)
 {
-    char *tag     = NULL;
+    char *tag = NULL;
     char *tag_val = NULL;
+    request_rec *r = f->r;
     char *file = r->filename;
-    apr_bucket  *tmp_buck;
     char parsed_string[MAX_STRING_LEN];
 
-    *inserted_head = NULL;
-    if (ctx->flags & FLAG_PRINTING) {
-        if (ctx->flags & FLAG_NO_EXEC) {
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
-                          "exec used but not allowed in %s", r->filename);
-            CREATE_ERROR_BUCKET(ctx, tmp_buck, head_ptr, *inserted_head);
+    if (!ctx->argc) {
+        ap_log_rerror(APLOG_MARK,
+                      (ctx->flags & SSI_FLAG_PRINTING)
+                          ? APLOG_ERR : APLOG_WARNING,
+                      0, r, "missing argument for exec element in %s",
+                      r->filename);
+    }
+
+    if (!(ctx->flags & SSI_FLAG_PRINTING)) {
+        return APR_SUCCESS;
+    }
+
+    if (!ctx->argc) {
+        SSI_CREATE_ERROR_BUCKET(ctx, f, bb);
+        return APR_SUCCESS;
+    }
+
+    if (ctx->flags & SSI_FLAG_NO_EXEC) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "exec used but not allowed "
+                      "in %s", r->filename);
+        SSI_CREATE_ERROR_BUCKET(ctx, f, bb);
+        return APR_SUCCESS;
+    }
+
+    while (1) {
+        cgi_pfn_gtv(ctx, &tag, &tag_val, SSI_VALUE_DECODED);
+        if (!tag || !tag_val) {
+            break;
         }
-        else {
-            while (1) {
-                cgi_pfn_gtv(ctx, &tag, &tag_val, 1);
-                if (tag_val == NULL) {
-                    if (tag == NULL) {
-                        return 0;
-                    }
-                    else {
-                        return 1;
-                    }
-                }
-                if (!strcmp(tag, "cmd")) {
-                    cgi_pfn_ps(r, ctx, tag_val, parsed_string,
-                               sizeof(parsed_string), 1);
-                    if (include_cmd(ctx, bb, parsed_string, r, f) == -1) {
-                        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
-                                    "execution failure for parameter \"%s\" "
-                                    "to tag exec in file %s", tag, r->filename);
-                        CREATE_ERROR_BUCKET(ctx, tmp_buck, head_ptr,
-                                            *inserted_head);
-                    }
-                }
-                else if (!strcmp(tag, "cgi")) {
-                    apr_status_t retval = APR_SUCCESS;
 
-                    cgi_pfn_ps(r, ctx, tag_val, parsed_string,
-                               sizeof(parsed_string), 0);
+        if (!strcmp(tag, "cmd")) {
+            apr_status_t rv;
 
-                    SPLIT_AND_PASS_PRETAG_BUCKETS(*bb, ctx, f->next, retval);
-                    if (retval != APR_SUCCESS) {
-                        return retval;
-                    }
-
-                    if (include_cgi(parsed_string, r, f->next, head_ptr,
-                                    inserted_head) == -1) {
-                        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
-                                      "invalid CGI ref \"%s\" in %s",
-                                      tag_val, file);
-                        CREATE_ERROR_BUCKET(ctx, tmp_buck, head_ptr,
-                                            *inserted_head);
-                    }
-                }
-                else {
-                    ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
-                                  "unknown parameter \"%s\" to tag exec in %s",
-                                  tag, file);
-                    CREATE_ERROR_BUCKET(ctx, tmp_buck, head_ptr,
-                                        *inserted_head);
-                }
+            cgi_pfn_ps(ctx, tag_val, parsed_string, sizeof(parsed_string),
+                       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 "
+                              "for parameter \"%s\" to tag exec in file %s",
+                              tag, r->filename);
+                SSI_CREATE_ERROR_BUCKET(ctx, f, bb);
+                break;
+            }
+        }
+        else if (!strcmp(tag, "cgi")) {
+            apr_status_t rv;
+
+            cgi_pfn_ps(ctx, tag_val, parsed_string, sizeof(parsed_string),
+                       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 "
+                              "\"%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 "
+                          "\"%s\" to tag exec in %s", tag, file);
+            SSI_CREATE_ERROR_BUCKET(ctx, f, bb);
+            break;
+        }
     }
-    return 0;
+
+    return APR_SUCCESS;
 }