From: florian <florian@openbsd.org>
Date: Fri, 1 Aug 2014 08:34:46 +0000 (+0000)
Subject: Correctly parse fcgi records if we don't get the whole record in one
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=19d2af9e72d96a567d0bd8e1a871625f8882309a;p=openbsd

Correctly parse fcgi records if we don't get the whole record in one
bufferevent_read().
Input/OK reyk@
---

diff --git a/usr.sbin/httpd/httpd.h b/usr.sbin/httpd/httpd.h
index d45783a6303..8ea75db4e95 100644
--- a/usr.sbin/httpd/httpd.h
+++ b/usr.sbin/httpd/httpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: httpd.h,v 1.26 2014/07/31 18:07:11 reyk Exp $	*/
+/*	$OpenBSD: httpd.h,v 1.27 2014/08/01 08:34:46 florian Exp $	*/
 
 /*
  * Copyright (c) 2006 - 2014 Reyk Floeter <reyk@openbsd.org>
@@ -237,6 +237,11 @@ struct privsep_proc {
 	struct httpd		*p_env;
 };
 
+enum fcgistate {
+	FCGI_READ_HEADER,
+	FCGI_READ_CONTENT
+};
+
 struct client {
 	u_int32_t		 clt_id;
 	pid_t			 clt_pid;
@@ -263,6 +268,10 @@ struct client {
 	int			 clt_done;
 	int			 clt_chunk;
 	int			 clt_inflight;
+	enum fcgistate		 clt_fcgi_state;
+	int			 clt_fcgi_toread;
+	int			 clt_fcgi_type;
+	struct evbuffer		*clt_srvevb;
 
 	struct evbuffer		*clt_log;
 	struct timeval		 clt_timeout;
diff --git a/usr.sbin/httpd/server.c b/usr.sbin/httpd/server.c
index 066596d2522..2e4cbaa0e07 100644
--- a/usr.sbin/httpd/server.c
+++ b/usr.sbin/httpd/server.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: server.c,v 1.18 2014/07/31 14:25:14 reyk Exp $	*/
+/*	$OpenBSD: server.c,v 1.19 2014/08/01 08:34:46 florian Exp $	*/
 
 /*
  * Copyright (c) 2006 - 2014 Reyk Floeter <reyk@openbsd.org>
@@ -624,6 +624,8 @@ server_close(struct client *clt, const char *msg)
 		bufferevent_free(clt->clt_bev);
 	if (clt->clt_output != NULL)
 		evbuffer_free(clt->clt_output);
+	if (clt->clt_srvevb != NULL)
+		evbuffer_free(clt->clt_srvevb);
 
 	if (clt->clt_srvbev != NULL)
 		bufferevent_free(clt->clt_srvbev);
diff --git a/usr.sbin/httpd/server_fcgi.c b/usr.sbin/httpd/server_fcgi.c
index 01187df061f..200666023c3 100644
--- a/usr.sbin/httpd/server_fcgi.c
+++ b/usr.sbin/httpd/server_fcgi.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: server_fcgi.c,v 1.5 2014/07/31 18:07:11 reyk Exp $	*/
+/*	$OpenBSD: server_fcgi.c,v 1.6 2014/08/01 08:34:46 florian Exp $	*/
 
 /*
  * Copyright (c) 2014 Florian Obser <florian@openbsd.org>
@@ -114,6 +114,18 @@ server_fcgi(struct httpd *env, struct client *clt)
 	if (connect(fd, (struct sockaddr *)&sun, sizeof(sun)) == -1)
 		goto fail;
 
+	clt->clt_fcgi_state = FCGI_READ_HEADER;
+	clt->clt_fcgi_toread = sizeof(struct fcgi_record_header);
+
+	if (clt->clt_srvevb != NULL)
+		evbuffer_free(clt->clt_srvevb);
+
+	clt->clt_srvevb = evbuffer_new();
+	if (clt->clt_srvevb == NULL) {
+		errstr = "failed to allocate evbuffer";
+		goto fail;
+	}
+
 	if (clt->clt_srvbev != NULL)
 		bufferevent_free(clt->clt_srvbev);
 
@@ -218,23 +230,45 @@ server_fcgi_read(struct bufferevent *bev, void *arg)
 	uint8_t	 buf[FCGI_RECORD_SIZE];
 	size_t	 len;
 
-	len = bufferevent_read(bev, &buf, FCGI_RECORD_SIZE);
-	DPRINTF("%s: %lu", __func__, len);
-	
-	h = (struct fcgi_record_header *) &buf;
-	DPRINTF("%s: record header: version %d type %d id %d content len %d",
-	    __func__, h->version, h->type, ntohs(h->id),
-	    ntohs(h->content_len));
-
-	if (h->type == FCGI_STDOUT && ntohs(h->content_len) > 0) {
-		DPRINTF("%s", (char *) &buf +
-                    sizeof(struct fcgi_record_header));
-
-		if (++clt->clt_chunk == 1)
-			server_fcgi_header(clt, 200);
-		server_bufferevent_write(clt, (char *)&buf +
-		    sizeof(struct fcgi_record_header),
-		    len - sizeof(struct fcgi_record_header));
+	len = bufferevent_read(bev, &buf, clt->clt_fcgi_toread);
+	/* XXX error handling */
+	evbuffer_add(clt->clt_srvevb, &buf, len);
+	clt->clt_fcgi_toread -= len;
+	DPRINTF("%s: len: %lu toread: %d state: %d", __func__, len,
+	    clt->clt_fcgi_toread, clt->clt_fcgi_state);
+
+	if (clt->clt_fcgi_toread != 0)
+		return;
+
+	switch (clt->clt_fcgi_state) {
+	case FCGI_READ_HEADER:
+		clt->clt_fcgi_state = FCGI_READ_CONTENT;
+		h = (struct fcgi_record_header *)
+		    EVBUFFER_DATA(clt->clt_srvevb);
+		DPRINTF("%s: record header: version %d type %d id %d "
+		    "content len %d", __func__, h->version, h->type,
+		    ntohs(h->id), ntohs(h->content_len));
+		clt->clt_fcgi_type = h->type;
+		clt->clt_fcgi_toread = ntohs(h->content_len);
+		evbuffer_drain(clt->clt_srvevb,
+		    EVBUFFER_LENGTH(clt->clt_srvevb));
+		if (clt->clt_fcgi_toread != 0)
+			break;
+
+		/* fallthrough if content_len == 0 */
+	case FCGI_READ_CONTENT:
+		if (clt->clt_fcgi_type == FCGI_STDOUT &&
+		    EVBUFFER_LENGTH(clt->clt_srvevb) > 0) {
+			if (++clt->clt_chunk == 1)
+				server_fcgi_header(clt, 200);
+			server_bufferevent_write_buffer(clt,
+			    clt->clt_srvevb);
+		}
+		evbuffer_drain(clt->clt_srvevb,
+		    EVBUFFER_LENGTH(clt->clt_srvevb));
+		clt->clt_fcgi_state = FCGI_READ_HEADER;
+		clt->clt_fcgi_toread =
+		    sizeof(struct fcgi_record_header);
 	}
 }