Changeset 2116

Timestamp:

03/02/2008 03:42:14 PM (6 months ago)

Author:

stbuehler

Message:

mod_cgi: add a event-handler for STDERR_FILENO and log it with ERROR()

• it was not a good idea to just close STDERR_FILENO; if the cgi
  opens a file and gets the filedescriptor 2 it will write its errors
  to this file too (imagine a cgi generating some images... )
  

Location:

trunk

Files:

• NEWS (modified) (1 diff)
• src/mod_cgi.c (modified) (15 diffs)

trunk/NEWS

@@ -20 +20 @@
   * fix sending source of cgi script instead of 500 error if fork fails
   * fix min-procs handling in mod_scgi.c, just set to max-procs (patch from #623)
+  * mod_cgi: add a event-handler for STDERR_FILENO and log it with ERROR()
 
 - 1.5.0-r19.. -

trunk/src/mod_cgi.c

@@ -78 +78 @@
 
         iosocket *sock;
+        iosocket *sock_err;
         iosocket *wb_sock;
 
         chunkqueue *rb;
+        chunkqueue *rb_err;
         chunkqueue *wb;
 
@@ -93 +95 @@
 
         sess->sock = iosocket_init();
+        sess->sock_err = iosocket_init();
         sess->wb_sock = iosocket_init();
         sess->wb = chunkqueue_init();
         sess->rb = chunkqueue_init();
+        sess->rb_err = chunkqueue_init();
 
         return sess;
@@ -104 +108 @@
 
         iosocket_free(sess->sock);
+        iosocket_free(sess->sock_err);
         iosocket_free(sess->wb_sock);
 
         chunkqueue_free(sess->wb);
         chunkqueue_free(sess->rb);
+        chunkqueue_free(sess->rb_err);
 
         free(sess);
@@ -402 +408 @@
         }
 
+        if (sess->sock_err->fd != -1) {
+                /* close connection to the cgi-script */
+                fdevent_event_del(srv->ev, sess->sock_err);
+                fdevent_unregister(srv->ev, sess->sock_err);
+        }
+
         if (sess->wb_sock->fd != -1) {
                 close(sess->wb_sock->fd);
@@ -547 +559 @@
 }
 
+/* so all cgi errors have the same source line as origin */
+static void cgi_log_err(const char *msg) {
+        ERROR("error from cgi: %s", msg);
+}
+
+static void cgi_copy_err(chunkqueue *cq) {
+        buffer *line = buffer_init();
+        chunk *c;
+
+        for (c = cq->first; c; c = c->next) {
+                off_t we_have;
+                char *str, *nl;
+
+                if (c->type != MEM_CHUNK) {
+                        ERROR("%s", "wrong chunk type");
+                        chunk_set_done(c);
+                        continue;
+                }
+
+                we_have = c->mem->used - 1 - c->offset;
+                str = c->mem->ptr + c->offset;
+                if (we_have <= 0) continue;
+
+                for ( ; NULL != (nl = strchr(str, '\n')); str = nl+1) {
+                        *nl = '\0';
+                        if (!buffer_is_empty(line)) {
+                                buffer_append_string(line, str);
+                                cgi_log_err(SAFE_BUF_STR(line));
+                                buffer_reset(line);
+                        } else {
+                                cgi_log_err(str);
+                        }
+                }
+                if (*str) {
+                        buffer_append_string(line, str);
+                }
+                chunk_set_done(c);
+        }
+
+        if (!buffer_is_empty(line)) {
+                cgi_log_err(SAFE_BUF_STR(line));
+        }
+        chunkqueue_remove_finished_chunks(cq);
+}
+
+static handler_t cgi_handle_err_fdevent(void *s, void *ctx, int revents) {
+        server      *srv  = (server *)s;
+        cgi_session *sess = ctx;
+        connection  *con  = sess->remote_con;
+
+        if (revents & FDEVENT_IN) {
+                switch (srv->network_backend_read(srv, con, sess->sock_err, sess->rb_err)) {
+                case NETWORK_STATUS_CONNECTION_CLOSE:
+                        fdevent_event_del(srv->ev, sess->sock_err);
+
+                        /* connection closed. close the read chunkqueue. */
+                        sess->rb_err->is_closed = 1;
+                        break;
+                case NETWORK_STATUS_SUCCESS:
+                        break;
+                default:
+                        ERROR("%s", "oops, we failed to read");
+                        break;
+                }
+
+                cgi_copy_err(sess->rb_err);
+        }
+
+        if (revents & FDEVENT_ERR) {
+                fdevent_event_del(srv->ev, sess->sock_err);
+        }
+
+        return HANDLER_FINISHED;
+}
+
 
 static int cgi_env_add(char_array *env, const char *key, size_t key_len, const char *val, size_t val_len) {
@@ -581 +668 @@
         int to_cgi_fds[2];
         int from_cgi_fds[2];
+        int from_cgi_err_fds[2];
         struct stat st;
 
@@ -601 +689 @@
 
         if (pipe(from_cgi_fds)) {
+                close(to_cgi_fds[0]); close(to_cgi_fds[1]);
+                log_error_write(srv, __FILE__, __LINE__, "ss", "pipe failed:", strerror(errno));
+                return -1;
+        }
+
+        if (pipe(from_cgi_err_fds)) {
+                close(to_cgi_fds[0]); close(to_cgi_fds[1]);
+                close(from_cgi_fds[0]); close(from_cgi_fds[1]);
                 log_error_write(srv, __FILE__, __LINE__, "ss", "pipe failed:", strerror(errno));
                 return -1;
@@ -626 +722 @@
                 close(from_cgi_fds[0]);
 
+                /* move stderr to from_cgi_err_fd[1] */
+                close(STDERR_FILENO);
+                dup2(from_cgi_err_fds[1], STDERR_FILENO);
+                close(from_cgi_err_fds[1]);
+                /* not needed */
+                close(from_cgi_err_fds[0]);
+
                 /* move the stdin to to_cgi_fd[0] */
                 close(STDIN_FILENO);
@@ -632 +735 @@
                 /* not needed */
                 close(to_cgi_fds[1]);
-
-                /**
-                 * FIXME: add a event-handler for STDERR_FILENO and let it LOG()
-                 */
-
-                close(STDERR_FILENO);
 
                 /* create environment */
@@ -860 +957 @@
                 /* error */
                 ERROR("fork() failed: %s", strerror(errno));
+                close(to_cgi_fds[0]); close(to_cgi_fds[1]);
+                close(from_cgi_fds[0]); close(from_cgi_fds[1]);
+                close(from_cgi_err_fds[0]); close(from_cgi_err_fds[1]);
                 return -1;
                 break;
@@ -867 +967 @@
 
                 close(from_cgi_fds[1]);
+                close(from_cgi_err_fds[1]);
                 close(to_cgi_fds[0]);
 
@@ -882 +983 @@
                 sess->sock->fd = from_cgi_fds[0];
                 sess->sock->type = IOSOCKET_TYPE_PIPE;
+                sess->sock_err->fd = from_cgi_err_fds[0];
+                sess->sock_err->type = IOSOCKET_TYPE_PIPE;
                 sess->wb_sock->fd = to_cgi_fds[1];
                 sess->wb_sock->type = IOSOCKET_TYPE_PIPE;
@@ -893 +996 @@
                 }
 
+                if (-1 == fdevent_fcntl_set(srv->ev, sess->sock_err)) {
+                        log_error_write(srv, __FILE__, __LINE__, "ss", "fcntl failed: ", strerror(errno));
+
+                        cgi_session_free(sess);
+
+                        return -1;
+                }
+
                 con->plugin_ctx[p->id] = sess;
 
                 fdevent_register(srv->ev, sess->sock, cgi_handle_fdevent, sess);
                 fdevent_event_add(srv->ev, sess->sock, FDEVENT_IN);
+
+                fdevent_register(srv->ev, sess->sock_err, cgi_handle_err_fdevent, sess);
+                fdevent_event_add(srv->ev, sess->sock_err, FDEVENT_IN);
 
                 sess->state = CGI_STATE_READ_RESPONSE_HEADER;
@@ -1090 +1204 @@
                 fdevent_unregister(srv->ev, sess->sock);
 
+                fdevent_event_del(srv->ev, sess->sock_err);
+                fdevent_unregister(srv->ev, sess->sock_err);
+
                 cgi_session_free(sess);
                 sess = NULL;
@@ -1113 +1230 @@
                 fdevent_event_del(srv->ev, sess->sock);
                 fdevent_unregister(srv->ev, sess->sock);
+
+                fdevent_event_del(srv->ev, sess->sock_err);
+                fdevent_unregister(srv->ev, sess->sock_err);
 
                 cgi_session_free(sess);