[master] 3446fb1 Ensure bereq changes are not lost across vcl_backend_xxx methods
Federico G. Schwindt
fgsch at lodoss.net
Thu Jul 31 13:55:11 CEST 2014
commit 3446fb1c563357054c6aa93a08c946ce080c26f1
Author: Federico G. Schwindt <fgsch at lodoss.net>
Date: Thu Jul 31 12:48:03 2014 +0100
Ensure bereq changes are not lost across vcl_backend_xxx methods
Add std.rollback() to rollback req or bereq and mark the builtin rollback
as deprecated.
Original diff by Nils Goroll. Tweaks and reworked after phk comments
by me.
Fixes #1512
diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index f21c57d..e171d66 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -490,6 +490,7 @@ struct busyobj {
enum busyobj_state_e state;
struct ws ws[1];
+ char *ws_bo;
struct vbc *vbc;
struct http *bereq0;
struct http *bereq;
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 1e45392..70f467a 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -220,6 +220,10 @@ vbf_stp_mkbereq(const struct worker *wrk, struct busyobj *bo)
}
}
+ HTTP_Setup(bo->bereq, bo->ws, bo->vsl, SLT_BereqMethod);
+ bo->ws_bo = WS_Snapshot(bo->ws);
+ HTTP_Copy(bo->bereq, bo->bereq0);
+
VBO_setstate(bo, BOS_REQ_DONE);
return (F_STP_STARTFETCH);
}
@@ -267,9 +271,6 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo)
else
AZ(bo->req);
- HTTP_Setup(bo->bereq, bo->ws, bo->vsl, SLT_BereqMethod);
- HTTP_Copy(bo->bereq, bo->bereq0);
-
http_PrintfHeader(bo->bereq, "X-Varnish: %u", VXID(bo->vsl->wid));
VCL_backend_fetch_method(bo->vcl, wrk, NULL, bo, bo->bereq->ws);
diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index 4038693..edfa2a5 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -357,13 +357,22 @@ VRT_BOOL_string(unsigned val)
/*--------------------------------------------------------------------*/
void
-VRT_Rollback(const struct vrt_ctx *ctx)
+VRT_Rollback(const struct vrt_ctx *ctx, const struct http *hp)
{
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
- CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC);
- HTTP_Copy(ctx->req->http, ctx->req->http0);
- WS_Reset(ctx->req->ws, ctx->req->ws_req);
+ CHECK_OBJ_NOTNULL(hp, HTTP_MAGIC);
+ if (hp == ctx->http_req) {
+ CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC);
+ HTTP_Copy(ctx->req->http, ctx->req->http0);
+ WS_Reset(ctx->req->ws, ctx->req->ws_req);
+ } else if (hp == ctx->http_bereq) {
+ CHECK_OBJ_NOTNULL(ctx->bo, BUSYOBJ_MAGIC);
+ HTTP_Copy(ctx->bo->bereq, ctx->bo->bereq0);
+ WS_Reset(ctx->bo->bereq->ws, ctx->bo->ws_bo);
+ WS_Reset(ctx->bo->ws, ctx->bo->ws_bo);
+ } else
+ WRONG("VRT_Rollback 'hp' invalid");
}
/*--------------------------------------------------------------------*/
diff --git a/bin/varnishtest/tests/m00017.vtc b/bin/varnishtest/tests/m00017.vtc
new file mode 100644
index 0000000..5622ec0
--- /dev/null
+++ b/bin/varnishtest/tests/m00017.vtc
@@ -0,0 +1,69 @@
+varnishtest "Test std.rollback"
+
+server s1 {
+ rxreq
+ expect req.url == "/foo"
+ expect req.http.foobar == "bar"
+ txresp -status 400
+ accept
+ rxreq
+ expect req.url == "/bar"
+ expect req.http.foobar == "foo"
+ txresp
+ accept
+ rxreq
+ expect req.url == "/baz"
+ expect req.http.foobar == "qux"
+ txresp -status 400
+ accept
+ rxreq
+ expect req.url == "/qux"
+ expect req.http.foobar == "baz"
+ txresp
+} -start
+
+varnish v1 -vcl+backend {
+ import ${vmod_std};
+
+ sub vcl_recv {
+ if (req.url == "/foo") {
+ set req.http.foobar = "bar";
+ }
+ }
+
+ sub vcl_deliver {
+ if (resp.status == 400) {
+ std.rollback(req);
+ set req.url = "/bar";
+ return (restart);
+ }
+ }
+} -start
+
+client c1 {
+ txreq -url "/foo" -hdr "foobar: foo"
+ rxresp
+} -run
+
+varnish v1 -vcl+backend {
+ import ${vmod_std};
+
+ sub vcl_backend_fetch {
+ if (bereq.url == "/baz") {
+ set bereq.http.foobar = "qux";
+ }
+ }
+
+ sub vcl_backend_response {
+ if (beresp.status == 400) {
+ std.rollback(bereq);
+ set bereq.url = "/qux";
+ return (retry);
+ }
+ }
+}
+
+client c1 {
+ txreq -url "/baz" -hdr "foobar: baz"
+ rxresp
+} -run
diff --git a/bin/varnishtest/tests/r01512.vtc b/bin/varnishtest/tests/r01512.vtc
new file mode 100644
index 0000000..71496a2
--- /dev/null
+++ b/bin/varnishtest/tests/r01512.vtc
@@ -0,0 +1,56 @@
+varnishtest "Regression test for #1512"
+
+# First test bereq changes across v_b_r and v_b_f
+
+server s1 {
+ rxreq
+ txresp -status 700
+} -start
+
+varnish v1 -vcl+backend {
+ sub vcl_backend_fetch {
+ if (bereq.http.x-abandon == "1") {
+ return (abandon);
+ }
+ }
+ sub vcl_backend_response {
+ if (beresp.status == 700) {
+ set bereq.http.x-abandon = "1";
+ return (retry);
+ }
+ }
+ sub vcl_synth {
+ set resp.status = 701;
+ }
+} -start
+
+client c1 {
+ txreq
+ rxresp
+ expect resp.status == 701
+} -run
+
+# Then across v_b_e and v_b_f
+
+varnish v1 -vcl {
+ backend bad { .host = "${bad_ip}"; }
+ sub vcl_backend_fetch {
+ set bereq.backend = bad;
+ if (bereq.http.x-abandon == "2") {
+ return (abandon);
+ }
+ }
+ sub vcl_backend_error {
+ set bereq.http.x-abandon = "2";
+ return (retry);
+ }
+ sub vcl_synth {
+ set resp.status = 702;
+ }
+}
+
+client c1 {
+ txreq
+ rxresp
+ expect resp.status == 702
+} -run
diff --git a/doc/sphinx/reference/vcl.rst b/doc/sphinx/reference/vcl.rst
index 743689e..6828f99 100644
--- a/doc/sphinx/reference/vcl.rst
+++ b/doc/sphinx/reference/vcl.rst
@@ -369,7 +369,8 @@ return()
in the request handling state machine.
rollback()
- Restore request HTTP headers to their original state.
+ Restore *req* HTTP headers to their original state. This function is
+ deprecated. Use std.rollback() instead.
synthetic(STRING)
Prepare a synthetic response body containing the STRING. Available in
diff --git a/include/vrt.h b/include/vrt.h
index efc9c94..ead55d3 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -249,7 +249,7 @@ void VRT_hashdata(const struct vrt_ctx *, const char *str, ...);
int VRT_strcmp(const char *s1, const char *s2);
void VRT_memmove(void *dst, const void *src, unsigned len);
-void VRT_Rollback(const struct vrt_ctx *);
+void VRT_Rollback(const struct vrt_ctx *, const struct http *);
/* Synthetic pages */
void VRT_synth_page(const struct vrt_ctx *, const char *, ...);
diff --git a/lib/libvcc/vcc_action.c b/lib/libvcc/vcc_action.c
index 9b1d22d..1b0c4b2 100644
--- a/lib/libvcc/vcc_action.c
+++ b/lib/libvcc/vcc_action.c
@@ -376,7 +376,7 @@ parse_rollback(struct vcc *tl)
{
vcc_NextToken(tl);
- Fb(tl, 1, "VRT_Rollback(ctx);\n");
+ Fb(tl, 1, "VRT_Rollback(ctx, VRT_r_req(ctx));\n");
}
/*--------------------------------------------------------------------*/
diff --git a/lib/libvmod_std/vmod.vcc b/lib/libvmod_std/vmod.vcc
index fbee007..5bcc403 100644
--- a/lib/libvmod_std/vmod.vcc
+++ b/lib/libvmod_std/vmod.vcc
@@ -174,6 +174,13 @@ $Function INT port(IP)
Description
Returns the port number of an IP address.
+$Function VOID rollback(HTTP)
+
+Description
+ Restore *h* HTTP headers to their original state.
+Example
+ std.rollback(bereq);
+
$Function VOID timestamp(STRING)
Description
diff --git a/lib/libvmod_std/vmod_std.c b/lib/libvmod_std/vmod_std.c
index 718fb71..c5d0cfe 100644
--- a/lib/libvmod_std/vmod_std.c
+++ b/lib/libvmod_std/vmod_std.c
@@ -205,6 +205,12 @@ vmod_port(const struct vrt_ctx *ctx, VCL_IP ip)
return (VSA_Port(ip));
}
+VCL_VOID __match_proto__(td_std_rollback)
+vmod_rollback(const struct vrt_ctx *ctx, VCL_HTTP hp)
+{
+ VRT_Rollback(ctx, hp);
+}
+
VCL_VOID __match_proto__(td_std_timestamp)
vmod_timestamp(const struct vrt_ctx *ctx, VCL_STRING label)
{
More information about the varnish-commit
mailing list