[PATCH 0/3] httpd: debloating and refining

[PATCH 0/3] httpd: debloating and refining

[Eric Wong]

Too much of the initial design was based on dedicated,
high-performance HTTP servers, which mwrap-httpd isn't.
So try to save some icache and binary space by trimming
unneeded, overkill features.

Eric Wong (3):
  httpd: drop connection if 404 on POST bodies
  httpd: drop persistent connection support
  httpd: do not waste TSD space

 httpd.h   | 78 +++++++++++++++++++++++--------------------------------
 t/httpd.t |  4 +++
 2 files changed, 36 insertions(+), 46 deletions(-)

[PATCH 1/3] httpd: drop connection if 404 on POST bodies

[Eric Wong]

Persistent connections and undrained input don't mix; so the
simplest thing is to drop the persistent connection when we
short-circuit out on /$PID/ prefix mismatches.

In retrospect, it's probably not worth supporting persistent
connections at all for a AF_UNIX-only server...
---
 httpd.h   | 1 +
 t/httpd.t | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/httpd.h b/httpd.h
index eddea97..a59c9dd 100644
--- a/httpd.h
+++ b/httpd.h
@@ -310,6 +310,7 @@ static enum mw_qev h1_404(struct mw_h1 *h1)
 	static const char r404[] = "HTTP/1.1 404 Not Found\r\n"
 		"Content-Type: text/html\r\n"
 		"Content-Length: 10\r\n\r\n" "Not Found\n";
+	if (h1->has_input) h1->persist = 0;
 	return h1_res_oneshot(h1, r404, sizeof(r404) - 1);
 }
 
diff --git a/t/httpd.t b/t/httpd.t
index 3fe9c1f..53cf420 100644
--- a/t/httpd.t
+++ b/t/httpd.t
@@ -161,6 +161,10 @@ SKIP: {
 		'-HX-Mwrap-BT-Depth:10', '-XPOST', "http://0/$pid/ctl");
 	is($rc, 0, 'curl /ctl (X-Mwrap-BT-Depth)');
 	like(slurp($cout), qr/\bMWRAP=bt:10\b/, 'changed bt depth');
+
+	$rc = system(qw(curl -vsSf --unix-socket), $sock, '-o', $cout,
+		'-HX-Mwrap-BT-Depth:10', '-d', 'blah', "http://0/ctl");
+	is($rc >> 8, 22, '404 w/o PID prefix');
 };
 
 

[PATCH 2/3] httpd: drop persistent connection support

[Eric Wong]

It's not worth the extra complexity, binary size, and icache
bloat for a AF_UNIX-only server.
---
 httpd.h | 33 +++++++--------------------------
 1 file changed, 7 insertions(+), 26 deletions(-)

diff --git a/httpd.h b/httpd.h
index a59c9dd..128836c 100644
--- a/httpd.h
+++ b/httpd.h
@@ -78,9 +78,8 @@ struct mw_h1 { /* each HTTP/1.x client (heap) */
 	int fd;
 	short events; /* for poll */
 	unsigned prev_len:13; /* capped by MW_RBUF_SIZE */
-	unsigned persist:1; /* HTTP/1.1 */
 	unsigned has_input:1;
-	unsigned unused_:1;
+	unsigned unused_:2;
 	struct mw_h1req *h1r; /* only for slow clients */
 	unsigned long in_len;
 	struct mw_wbuf *wbuf;
@@ -211,9 +210,7 @@ static enum mw_qev h1_send_flush(struct mw_h1 *h1)
 			}
 		}
 	} while (mh.msg_iovlen);
-	free(wbuf);
-	h1->wbuf = NULL;
-	return h1->persist ? MW_QEV_RD : h1_close(h1);
+	return h1_close(h1);
 }
 
 static FILE *fbuf_init(struct mw_fbuf *fb)
@@ -285,6 +282,7 @@ static enum mw_qev h1_200(struct mw_h1 *h1, struct mw_fbuf *fb, const char *ct)
 	clen -= sizeof(struct mw_wbuf);
 	mwrap_assert(clen >= 0);
 	FPUTS("HTTP/1.1 200 OK\r\n"
+		"Connection: close\r\n"
 		"Expires: Fri, 01 Jan 1980 00:00:00 GMT\r\n"
 		"Pragma: no-cache\r\n"
 		"Cache-Control: no-cache, max-age=0, must-revalidate\r\n"
@@ -309,8 +307,8 @@ static enum mw_qev h1_404(struct mw_h1 *h1)
 {
 	static const char r404[] = "HTTP/1.1 404 Not Found\r\n"
 		"Content-Type: text/html\r\n"
+		"Connection: close\r\n"
 		"Content-Length: 10\r\n\r\n" "Not Found\n";
-	if (h1->has_input) h1->persist = 0;
 	return h1_res_oneshot(h1, r404, sizeof(r404) - 1);
 }
 
@@ -319,16 +317,12 @@ static int name_eq(const struct phr_header *h, const char *name, size_t len)
 {
 	return h->name_len == len && !strncasecmp(name, h->name, len);
 }
-#define VAL_EQ(h, VAL) val_eq(h, VAL, sizeof(VAL)-1)
-static int val_eq(const struct phr_header *h, const char *val, size_t len)
-{
-	return h->value_len == len && !strncasecmp(val, h->value, len);
-}
 
 static enum mw_qev h1_do_reset(struct mw_h1 *h1)
 {
 	static const char r200[] = "HTTP/1.1 200 OK\r\n"
 		"Content-Type: text/plain\r\n"
+		"Connection: close\r\n"
 		"Content-Length: 6\r\n\r\n" "reset\n";
 	mwrap_reset();
 	return h1_res_oneshot(h1, r200, sizeof(r200) - 1);
@@ -338,6 +332,7 @@ static enum mw_qev h1_do_trim(struct mw_h1 *h1)
 {
 	static const char r200[] = "HTTP/1.1 200 OK\r\n"
 		"Content-Type: text/plain\r\n"
+		"Connection: close\r\n"
 		"Content-Length: 9\r\n\r\n" "trimming\n";
 	malloc_trim(0);
 	return h1_res_oneshot(h1, r200, sizeof(r200) - 1);
@@ -867,15 +862,13 @@ static void ctl_set(struct mw_h1 *h1, long n)
 
 static enum mw_qev h1_parse_harder(struct mw_h1 *h1, struct mw_h1req *h1r)
 {
-	enum { HDR_IGN, HDR_CONN, HDR_XENC, HDR_CLEN } cur = HDR_IGN;
-	bool conn_set = false;
+	enum { HDR_IGN, HDR_XENC, HDR_CLEN } cur = HDR_IGN;
 	char *end;
 	struct phr_header *hdr = h1r->hdr;
 	long depth = -1;
 
 	h1->prev_len = 0;
 	h1->has_input = 0;
-	h1->persist = h1r->minor_ver >= 1 ? 1 : 0;
 	h1->in_len = 0;
 
 	for (hdr = h1r->hdr; h1r->nr_hdr--; hdr++) {
@@ -883,8 +876,6 @@ static enum mw_qev h1_parse_harder(struct mw_h1 *h1, struct mw_h1req *h1r)
 			cur = HDR_XENC;
 		else if (NAME_EQ(hdr, "Content-Length"))
 			cur = HDR_CLEN;
-		else if (NAME_EQ(hdr, "Connection"))
-			cur = HDR_CONN;
 		else if (NAME_EQ(hdr, "Trailer"))
 			return h1_400(h1);
 		else if (hdr->name) {
@@ -906,16 +897,6 @@ static enum mw_qev h1_parse_harder(struct mw_h1 *h1, struct mw_h1req *h1r)
 		if (!hdr->value_len)
 			continue;
 		switch (cur) {
-		case HDR_CONN:
-			if (conn_set) return h1_400(h1);
-			conn_set = true;
-			if (VAL_EQ(hdr, "close"))
-				h1->persist = 0;
-			else if (VAL_EQ(hdr, "keep-alive"))
-				h1->persist = 1;
-			else
-				return h1_400(h1);
-			break;
 		case HDR_XENC:
 			return h1_400(h1);
 		case HDR_CLEN:

[PATCH 3/3] httpd: do not waste TSD space

[Eric Wong]

mwrap-httpd is inherently single-threaded, and the TSD pointer
needs to be alotted to every thread.  So tie the request pointer
to the daemon itself instead of per-thread space.

If this were a dedicated multi-threaded httpd, I would be using
TSD for this.  But this is only a single-threaded embedded httpd
with many threads doing completely unrelated things.
---
 httpd.h | 46 +++++++++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/httpd.h b/httpd.h
index 128836c..e528d2b 100644
--- a/httpd.h
+++ b/httpd.h
@@ -94,6 +94,7 @@ struct mw_h1d { /* the daemon + listener, a singleton */
 	/* use open_memstream + fwrite to implement a growing pollfd array */
 	struct mw_fbuf pb; /* pollfd vector */
 	pthread_t tid;
+	struct mw_h1req *shared_h1r; /* shared by all fast clients */
 	size_t pid_len;
 	char pid_str[10];
 };
@@ -104,7 +105,6 @@ union mw_sockaddr { /* cast-avoiding convenience :> */
 };
 
 static struct mw_h1d g_h1d = { .lfd = -1 };
-static MWRAP_TSD struct mw_h1req *tsd_h1r;
 
 /* sortable snapshot version of struct src_loc */
 struct h1_src_loc {
@@ -787,12 +787,22 @@ static enum mw_qev h1_dispatch(struct mw_h1 *h1, struct mw_h1req *h1r)
 	return h1_404(h1);
 }
 
+static void
+prep_trickle(struct mw_h1 *h1, struct mw_h1req *h1r, struct mw_h1d *h1d)
+{
+	if (h1->h1r) return; /* already trickling */
+	h1->h1r = h1r;
+	mwrap_assert(h1d->shared_h1r == h1r);
+	h1d->shared_h1r = NULL;
+}
+
 /*
  * nothing in the PSGI app actually reads input, but clients tend
  * to send something in the body of POST requests anyways, so we
  * just drain it
  */
-static enum mw_qev h1_drain_input(struct mw_h1 *h1, struct mw_h1req *h1r)
+static enum mw_qev h1_drain_input(struct mw_h1 *h1, struct mw_h1req *h1r,
+	struct mw_h1d *h1d)
 {
 	if (h1r) { /* initial */
 		ssize_t overread = h1r->rbuf_len - h1r->pret;
@@ -822,11 +832,7 @@ static enum mw_qev h1_drain_input(struct mw_h1 *h1, struct mw_h1req *h1r)
 		} else {
 			switch (errno) {
 			case EAGAIN:
-				if (!h1->h1r) {
-					h1->h1r = h1r;
-					mwrap_assert(tsd_h1r == h1r);
-					tsd_h1r = NULL;
-				}
+				prep_trickle(h1, h1r, h1d);
 				return MW_QEV_RD;
 			case ECONNRESET: /* common */
 			case ENOTCONN:
@@ -860,7 +866,8 @@ static void ctl_set(struct mw_h1 *h1, long n)
 	}
 }
 
-static enum mw_qev h1_parse_harder(struct mw_h1 *h1, struct mw_h1req *h1r)
+static enum mw_qev h1_parse_harder(struct mw_h1 *h1, struct mw_h1req *h1r,
+	struct mw_h1d *h1d)
 {
 	enum { HDR_IGN, HDR_XENC, HDR_CLEN } cur = HDR_IGN;
 	char *end;
@@ -940,10 +947,10 @@ static enum mw_qev h1_parse_harder(struct mw_h1 *h1, struct mw_h1req *h1r)
 		h1r->qlen = h1r->path + h1r->path_len - h1r->qstr;
 		h1r->path_len -= (h1r->qlen + 1);
 	}
-	return h1_drain_input(h1, h1r);
+	return h1_drain_input(h1, h1r, h1d);
 }
 
-static enum mw_qev h1_event_step(struct mw_h1 *h1)
+static enum mw_qev h1_event_step(struct mw_h1 *h1, struct mw_h1d *h1d)
 {
 	struct mw_h1req *h1r;
 
@@ -956,14 +963,14 @@ static enum mw_qev h1_event_step(struct mw_h1 *h1)
 		return h1_send_flush(h1);
 
 	if (h1->has_input)
-		return h1_drain_input(h1, NULL);
+		return h1_drain_input(h1, NULL, h1d);
 	/*
-	 * The majority of requests can be served using TSD rbuf,
+	 * The majority of requests can be served using per-daemon rbuf,
 	 * no need for per-client allocations unless a client trickles
 	 */
-	h1r = h1->h1r ? h1->h1r : tsd_h1r;
+	h1r = h1->h1r ? h1->h1r : h1d->shared_h1r;
 	if (!h1r) {
-		h1r = tsd_h1r = malloc(sizeof(*h1r));
+		h1r = h1d->shared_h1r = malloc(sizeof(*h1r));
 		if (!h1r) {
 			fprintf(stderr, "h1r malloc: %m\n");
 			return h1_close(h1);
@@ -982,7 +989,7 @@ static enum mw_qev h1_event_step(struct mw_h1 *h1)
 						&h1r->minor_ver, h1r->hdr,
 						&h1r->nr_hdr, h1->prev_len);
 			if (h1r->pret > 0)
-				return h1_parse_harder(h1, h1r);
+				return h1_parse_harder(h1, h1r, h1d);
 			if (h1r->pret == -1)
 				return h1_400(h1); /* parser error */
 
@@ -1001,11 +1008,8 @@ static enum mw_qev h1_event_step(struct mw_h1 *h1)
 		} else { /* r < 0 */
 			switch (errno) {
 			case EAGAIN: /* likely, detach to per-client buffer */
-				if (h1->prev_len && !h1->h1r) {
-					h1->h1r = h1r;
-					mwrap_assert(tsd_h1r == h1r);
-					tsd_h1r = NULL;
-				}
+				if (h1->prev_len)
+					prep_trickle(h1, h1r, h1d);
 				return MW_QEV_RD;
 			case ECONNRESET: /* common */
 			case ENOTCONN:
@@ -1258,7 +1262,7 @@ static void *h1d_run(void *x) /* pthread_create cb */
 					h1d_event_step(h1d);
 				} else {
 					h1 = h1_lookup(h1d, pfd[i].fd);
-					ev = h1_event_step(h1);
+					ev = h1_event_step(h1, h1d);
 					if (ev == MW_QEV_IGNORE)
 						continue;
 					h1->events = ev;