[master] 3228cb701 Fix VRT_priv_top() to use the topreq's workspace and change v43.vtc to test the issue

Wed Jan 13 10:29:06 UTC 2021

commit 3228cb70159a77a677945f4520e9c7b51edb6e95
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Wed Jan 13 11:02:17 2021 +0100

    Fix VRT_priv_top() to use the topreq's workspace and change v43.vtc to
    test the issue
    
    This bug triggered when a PRIV_TOP was requested only from esi_level >
    0 and used by a "sibling" ESI request, that is, another request on the
    same ESI level or its descendants.
    
    The bug was introduced by someone(tm) in
    542bf9b820bbc40c2c95b5fa0282621b997ac594: I removed req =
    req->top->topreq but continued to use the workspace from req->ws.
    
    Fixes #3496

diff --git a/bin/varnishd/cache/cache_vrt_priv.c b/bin/varnishd/cache/cache_vrt_priv.c
index 67d5b5755..aeb7cb87c 100644
--- a/bin/varnishd/cache/cache_vrt_priv.c
+++ b/bin/varnishd/cache/cache_vrt_priv.c
@@ -218,6 +218,8 @@ VRT_priv_top(VRT_CTX, const void *vmod_id)
 	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
 	top = req->top;
 	CHECK_OBJ_NOTNULL(top, REQTOP_MAGIC);
+	req = top->topreq;
+	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 
 	Lck_Lock(&sp->mtx);
 	priv = vrt_priv_dynamic(req->ws, top->privs, (uintptr_t)vmod_id);
diff --git a/bin/varnishtest/tests/v00043.vtc b/bin/varnishtest/tests/v00043.vtc
index 8933aa5ba..949e24bfc 100644
--- a/bin/varnishtest/tests/v00043.vtc
+++ b/bin/varnishtest/tests/v00043.vtc
@@ -7,15 +7,18 @@ server s1 {
 	expect req.url == "/a"
 	expect req.http.x0 == "/a0"
 	expect req.http.x1 == "/a0"
+	expect req.http.o1 == <undef>
 	txresp -body {
 		<html>
-		<esi:include src="/foo"/>
+		<esi:include src="/foo1"/>
+		<esi:include src="/foo2"/>
 	}
 
 	rxreq
-	expect req.url == "/foo"
+	expect req.url == "/foo1"
 	expect req.http.x0 == "/a0"
 	expect req.http.x1 == "/a0"
+	expect req.http.o1 == "/foo11"
 	txresp -body {
 		<html>
 		<esi:include src="/bar"/>
@@ -25,12 +28,24 @@ server s1 {
 	expect req.url == "/bar"
 	expect req.http.x0 == "/a0"
 	expect req.http.x1 == "/a0"
+	expect req.http.o1 == "/foo11"
 	txresp
 
+	rxreq
+	expect req.url == "/foo2"
+	expect req.http.x0 == "/a0"
+	expect req.http.x1 == "/a0"
+	expect req.http.o1 == "/foo11"
+	txresp -body {
+		<html>
+		<esi:include src="/bar"/>
+	}
+
 	rxreq
 	expect req.url == "/b"
 	expect req.http.x0 == "/b0"
 	expect req.http.x1 == "/b0"
+	expect req.http.o1 == <undef>
 
 	txresp
 } -start
@@ -40,15 +55,23 @@ varnish v1 -cliok "param.set debug +syncvsl" -vcl+backend {
 
 	sub vcl_init {
 		new o = debug.obj();
+		new o2 = debug.obj();
 	}
 
 	sub vcl_recv {
 		set req.http.x0 = debug.test_priv_top(req.url + req.esi_level);
-		o.test_priv_top(req.url + req.esi_level);
+		if (req.url == "/foo1") {
+			o.test_priv_top(req.url + req.esi_level);
+		} else {
+			o2.test_priv_top(req.url + req.esi_level);
+		}
 	}
 
 	sub vcl_miss {
 		set req.http.x1 = debug.test_priv_top("");
+		if (req.esi_level > 0) {
+			set req.http.o1 = o.test_priv_top("");
+		}
 	}
 
 	sub vcl_backend_response {
@@ -58,7 +81,6 @@ varnish v1 -cliok "param.set debug +syncvsl" -vcl+backend {
 
 	sub vcl_deliver {
 		set resp.http.x1 = debug.test_priv_top("");
-		set resp.http.o1 = o.test_priv_top("");
 	}
 } -start
 
@@ -67,12 +89,10 @@ client c1 {
 	txreq -url /a
 	rxresp
 	expect resp.http.x1 == "/a0"
-	expect resp.http.o1 == "/a0"
 
 	txreq -url /b
 	rxresp
 	expect resp.http.x1 == "/b0"
-	expect resp.http.o1 == "/b0"
 } -run
 
 varnish v1 -expect client_req == 2
