Inspired by Martin I have stared hard at http_conn for a few days.

This commit cleans up a number of weeds and loose ends:

The complete_function should not muck about with the pipelining or WS
reservation.

Only when we have dissected the headers for good do we know how long
the headers were.

Make the HTTP1_Dissect* functions have consistent parameters.

bin/varnishd/cache/cache.h

@@ -838,8 +838,8 @@ enum sess_close http_DoConnection(struct http *hp);
 /* cache_http1_proto.c */
 
 htc_complete_f HTTP1_Complete;
-uint16_t HTTP1_DissectRequest(struct http_conn *htc, struct http *hp);
-uint16_t HTTP1_DissectResponse(struct http *sp, struct http_conn *htc);
+uint16_t HTTP1_DissectRequest(struct http_conn *, struct http *);
+uint16_t HTTP1_DissectResponse(struct http_conn *, struct http *);
 unsigned HTTP1_Write(const struct worker *w, const struct http *hp, const int*);
 
 #define HTTPH(a, b, c) extern char b[];

bin/varnishd/cache/cache_session.c

@@ -219,7 +219,7 @@ SES_Rx(struct http_conn *htc, double tmo)
  * Receive a request/packet/whatever, with timeouts
  *
  * t0 is when we start
- * *t1 becomes time of first non-idle rx 
+ * *t1 becomes time of first non-idle rx
  * *t2 becomes time of complete rx
  * ti is when we return IDLE if nothing has arrived
  * tn is when we timeout on non-complete
@@ -266,6 +266,7 @@ SES_RxStuff(struct http_conn *htc, htc_complete_f *func, double t0,
 			return (HTC_S_JUNK);
 		}
 		if (hs == HTC_S_COMPLETE) {
+			WS_ReleaseP(htc->ws, htc->rxbuf_e);
 			/* Got it, run with it */
 			if (t1 != NULL && isnan(*t1))
 				*t1 = now;

bin/varnishd/http1/cache_http1_fetch.c

@@ -138,7 +138,7 @@ V1F_FetchRespHdr(struct busyobj *bo)
 
 	struct http *hp;
 	enum htc_status_e hs;
-	int first;
+	int first, i;
 	struct http_conn *htc;
 
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
@@ -188,11 +188,13 @@ V1F_FetchRespHdr(struct busyobj *bo)
 			    htc->between_bytes_timeout);
 		}
 	} while (hs != HTC_S_COMPLETE);
-	bo->acct.beresp_hdrbytes += htc->rxbuf_e - htc->rxbuf_b;
+	WS_ReleaseP(htc->ws, htc->rxbuf_e);
 
 	hp = bo->beresp;
 
-	if (HTTP1_DissectResponse(hp, htc)) {
+	i = HTTP1_DissectResponse(htc, hp);
+	bo->acct.beresp_hdrbytes += htc->rxbuf_e - htc->rxbuf_b;
+	if (i) {
 		VSLb(bo->vsl, SLT_FetchError, "http format error");
 		htc->doclose = SC_RX_JUNK;
 		return (-1);

bin/varnishd/http1/cache_http1_fsm.c

@@ -159,6 +159,7 @@ HTTP1_Session(struct worker *wrk, struct req *req)
 {
 	enum htc_status_e hs;
 	struct sess *sp;
+	int i;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
@@ -234,9 +235,6 @@ HTTP1_Session(struct worker *wrk, struct req *req)
 			if (hs != HTC_S_COMPLETE)
 				WRONG("htc_status (nonbad)");
 
-			req->acct.req_hdrbytes +=
-			    req->htc->rxbuf_e - req->htc->rxbuf_b;
-
 			sp->sess_step = S_STP_H1WORKING;
 			break;
 		case S_STP_H1BUSY:
@@ -255,7 +253,10 @@ HTTP1_Session(struct worker *wrk, struct req *req)
 			sp->sess_step = S_STP_H1PROC;
 			break;
 		case S_STP_H1WORKING:
-			if (http1_dissect(wrk, req)) {
+			i = http1_dissect(wrk, req);
+			req->acct.req_hdrbytes +=
+			    req->htc->rxbuf_e - req->htc->rxbuf_b;
+			if (i) {
 				SES_Close(req->sp, req->doclose);
 				sp->sess_step = S_STP_H1CLEANUP;
 				break;
@@ -277,16 +278,16 @@ HTTP1_Session(struct worker *wrk, struct req *req)
 				return;
 			SES_RxReInit(req->htc);
 			if (HTTP1_Complete(req->htc) == HTC_S_COMPLETE) {
+				WS_ReleaseP(req->htc->ws, req->htc->rxbuf_e);
 				AZ(req->vsl->wid);
 				req->t_first = req->t_req = sp->t_idle;
 				wrk->stats->sess_pipeline++;
-				req->acct.req_hdrbytes +=
-				    req->htc->rxbuf_e - req->htc->rxbuf_b;
 				sp->sess_step = S_STP_H1WORKING;
 			} else {
 				if (req->htc->rxbuf_e != req->htc->rxbuf_b)
 					wrk->stats->sess_readahead++;
 				sp->sess_step = S_STP_H1NEWREQ;
+
 			}
 			break;
 		default:

bin/varnishd/http1/cache_http1_proto.c

@@ -83,6 +83,10 @@ HTTP1_Complete(struct http_conn *htc)
 		*htc->rxbuf_e = '\0';
 		return (HTC_S_EMPTY);
 	}
+	/*
+	 * Here we just look for NL[CR]NL to see that reception
+	 * is completed.  More stringent validation happens later.
+	 */
 	while (1) {
 		p = strchr(p, '\n');
 		if (p == NULL)
@@ -93,13 +97,6 @@ HTTP1_Complete(struct http_conn *htc)
 		if (*p == '\n')
 			break;
 	}
-	p++;
-	WS_ReleaseP(htc->ws, htc->rxbuf_e);
-	if (p < htc->rxbuf_e) {
-		htc->pipeline_b = p;
-		htc->pipeline_e = htc->rxbuf_e;
-		htc->rxbuf_e = p;
-	}
 	return (HTC_S_COMPLETE);
 }
 
@@ -109,7 +106,7 @@ HTTP1_Complete(struct http_conn *htc)
  */
 
 static uint16_t
-http1_dissect_hdrs(struct http *hp, char *p, const struct http_conn *htc)
+http1_dissect_hdrs(struct http *hp, char *p, struct http_conn *htc)
 {
 	char *q, *r;
 
@@ -122,6 +119,8 @@ http1_dissect_hdrs(struct http *hp, char *p, const struct http_conn *htc)
 
 		/* Find end of next header */
 		q = r = p;
+		if (vct_iscrlf(p))
+			break;
 		while (r < htc->rxbuf_e) {
 			if (!vct_iscrlf(r)) {
 				r++;
@@ -132,6 +131,8 @@ http1_dissect_hdrs(struct http *hp, char *p, const struct http_conn *htc)
 			r += vct_skipcrlf(r);
 			if (r >= htc->rxbuf_e)
 				break;
+			if (vct_iscrlf(r))
+				break;
 			/* If line does not continue: got it. */
 			if (!vct_issp(*r))
 				break;
@@ -184,6 +185,13 @@ http1_dissect_hdrs(struct http *hp, char *p, const struct http_conn *htc)
 			return (400);
 		}
 	}
+	if (p < htc->rxbuf_e)
+		p += vct_skipcrlf(p);
+	if (p < htc->rxbuf_e) {
+		htc->pipeline_b = p;
+		htc->pipeline_e = htc->rxbuf_e;
+		htc->rxbuf_e = p;
+	}
 	return (0);
 }
 
@@ -192,7 +200,7 @@ http1_dissect_hdrs(struct http *hp, char *p, const struct http_conn *htc)
  */
 
 static uint16_t
-http1_splitline(struct http *hp, const struct http_conn *htc, const int *hf)
+http1_splitline(struct http *hp, struct http_conn *htc, const int *hf)
 {
 	char *p;
 	int i;
@@ -390,7 +398,7 @@ HTTP1_DissectRequest(struct http_conn *htc, struct http *hp)
 /*--------------------------------------------------------------------*/
 
 uint16_t
-HTTP1_DissectResponse(struct http *hp, struct http_conn *htc)
+HTTP1_DissectResponse(struct http_conn *htc, struct http *hp)
 {
 	uint16_t retval = 0;
 	const char *p;

bin/varnishd/proxy/cache_proxy_proto.c

@@ -376,7 +376,6 @@ VPX_Proto_Sess(struct worker *wrk, void *priv)
 		req->htc->pipeline_b = NULL;
 	else
 		req->htc->pipeline_e = req->htc->rxbuf_e;
-	WS_Release(req->htc->ws, 0);
 	SES_RxReInit(req->htc);
 	req->sp->sess_step = S_STP_H1NEWREQ;
 	wrk->task.func = SES_Proto_Req;