[master] 7de4ae97e outlaw std.rollback(req) in vcl_pipe {}

[Nils Goroll]

commit 7de4ae97e18ba2f31f7d6a24c1194746e6834e5e
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Fri Nov 27 18:28:47 2020 +0100

    outlaw std.rollback(req) in vcl_pipe {}
    
    With the previous commits, as the bereq uses the req workspace, we
    cannot roll back the req without also rolling back the bereq.
    
    As the use case is rather exotic, we spare the additional complications
    and just outlaw rollbacks from vcl_pipe.
    
    std.rollback(bereq) is already forbidden by the compiler because the
    bereq symbol is not available in vcl_pipe {}.
    
    We could, alternative to the runtime failure for std.rollback(req),
    remove the req symbol from vcl_pipe {}, but that could break use of req
    to vmod functions also. So we chose the more compatible route of
    selectively failing rollback rather than making impossible other use
    cases.

diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index 327a651f1..67abc24b2 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -795,6 +795,10 @@ VRT_Rollback(VRT_CTX, VCL_HTTP hp)
 
 	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
 	CHECK_OBJ_NOTNULL(hp, HTTP_MAGIC);
+	if (ctx->method & VCL_MET_PIPE) {
+		VRT_fail(ctx, "Cannot rollback in vcl_pipe {}");
+		return;
+	}
 	if (hp == ctx->http_req) {
 		CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC);
 		Req_Rollback(ctx->req);
diff --git a/bin/varnishtest/tests/m00017.vtc b/bin/varnishtest/tests/m00017.vtc
index 78e3bdcbc..012e92989 100644
--- a/bin/varnishtest/tests/m00017.vtc
+++ b/bin/varnishtest/tests/m00017.vtc
@@ -3,6 +3,8 @@ varnishtest "Test std.rollback"
 # bug regressions:
 # - #3009
 # - #3083
+# - #3329
+# - #3385
 
 server s1 {
 	rxreq
@@ -335,3 +337,40 @@ client c6 {
 } -run
 
 server s1 -wait
+
+# this could work, but would need additional coding to save
+# the right snapshot of the bereq on the req ws
+varnish v1 -errvcl {Not available in subroutine 'vcl_pipe'} {
+	import std;
+
+	backend proforma None;
+
+	sub vcl_pipe {
+		std.rollback(bereq);
+	}
+}
+
+# vcl_pipe { std.rollback(req); } cannot work unless it also implied
+# rolling back the bereq first.
+# We would want to remove req from vcl_pipe, but that could break
+# vmods, so we fail specifically at runtime
+
+varnish v1 -vcl {
+	import std;
+
+	backend proforma None;
+
+	sub vcl_recv {
+		return (pipe);
+	}
+	sub vcl_pipe {
+		std.rollback(req);
+	}
+}
+
+client c7 {
+	txreq -url /
+	rxresp
+	expect resp.status == 503
+	expect resp.reason == "VCL failed"
+} -run