[master] dc5bddbd3 range: Propagate the VDP error for short ranges

[Nils Goroll]

commit dc5bddbd301529b101598b644544b99ccabca12c
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Mon Mar 8 22:28:37 2021 +0100

    range: Propagate the VDP error for short ranges
    
    And fix the h2_req VDP error handling as per the VDP contract.
    
    Test case inspired by Simon. Since this is one of those test cases that
    explicitly mix two features I wasn't sure whether I wanted to make this
    an h2 test case or a range test case. Since this was ultimately a range
    bug I decided to register it in a range test case.
    
    It's not obvious what should have been authoritative here. The range VDP
    was rightfully latching an error via SC_RANGE_SHORT that is defined as an
    error-type session close reason, but VDP_DeliverObj() doesn't take that
    into account. While SC_RANGE_SHORT isn't a session/protocol error for h2
    but rather a stream error it is not obvious what VDP_DeliverObj() should
    do in the absence of a negative retval and the presence of a non-null
    sess_close.
    
    Maybe another way could be to turn enum sess_close into a struct and
    embed http1 and h2 specificities directly in struct fields. We already
    have somewhat structured information in the sess_close.h table.
    
    Refs 03f71c6e6dae

diff --git a/bin/varnishd/cache/cache_range.c b/bin/varnishd/cache/cache_range.c
index 007cf2c74..bbc40cb46 100644
--- a/bin/varnishd/cache/cache_range.c
+++ b/bin/varnishd/cache/cache_range.c
@@ -55,8 +55,10 @@ vrg_range_fini(struct vdp_ctx *vdc, void **priv)
 
 	CHECK_OBJ_NOTNULL(vdc, VDP_CTX_MAGIC);
 	CAST_OBJ_NOTNULL(vrg_priv, *priv, VRG_PRIV_MAGIC);
-	if (vrg_priv->range_off < vrg_priv->range_high)
+	if (vrg_priv->range_off < vrg_priv->range_high) {
 		Req_Fail(vrg_priv->req, SC_RANGE_SHORT);
+		vrg_priv->req->vdc->retval = -1;
+	}
 	*priv = NULL;	/* struct on ws, no need to free */
 	return (0);
 }
diff --git a/bin/varnishd/http2/cache_http2_deliver.c b/bin/varnishd/http2/cache_http2_deliver.c
index a73c9943d..8b0514228 100644
--- a/bin/varnishd/http2/cache_http2_deliver.c
+++ b/bin/varnishd/http2/cache_http2_deliver.c
@@ -95,7 +95,7 @@ h2_fini(struct vdp_ctx *vdc, void **priv)
 	if (r2->error)
 		return (0);
 
-	if (vdc->retval) {
+	if (vdc->retval < 0) {
 		r2->error = H2SE_INTERNAL_ERROR; /* XXX: proper error? */
 		H2_Send_Get(vdc->wrk, r2->h2sess, r2);
 		H2_Send_RST(vdc->wrk, r2->h2sess, r2, r2->stream, r2->error);
diff --git a/bin/varnishtest/tests/c00034.vtc b/bin/varnishtest/tests/c00034.vtc
index 3fbe5fefd..47366287e 100644
--- a/bin/varnishtest/tests/c00034.vtc
+++ b/bin/varnishtest/tests/c00034.vtc
@@ -191,3 +191,23 @@ client c1 {
 	gunzip
 	expect resp.bodylen == 100
 } -run
+
+# Test partial range with http2
+
+server s1 {
+	rxreq
+	txresp -hdr "Content-length: 3" -body "BLA"
+} -start
+
+varnish v1 -cliok "param.set feature +http2"
+varnish v1 -vcl+backend ""
+
+client c2 {
+	stream 1 {
+		txreq -hdr "range" "bytes=0-1"
+		rxresp
+		expect resp.status == 206
+		expect resp.http.content-length == 2
+		expect resp.body == BL
+	} -run
+} -run