[master] dc5bddbd3 range: Propagate the VDP error for short ranges
Nils Goroll
nils.goroll at uplex.de
Wed Mar 10 11:07:07 UTC 2021
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
More information about the varnish-commit
mailing list