[master] d1ef9b9bf shard director: Fix regression from 08b642d70ff15346eb1818171862f70a3a9266af

Nils Goroll nils.goroll at uplex.de
Mon Jan 18 15:18:07 UTC 2021


commit d1ef9b9bf7ae4aaf32aec0f27a4bd3995bb1cd2b
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Mon Jan 18 14:14:22 2021 +0100

    shard director: Fix regression from 08b642d70ff15346eb1818171862f70a3a9266af
    
    and add vtc code to avoid it from happening again.
    
    The mentioned commit lead to resolve=NOW modify the shard's parameter
    PRIV_TASK when only parameters on the stack should have been modified.
    
    We will adjust this to use shard_param_task_r() in a follow-up
    
    Spotted thanks to Coverity, ref CID 1472212

diff --git a/bin/varnishtest/tests/d00021.vtc b/bin/varnishtest/tests/d00021.vtc
index cf0a1be27..730c32705 100644
--- a/bin/varnishtest/tests/d00021.vtc
+++ b/bin/varnishtest/tests/d00021.vtc
@@ -70,6 +70,10 @@ varnish v1 -vcl+backend {
 		set bereq.backend =
 		  vd.backend(resolve=LAZY, by=KEY, key=4294967295);
 	    }
+	    set bereq.http.bereq-now-1 = vd.backend(resolve=NOW);
+	    set bereq.http.bereq-now-mod =
+		vd.backend(resolve=NOW, alt=1);
+	    set bereq.http.bereq-now-2 = vd.backend(resolve=NOW);
 	}
 
 	sub backend_fetch_layered {
@@ -83,6 +87,10 @@ varnish v1 -vcl+backend {
 	    if (bereq.url == "/3") {
 		lp.set(by=KEY, key=4294967295);
 	    }
+	    set bereq.http.bereq-now-1 = l.backend(resolve=NOW);
+	    set bereq.http.bereq-now-mod =
+		l.backend(resolve=NOW, alt=1);
+	    set bereq.http.bereq-now-2 = l.backend(resolve=NOW);
 	}
 
 	sub vcl_backend_fetch {
@@ -106,6 +114,9 @@ varnish v1 -vcl+backend {
 	    }
 	    set beresp.http.director = bereq.backend;
 	    set beresp.http.backend = beresp.backend;
+	    set beresp.http.bereq-now-1 = bereq.http.bereq-now-1;
+	    set beresp.http.bereq-now-mod = bereq.http.bereq-now-mod;
+	    set beresp.http.bereq-now-2 = bereq.http.bereq-now-2;
 	}
 } -start
 
@@ -117,6 +128,9 @@ client c1 {
 	expect resp.http.healthy == "true"
 	expect resp.http.director == "vd"
 	expect resp.http.backend == "s1"
+	expect resp.http.backend == resp.http.bereq-now-1
+	expect resp.http.backend != resp.http.bereq-now-mod
+	expect resp.http.backend == resp.http.bereq-now-2
 
 	txreq -url /2
 	rxresp
@@ -124,6 +138,9 @@ client c1 {
 	expect resp.http.healthy == "true"
 	expect resp.http.director == "vd"
 	expect resp.http.backend == "s2"
+	expect resp.http.backend == resp.http.bereq-now-1
+	expect resp.http.backend != resp.http.bereq-now-mod
+	expect resp.http.backend == resp.http.bereq-now-2
 
 	txreq -url /3
 	rxresp
@@ -131,6 +148,9 @@ client c1 {
 	expect resp.http.healthy == "true"
 	expect resp.http.director == "vd"
 	expect resp.http.backend == "s3"
+	expect resp.http.backend == resp.http.bereq-now-1
+	expect resp.http.backend != resp.http.bereq-now-mod
+	expect resp.http.backend == resp.http.bereq-now-2
 
 	txreq -url /1 -hdr "layered: true"
 	rxresp
@@ -139,6 +159,9 @@ client c1 {
 	expect resp.http.director == "ll"
 	expect resp.http.backend == "s1"
 	expect resp.http.backend == resp.http.backend-now
+	expect resp.http.backend == resp.http.bereq-now-1
+	expect resp.http.backend != resp.http.bereq-now-mod
+	expect resp.http.backend == resp.http.bereq-now-2
 
 	txreq -url /2 -hdr "layered: true"
 	rxresp
@@ -147,6 +170,9 @@ client c1 {
 	expect resp.http.director == "ll"
 	expect resp.http.backend == "s2"
 	expect resp.http.backend == resp.http.backend-now
+	expect resp.http.backend == resp.http.bereq-now-1
+	expect resp.http.backend != resp.http.bereq-now-mod
+	expect resp.http.backend == resp.http.bereq-now-2
 
 	txreq -url /3 -hdr "layered: true"
 	rxresp
@@ -155,6 +181,9 @@ client c1 {
 	expect resp.http.director == "ll"
 	expect resp.http.backend == "s3"
 	expect resp.http.backend == resp.http.backend-now
+	expect resp.http.backend == resp.http.bereq-now-1
+	expect resp.http.backend != resp.http.bereq-now-mod
+	expect resp.http.backend == resp.http.bereq-now-2
 
 	txreq -url /1 -hdr "resolve: true"
 	rxresp
@@ -163,6 +192,9 @@ client c1 {
 	expect resp.http.director == "s1"
 	expect resp.http.backend == "s1"
 	expect resp.http.backend == resp.http.backend-now
+	expect resp.http.backend == resp.http.bereq-now-1
+	expect resp.http.backend != resp.http.bereq-now-mod
+	expect resp.http.backend == resp.http.bereq-now-2
 
 	txreq -url /2 -hdr "resolve: true"
 	rxresp
@@ -171,6 +203,9 @@ client c1 {
 	expect resp.http.director == "s2"
 	expect resp.http.backend == "s2"
 	expect resp.http.backend == resp.http.backend-now
+	expect resp.http.backend == resp.http.bereq-now-1
+	expect resp.http.backend != resp.http.bereq-now-mod
+	expect resp.http.backend == resp.http.bereq-now-2
 
 	txreq -url /3 -hdr "resolve: true"
 	rxresp
@@ -179,4 +214,7 @@ client c1 {
 	expect resp.http.director == "s3"
 	expect resp.http.backend == "s3"
 	expect resp.http.backend == resp.http.backend-now
+	expect resp.http.backend == resp.http.bereq-now-1
+	expect resp.http.backend != resp.http.bereq-now-mod
+	expect resp.http.backend == resp.http.bereq-now-2
 } -run
diff --git a/vmod/vmod_directors_shard.c b/vmod/vmod_directors_shard.c
index 2a214279d..1d0ce41f7 100644
--- a/vmod/vmod_directors_shard.c
+++ b/vmod/vmod_directors_shard.c
@@ -615,6 +615,13 @@ vmod_shard_backend(VRT_CTX, struct vmod_directors_shard *vshard,
 	else
 		resolve = VENUM(NOW);
 
+	if (ctx->method & SHARD_VCL_TASK_BEREQ) {
+		pp = shard_param_task_l(ctx, shardd, shardd->name,
+		    shardd->param);
+		if (pp == NULL)
+			return (NULL);
+	}
+
 	if (resolve == VENUM(LAZY)) {
 		if ((args & ~arg_resolve) == 0) {
 			AN(vshard->dir);
@@ -642,13 +649,6 @@ vmod_shard_backend(VRT_CTX, struct vmod_directors_shard *vshard,
 		WRONG("resolve enum");
 	}
 
-	if (ctx->method & SHARD_VCL_TASK_BEREQ) {
-		pp = shard_param_task_l(ctx, shardd, shardd->name,
-		    shardd->param);
-		if (pp == NULL)
-			return (NULL);
-	}
-
 	AN(pp);
 
 	if (args & arg_param) {


More information about the varnish-commit mailing list