jorton 2004/06/16 01:40:00
Modified: . Tag: APACHE_2_0_BRANCH CHANGES STATUS
modules/generators Tag: APACHE_2_0_BRANCH mod_cgi.c
Log:
Backport mod_cgi "CGI bucket" changes:
* modules/generators/mod_cgi.c (log_script_err): Return a read error.
(cgi_bucket_create, cgi_bucket_dup, cgi_read_stdout, cgi_bucket_read):
New functions.
(cgi_handler): Use new CGI bucket rather than a pipe bucket if
APR_FILES_AS_SOCKETS; use zero read timeout from stdout/stderr during
script execution.
* modules/generators/mod_cgi.c (cgi_handler): Combine common code
between nph and non-nph handling; use a CGI bucket for both cases if
APR_FILES_AS_SOCKETS.
* modules/generators/mod_cgi.c (cgi_handler): Soak up stderr from nph-
scripts correctly.
PR: 18348, 22030
Submitted by: Joe Orton, Jeff Trawick
Reviewed by: Joe Orton, Jeff Trawick, Jean-Jacques Clar
Revision Changes Path
No revision
No revision
1.988.2.305 +5 -0 httpd-2.0/CHANGES
Index: CHANGES
===================================================================
RCS file: /home/cvs/httpd-2.0/CHANGES,v
retrieving revision 1.988.2.304
retrieving revision 1.988.2.305
diff -d -w -u -r1.988.2.304 -r1.988.2.305
--- CHANGES 14 Jun 2004 22:09:40 -0000 1.988.2.304
+++ CHANGES 16 Jun 2004 08:39:59 -0000 1.988.2.305
@@ -1,5 +1,10 @@
Changes with Apache 2.0.50
+ *) mod_cgi: Handle output on stderr during script execution on Unix
+ platforms; preventing deadlock when stderr output fills pipe buffer.
+ Also fixes case where stderr from nph- scripts could be lost.
+ PR 22030, 18348. [Joe Orton, Jeff Trawick]
+
*) mod_alias now emits a warning if it detects overlapping *Alias*
directives. [André Malo]
1.751.2.927 +1 -7 httpd-2.0/STATUS
Index: STATUS
===================================================================
RCS file: /home/cvs/httpd-2.0/STATUS,v
retrieving revision 1.751.2.926
retrieving revision 1.751.2.927
diff -d -w -u -r1.751.2.926 -r1.751.2.927
--- STATUS 15 Jun 2004 17:56:15 -0000 1.751.2.926
+++ STATUS 16 Jun 2004 08:39:59 -0000 1.751.2.927
@@ -106,12 +106,6 @@
build/rpm/httpd.spec.in: r1.5
+1: minfrin, nd
- *) mod_cgi: Handle stderr output during script execution
- http://cvs.apache.org/viewcvs.cgi/httpd-2.0/modules/generators/mod_cgi.c?r1=1.160&r2=1.163
- PR: 22030, 18348
- +1: jorton, trawick, jjclar
- +1: nd (concept)
-
*) Prevent Win32 pool corruption at startup
server/mpm/winnt/child.c: r1.36
+1: ake, trawick, nd, stoddard
No revision
No revision
1.148.2.8 +206 -32 httpd-2.0/modules/generators/mod_cgi.c
Index: mod_cgi.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/generators/mod_cgi.c,v
retrieving revision 1.148.2.7
retrieving revision 1.148.2.8
diff -d -w -u -r1.148.2.7 -r1.148.2.8
--- mod_cgi.c 9 Feb 2004 20:53:17 -0000 1.148.2.7
+++ mod_cgi.c 16 Jun 2004 08:40:00 -0000 1.148.2.8
@@ -32,6 +32,7 @@
#include "apr_optional.h"
#include "apr_buckets.h"
#include "apr_lib.h"
+#include "apr_poll.h"
#define APR_WANT_STRFUNC
#include "apr_want.h"
@@ -191,13 +192,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 +207,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,
@@ -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;
@@ -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) {
@@ -710,18 +881,28 @@
apr_file_flush(script_out);
apr_file_close(script_out);
- /* Handle script return... */
- if (script_in && !nph) {
- conn_rec *c = r->connection;
- const char *location;
- char sbuf[MAX_STRING_LEN];
- int ret;
+ 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 (!nph) {
+ const char *location;
+ char sbuf[MAX_STRING_LEN];
+ int ret;
+
if ((ret = ap_scan_script_header_err_brigade(r, bb, sbuf))) {
return log_script(r, conf, ret, dbuf, sbuf, bb, script_err);
}
@@ -731,6 +912,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 +939,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,13 +954,19 @@
}
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. */
}
|