[master] 49e44e390 respect `send_timeout` for "dripping" http1 writes

[Nils Goroll]

commit 49e44e390488c57c03d8111fcd43e245933bc151
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Mon Jan 27 14:18:28 2020 +0100

    respect `send_timeout` for "dripping" http1 writes
    
    Previously, we only checked `v1l->deadline` (which gets initialized
    from the `send_timeout` session attribute or parameter) only for short
    writes, so for successful "dripping" http1 writes (streaming from a
    backend busy object with small chunks), we did not respect the
    timeout.
    
    This patch restructures `V1L_Flush()` to always check the deadline
    before a write by turning a `while() { ... }` into a `do { ... }
    while` with the same continuation criteria: `while (i != v1l->liov)`
    is turned into `if (i == v1l->liov) break;` and `while (i > 0 || errno
    == EWOULDBLOCK)` is kept to retry short writes.
    
    This also reduces the `writev()` call sites to one.
    
    Fixes #3189

diff --git a/bin/varnishd/http1/cache_http1_line.c b/bin/varnishd/http1/cache_http1_line.c
index a6914d3e6..fcb6af3ee 100644
--- a/bin/varnishd/http1/cache_http1_line.c
+++ b/bin/varnishd/http1/cache_http1_line.c
@@ -200,18 +200,8 @@ V1L_Flush(const struct worker *wrk)
 			v1l->iov[v1l->ciov].iov_len = 0;
 		}
 
-		i = writev(*v1l->wfd, v1l->iov, v1l->niov);
-		if (i > 0)
-			v1l->cnt += i;
-		while (i != v1l->liov && (i > 0 || errno == EWOULDBLOCK)) {
-			/* Remove sent data from start of I/O vector,
-			 * then retry; we hit a timeout, and some data
-			 * may have been sent.
-			 *
-			 * XXX: Add a "minimum sent data per timeout
-			 * counter to prevent slowloris attacks
-			*/
-
+		i = 0;
+		do {
 			if (VTIM_real() > v1l->deadline) {
 				VSLb(v1l->vsl, SLT_Debug,
 				    "Hit total send timeout, "
@@ -221,16 +211,28 @@ V1L_Flush(const struct worker *wrk)
 				break;
 			}
 
+			i = writev(*v1l->wfd, v1l->iov, v1l->niov);
+			if (i > 0)
+				v1l->cnt += i;
+
+			if (i == v1l->liov)
+				break;
+
+			/* we hit a timeout, and some data may have been sent:
+			 * Remove sent data from start of I/O vector, then retry
+			 *
+			 * XXX: Add a "minimum sent data per timeout counter to
+			 * prevent slowloris attacks
+			 */
+
 			VSLb(v1l->vsl, SLT_Debug,
 			    "Hit idle send timeout, wrote = %zd/%zd; retrying",
 			    i, v1l->liov);
 
 			if (i > 0)
 				v1l_prune(v1l, i);
-			i = writev(*v1l->wfd, v1l->iov, v1l->niov);
-			if (i > 0)
-				v1l->cnt += i;
-		}
+		} while (i > 0 || errno == EWOULDBLOCK);
+
 		if (i <= 0) {
 			VSLb(v1l->vsl, SLT_Debug,
 			    "Write error, retval = %zd, len = %zd, errno = %s",
diff --git a/bin/varnishtest/tests/r03189.vtc b/bin/varnishtest/tests/r03189.vtc
new file mode 100644
index 000000000..cf3d466c9
--- /dev/null
+++ b/bin/varnishtest/tests/r03189.vtc
@@ -0,0 +1,28 @@
+varnishtest "h1 send_timeout and streaming of dripping chunks"
+
+barrier b cond 2
+
+server s1 {
+	rxreq
+	txresp -nolen -hdr "Transfer-Encoding: chunked"
+	chunkedlen 1
+	delay 1
+	non_fatal
+	barrier b sync
+	chunkedlen 1
+	delay 1
+	chunkedlen 0
+} -start
+
+varnish v1				\
+	-arg "-p idle_send_timeout=.1"	\
+	-arg "-p send_timeout=.8"	\
+	-vcl+backend { } -start
+
+client c1 {
+	txreq
+	rxresphdrs
+	rxchunk
+	barrier b sync
+	expect_close
+} -run
diff --git a/bin/varnishtest/tests/s00011.vtc b/bin/varnishtest/tests/s00011.vtc
index 50e6e4612..11ff01e36 100644
--- a/bin/varnishtest/tests/s00011.vtc
+++ b/bin/varnishtest/tests/s00011.vtc
@@ -1,10 +1,8 @@
 varnishtest "backend send timeouts"
 
 server s1 {
+	non_fatal
 	rxreq
-	expect req.method == POST
-	expect req.body == helloworld
-	txresp
 } -start
 
 varnish v1 -vcl+backend "" -start
@@ -21,5 +19,5 @@ client c1 {
 	delay 2
 	send world
 	rxresp
-	expect resp.status == 200
+	expect resp.status == 503
 } -run