[6.0] a61b713d2 New fix for #2285 and #2624

[Dridi Boukelmoune]

commit a61b713d2533cb0473eeebeb4fb72d3c100c38a9
Author: Martin Blix Grydeland <martin at varnish-software.com>
Date:   Tue Jun 5 17:02:55 2018 +0200

    New fix for #2285 and #2624
    
    Previous fix for #2285 (and the duplicate #2624) was missdiagnosed. The
    problem stems from a wrong assumption that the number of bytes already
    pipelined will be less than maxbytes, with maxbytes beeing the maximum
    number of bytes the HTC_RxStuff may need to get a full work unit. That
    assumption may fail during the H/1 to H/2 upgrade path where maxbytes
    change with the context, or during runtime changing of parameters.
    
    This patch makes HTC_RxStuff not assert if the pipelined data turned out
    to exceed maxbytes, but return overflow if we run out of workspace.
    
    (#2624 has received a workaround in the H/2 code that perhaps should be
    reverted).

diff --git a/bin/varnishd/cache/cache_session.c b/bin/varnishd/cache/cache_session.c
index a6c8f8378..0500bcef2 100644
--- a/bin/varnishd/cache/cache_session.c
+++ b/bin/varnishd/cache/cache_session.c
@@ -239,6 +239,11 @@ HTC_RxPipeline(struct http_conn *htc, void *p)
 /*----------------------------------------------------------------------
  * Receive a request/packet/whatever, with timeouts
  *
+ * maxbytes is the maximum number of bytes the caller expects to need to
+ * reach a complete work unit. Note that due to pipelining the actual
+ * number of bytes passed to func in htc->rxbuf_b through htc->rxbuf_e may
+ * be larger.
+ *
  * t0 is when we start
  * *t1 becomes time of first non-idle rx
  * *t2 becomes time of complete rx
@@ -261,6 +266,7 @@ HTC_RxStuff(struct http_conn *htc, htc_complete_f *func,
 	AN(htc->ws->r);
 	AN(htc->rxbuf_b);
 	assert(htc->rxbuf_b <= htc->rxbuf_e);
+	assert(htc->rxbuf_e <= htc->ws->r);
 
 	AZ(isnan(tn));
 	if (t1 != NULL)
@@ -273,7 +279,7 @@ HTC_RxStuff(struct http_conn *htc, htc_complete_f *func,
 	}
 	z = (htc->ws->r - htc->rxbuf_b);
 	if (z < maxbytes)
-		maxbytes = z;
+		maxbytes = z;	/* Cap maxbytes at available WS */
 
 	while (1) {
 		now = VTIM_real();
@@ -308,8 +314,9 @@ HTC_RxStuff(struct http_conn *htc, htc_complete_f *func,
 		if (!isnan(ti) && ti < tn && hs == HTC_S_EMPTY)
 			tmo = ti - now;
 		z = maxbytes - (htc->rxbuf_e - htc->rxbuf_b);
-		assert(z >= 0);
-		if (z == 0) {
+		if (z <= 0) {
+			/* maxbytes reached but not HTC_S_COMPLETE. Return
+			 * overflow. */
 			WS_ReleaseP(htc->ws, htc->rxbuf_b);
 			return (HTC_S_OVERFLOW);
 		}
diff --git a/bin/varnishtest/tests/r02219.vtc b/bin/varnishtest/tests/r02219.vtc
index 7187abc79..ff1a924f7 100644
--- a/bin/varnishtest/tests/r02219.vtc
+++ b/bin/varnishtest/tests/r02219.vtc
@@ -5,6 +5,8 @@ server s1 {
 	txresp
 	rxreq
 	txresp
+	rxreq
+	txresp
 } -start
 
 varnish v1 -arg "-p workspace_client=9k" -proto PROXY -vcl+backend {
@@ -59,3 +61,8 @@ client c2 {
 }
 	rxresp
 } -run
+
+client c3 {
+	send "PROXY TCP4 127.0.0.1 127.0.0.1 1111 2222\r\nGET /CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC HTTP/1.1\r\n\r\n"
+	rxresp
+} -run