[mod_cgi] consolidate CGI cleanup code

(more consistent behavior)

From: Glenn Strauss <gstrauss@gluelogic.com>

git-svn-id: svn://svn.lighttpd.net/lighttpd/branches/lighttpd-1.4.x@3090 152afb58-edef-0310-8abb-c4023f1b3aa9

2 files changed, 26 insertions, 109 deletions

diff --git a/NEWS b/NEWS
index a40c13ef..17cfb4d5 100644
--- a/NEWS
+++ b/NEWS
@@ -20,6 +20,7 @@ NEWS
   * [plugins] don't include dlfcn.h if not needed (fixes #2548)
   * [mod_fastcgi] 404 for X-Sendfile file not found (fixes #2474)
   * [mod_cgi] send 500 if CGI ends and there is no response (fixes #2542)
+  * [mod_cgi] consolidate CGI cleanup code
 
 - 1.4.39 - 2016-01-02
   * [core] fix memset_s call (fixes #2698)
diff --git a/src/mod_cgi.c b/src/mod_cgi.c
index ae5cf9e9..39d67326 100644
--- a/src/mod_cgi.c
+++ b/src/mod_cgi.c
@@ -522,15 +522,8 @@ static int cgi_demux_response(server *srv, handler_ctx *hctx) {
 static handler_t cgi_connection_close(server *srv, handler_ctx *hctx) {
 	int status;
 	pid_t pid;
-	plugin_data *p;
-	connection  *con;
-
-	if (NULL == hctx) return HANDLER_GO_ON;
-
-	p    = hctx->plugin_data;
-	con  = hctx->remote_conn;
-
-	if (con->mode != p->id) return HANDLER_GO_ON;
+	plugin_data *p = hctx->plugin_data;
+	connection *con = hctx->remote_conn;
 
 #ifndef __WIN32
 
@@ -631,8 +624,12 @@ static handler_t cgi_connection_close(server *srv, handler_ctx *hctx) {
 
 static handler_t cgi_connection_close_callback(server *srv, connection *con, void *p_d) {
 	plugin_data *p = p_d;
+	handler_ctx *hctx = con->plugin_ctx[p->id];
+
+	if (con->mode != p->id) return HANDLER_GO_ON;
+	if (NULL == hctx) return HANDLER_GO_ON;
 
-	return cgi_connection_close(srv, con->plugin_ctx[p->id]);
+	return cgi_connection_close(srv, hctx);
 }
 
 
@@ -642,12 +639,6 @@ static handler_t cgi_handle_fdevent(server *srv, void *ctx, int revents) {
 
 	joblist_append(srv, con);
 
-	if (hctx->fd == -1) {
-		log_error_write(srv, __FILE__, __LINE__, "ddss", con->fd, hctx->fd, connection_get_state(con->state), "invalid cgi-fd");
-
-		return HANDLER_ERROR;
-	}
-
 	if (revents & FDEVENT_IN) {
 		switch (cgi_demux_response(srv, hctx)) {
 		case FDEVENT_HANDLED_NOT_FINISHED:
@@ -663,17 +654,10 @@ static handler_t cgi_handle_fdevent(server *srv, void *ctx, int revents) {
 			/* if we get a IN|HUP and have read everything don't exec the close twice */
 			return HANDLER_FINISHED;
 		case FDEVENT_HANDLED_ERROR:
-			/* Send an error if we haven't sent any data yet */
-			if (0 == con->file_started) {
-				connection_set_state(srv, con, CON_STATE_HANDLE_REQUEST);
-				con->http_status = 500;
-				con->mode = DIRECT;
-			} else {
-				con->file_finished = 1;
-			}
-
 			log_error_write(srv, __FILE__, __LINE__, "s", "demuxer failed: ");
-			break;
+
+			cgi_connection_close(srv, hctx);
+			return HANDLER_FINISHED;
 		}
 	}
 
@@ -689,13 +673,6 @@ static handler_t cgi_handle_fdevent(server *srv, void *ctx, int revents) {
 			http_chunk_append_buffer(srv, con, hctx->response_header);
 		}
 
-		if (con->file_finished == 0) {
-			http_chunk_close(srv, con);
-		}
-		con->file_finished = 1;
-
-		joblist_append(srv, con);
-
 # if 0
 		log_error_write(srv, __FILE__, __LINE__, "sddd", "got HUP from cgi", con->fd, hctx->fd, revents);
 # endif
@@ -703,8 +680,6 @@ static handler_t cgi_handle_fdevent(server *srv, void *ctx, int revents) {
 		/* rtsigs didn't liked the close */
 		cgi_connection_close(srv, hctx);
 	} else if (revents & FDEVENT_ERR) {
-		con->file_finished = 1;
-
 		/* kill all connections to the cgi process */
 		cgi_connection_close(srv, hctx);
 #if 1
@@ -1212,18 +1187,7 @@ static int cgi_create_env(server *srv, connection *con, plugin_data *p, buffer *
 
 		if (-1 == fdevent_fcntl_set(srv->ev, hctx->fd)) {
 			log_error_write(srv, __FILE__, __LINE__, "ss", "fcntl failed: ", strerror(errno));
-
-			fdevent_event_del(srv->ev, &(hctx->fde_ndx), hctx->fd);
-			fdevent_unregister(srv->ev, hctx->fd);
-
-			log_error_write(srv, __FILE__, __LINE__, "sd", "cgi close:", hctx->fd);
-
-			close(hctx->fd);
-
-			cgi_handler_ctx_free(hctx);
-
-			con->plugin_ctx[p->id] = NULL;
-
+			cgi_connection_close(srv, hctx);
 			return -1;
 		}
 
@@ -1370,8 +1334,8 @@ TRIGGER_FUNC(cgi_trigger) {
 
 /*
  * - HANDLER_GO_ON : not our job
- * - HANDLER_FINISHED: got response header
- * - HANDLER_WAIT_FOR_EVENT: waiting for response header
+ * - HANDLER_FINISHED: got response
+ * - HANDLER_WAIT_FOR_EVENT: waiting for response
  */
 SUBREQUEST_FUNC(mod_cgi_handle_subrequest) {
 	int status;
@@ -1385,84 +1349,39 @@ SUBREQUEST_FUNC(mod_cgi_handle_subrequest) {
 	log_error_write(srv, __FILE__, __LINE__, "sdd", "subrequest, pid =", hctx, hctx->pid);
 #endif
 
-	if (hctx->pid == 0) {
-		/* cgi already dead */
-		if (!con->file_started) return HANDLER_WAIT_FOR_EVENT;
-		return HANDLER_FINISHED;
-	}
-
 #ifndef __WIN32
 	switch(waitpid(hctx->pid, &status, WNOHANG)) {
 	case 0:
-		/* we only have for events here if we don't have the header yet,
-		 * otherwise the event-handler will send us the incoming data */
-		if (con->file_started) return HANDLER_FINISHED;
-
 		return HANDLER_WAIT_FOR_EVENT;
+
 	case -1:
 		if (errno == EINTR) return HANDLER_WAIT_FOR_EVENT;
 
-		if (errno == ECHILD && con->file_started == 0) {
-			/*
-			 * second round but still not response
-			 */
-			return HANDLER_WAIT_FOR_EVENT;
-		}
-
-		log_error_write(srv, __FILE__, __LINE__, "ss", "waitpid failed: ", strerror(errno));
-		con->mode = DIRECT;
-		con->http_status = 500;
-
 		hctx->pid = 0;
-
-		fdevent_event_del(srv->ev, &(hctx->fde_ndx), hctx->fd);
-		fdevent_unregister(srv->ev, hctx->fd);
-
-		if (close(hctx->fd)) {
-			log_error_write(srv, __FILE__, __LINE__, "sds", "cgi close failed ", hctx->fd, strerror(errno));
+		if (errno != ECHILD) {
+			log_error_write(srv, __FILE__, __LINE__, "ss", "waitpid failed: ", strerror(errno));
 		}
+		break;
 
-		cgi_handler_ctx_free(hctx);
-
-		con->plugin_ctx[p->id] = NULL;
-
-		return HANDLER_FINISHED;
 	default:
 		/* cgi process exited
 		 */
 
 		hctx->pid = 0;
 
-		if (cgi_demux_response(srv, hctx) == FDEVENT_HANDLED_ERROR) {
-			log_error_write(srv, __FILE__, __LINE__, "s", "demuxer failed: ");
+		if (!WIFEXITED(status)) {
+			log_error_write(srv, __FILE__, __LINE__, "s", "cgi died ?");
 		}
 
-		/* we already have response headers? just continue */
-		if (con->file_started) return HANDLER_FINISHED;
-
-		if (WIFEXITED(status)) {
-			/* clean exit - just continue */
-			return HANDLER_WAIT_FOR_EVENT;
-		}
-
-		/* cgi proc died, and we didn't get any data yet - send error message and close cgi con */
-		log_error_write(srv, __FILE__, __LINE__, "s", "cgi died ?");
-
-		con->http_status = 500;
-		con->mode = DIRECT;
-
-		fdevent_event_del(srv->ev, &(hctx->fde_ndx), hctx->fd);
-		fdevent_unregister(srv->ev, hctx->fd);
-
-		if (close(hctx->fd)) {
-			log_error_write(srv, __FILE__, __LINE__, "sds", "cgi close failed ", hctx->fd, strerror(errno));
+		if (cgi_demux_response(srv, hctx) == FDEVENT_HANDLED_ERROR) {
+			log_error_write(srv, __FILE__, __LINE__, "s", "demuxer failed: ");
 		}
 
-		cgi_handler_ctx_free(hctx);
-
-		con->plugin_ctx[p->id] = NULL;
-		return HANDLER_FINISHED;
+		break;
 	}
+
+	cgi_connection_close(srv, hctx);
+	return HANDLER_FINISHED;
 #else
 	return HANDLER_ERROR;
 #endif
@@ -1477,9 +1396,6 @@ int mod_cgi_plugin_init(plugin *p) {
 	p->connection_reset = cgi_connection_close_callback;
 	p->handle_subrequest_start = cgi_is_handled;
 	p->handle_subrequest = mod_cgi_handle_subrequest;
-#if 0
-	p->handle_fdevent = cgi_handle_fdevent;
-#endif
 	p->handle_trigger = cgi_trigger;
 	p->init           = mod_cgi_init;
 	p->cleanup        = mod_cgi_free;