I thought we probably won't want to bump the major version, so I kept rollback as a wrapper for rollback_req
Fixes #1512
>From 8e9c19d716777853a8b5cca8d553e7b5bfec48b4 Mon Sep 17 00:00:00 2001 From: Nils Goroll <[email protected]> Date: Mon, 21 Jul 2014 14:30:00 +0200 Subject: [PATCH 3/3] Do not reset bereq on retry. Add rollback_bereq for this purpose. Rename rollback to rollback_req. Mark rollback deprecated. Fixes #1512 --- bin/varnishd/cache/cache.h | 1 + bin/varnishd/cache/cache_fetch.c | 9 ++++++--- bin/varnishd/cache/cache_vrt.c | 24 +++++++++++++++++++++++- bin/varnishtest/tests/c00032.vtc | 4 ++-- bin/varnishtest/tests/c00069.vtc | 35 +++++++++++++++++++++++++++++++++++ bin/varnishtest/tests/r01168.vtc | 4 ++-- bin/varnishtest/tests/r01512.vtc | 33 +++++++++++++++++++++++++++++++++ bin/varnishtest/tests/v00018.vtc | 2 +- doc/sphinx/reference/index.rst | 3 ++- doc/sphinx/reference/vcl.rst | 11 ++++++++++- include/vrt.h | 9 ++++++++- lib/libvcc/vcc_action.c | 16 +++++++++++++--- 12 files changed, 136 insertions(+), 15 deletions(-) create mode 100644 bin/varnishtest/tests/c00069.vtc create mode 100644 bin/varnishtest/tests/r01512.vtc diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h index a6c7006..93f084f 100644 --- a/bin/varnishd/cache/cache.h +++ b/bin/varnishd/cache/cache.h @@ -489,6 +489,7 @@ struct busyobj { enum busyobj_state_e state; struct ws ws[1]; + char *ws_snap; /* ws snapshot for rollback */ 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 70efc36..9883e77 100644 --- a/bin/varnishd/cache/cache_fetch.c +++ b/bin/varnishd/cache/cache_fetch.c @@ -215,6 +215,12 @@ vbf_stp_mkbereq(const struct worker *wrk, struct busyobj *bo) } } + HTTP_Setup(bo->bereq, bo->ws, bo->vsl, SLT_BereqMethod); + + bo->ws_snap = WS_Snapshot(bo->ws); + + HTTP_Copy(bo->bereq, bo->bereq0); + VBO_setstate(bo, BOS_REQ_DONE); return (F_STP_STARTFETCH); } @@ -269,9 +275,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", bo->vsl->wid & VSL_IDENTMASK); diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c index 4038693..be80aed 100644 --- a/bin/varnishd/cache/cache_vrt.c +++ b/bin/varnishd/cache/cache_vrt.c @@ -357,7 +357,7 @@ VRT_BOOL_string(unsigned val) /*--------------------------------------------------------------------*/ void -VRT_Rollback(const struct vrt_ctx *ctx) +VRT_Rollback_req(const struct vrt_ctx *ctx) { CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC); @@ -366,6 +366,28 @@ VRT_Rollback(const struct vrt_ctx *ctx) WS_Reset(ctx->req->ws, ctx->req->ws_req); } +void +VRT_Rollback_bereq(const struct vrt_ctx *ctx) +{ + struct busyobj *bo; + + CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC); + bo = ctx->bo; + CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC); + + HTTP_Copy(bo->bereq, bo->bereq0); + WS_Reset(bo->bereq->ws, bo->ws_snap); + WS_Reset(bo->ws, bo->ws_snap); +} + +/* XXX deprecated - remove with next MAJOR version bump */ +void +VRT_Rollback(const struct vrt_ctx *ctx) +{ + return VRT_Rollback_req(ctx); +} + + /*--------------------------------------------------------------------*/ void diff --git a/bin/varnishtest/tests/c00032.vtc b/bin/varnishtest/tests/c00032.vtc index d5d06e3..1c1ef89 100644 --- a/bin/varnishtest/tests/c00032.vtc +++ b/bin/varnishtest/tests/c00032.vtc @@ -1,4 +1,4 @@ -varnishtest "Test Rollback" +varnishtest "Test Rollback_req" server s1 { @@ -22,7 +22,7 @@ varnish v1 -vcl+backend { sub vcl_deliver { if (resp.status == 400) { - rollback; + rollback_req; set req.url = "/bar"; return (restart); } diff --git a/bin/varnishtest/tests/c00069.vtc b/bin/varnishtest/tests/c00069.vtc new file mode 100644 index 0000000..7ba2827 --- /dev/null +++ b/bin/varnishtest/tests/c00069.vtc @@ -0,0 +1,35 @@ +varnishtest "Test Rollback_bereq" + + +server s1 { + rxreq + expect req.url == "/foo" + expect req.http.foobar == "harck-coff" + txresp -status 400 + accept + rxreq + expect req.url == "/bar" + expect req.http.foobar == "snark" + txresp -bodylen 5 +} -start + +varnish v1 -vcl+backend { + sub vcl_backend_fetch { + if (bereq.url == "/foo") { + set bereq.http.foobar = "harck-coff"; + } + } + + sub vcl_backend_response { + if (beresp.status == 400) { + rollback_bereq; + set bereq.url = "/bar"; + return (retry); + } + } +} -start + +client c1 { + txreq -url "/foo" -hdr "foobar: snark" + rxresp +} -run diff --git a/bin/varnishtest/tests/r01168.vtc b/bin/varnishtest/tests/r01168.vtc index e26b2ce..af9841a 100644 --- a/bin/varnishtest/tests/r01168.vtc +++ b/bin/varnishtest/tests/r01168.vtc @@ -1,4 +1,4 @@ -varnishtest "Test ESI rollback interaction" +varnishtest "Test ESI rollback_req interaction" server s1 { rxreq @@ -12,7 +12,7 @@ server s1 { varnish v1 -vcl+backend { sub vcl_recv { - rollback; + rollback_req; } sub vcl_backend_response { set beresp.do_esi = true; diff --git a/bin/varnishtest/tests/r01512.vtc b/bin/varnishtest/tests/r01512.vtc new file mode 100644 index 0000000..2c9e802 --- /dev/null +++ b/bin/varnishtest/tests/r01512.vtc @@ -0,0 +1,33 @@ +varnishtest "bereq changes lost between v_b_r and v_b_f" + +server s1 { + rxreq + txresp -status 700 + accept + rxreq + txresp +} -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 { + # return (abandon) should come here + set resp.status = 701; + } +} -start + +client c1 { + txreq + rxresp + expect resp.status == 701 +} -run diff --git a/bin/varnishtest/tests/v00018.vtc b/bin/varnishtest/tests/v00018.vtc index aae3723..04d0a67 100644 --- a/bin/varnishtest/tests/v00018.vtc +++ b/bin/varnishtest/tests/v00018.vtc @@ -3,7 +3,7 @@ varnishtest "VCL compiler coverage test: vcc_action.c" varnish v1 -vcl { backend b { .host = "127.0.0.1"; } sub vcl_hit { return (restart) ; } - sub vcl_miss { rollback; return (restart); } + sub vcl_miss { rollback_req; return (restart); } } varnish v1 -vcl { diff --git a/doc/sphinx/reference/index.rst b/doc/sphinx/reference/index.rst index 32458a6..b0818cf 100644 --- a/doc/sphinx/reference/index.rst +++ b/doc/sphinx/reference/index.rst @@ -53,7 +53,8 @@ The Varnish Reference Manual . - set . - unset . - esi - . - rollback + . - rollback_req + . - rollback_bereq Varnishtest . syntax etc. Shared Memory diff --git a/doc/sphinx/reference/vcl.rst b/doc/sphinx/reference/vcl.rst index 743689e..8411175 100644 --- a/doc/sphinx/reference/vcl.rst +++ b/doc/sphinx/reference/vcl.rst @@ -368,8 +368,17 @@ return() End execution of the current VCL subroutine, and continue to the next step in the request handling state machine. +rollback_req() + Restore request HTTP headers to their original state (only valid in + client side VCL subs). + +rollback_bereq() + Restore backend request HTTP headers to their original state (only + valid in backend VCL subs). + rollback() - Restore request HTTP headers to their original state. + Deprecated. Use rollback_req() instead. This function will get + removed in a future Varnish version. synthetic(STRING) Prepare a synthetic response body containing the STRING. Available in diff --git a/include/vrt.h b/include/vrt.h index 066d6c8..8010cd6 100644 --- a/include/vrt.h +++ b/include/vrt.h @@ -37,11 +37,15 @@ * Whenever something is added, increment MINOR version * Whenever something is deleted or changed in a way which is not * binary/load-time compatible, increment MAJOR version + * + * XXX TODO WHEN INCREMENTING MAJOR versoin + * - remove VRT_Rollback + * */ #define VRT_MAJOR_VERSION 1U -#define VRT_MINOR_VERSION 1U +#define VRT_MINOR_VERSION 2U /***********************************************************************/ @@ -247,6 +251,9 @@ 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_req(const struct vrt_ctx *); +void VRT_Rollback_bereq(const struct vrt_ctx *); +/* XXX deprecated - remove with next MAJOR version bump */ void VRT_Rollback(const struct vrt_ctx *); /* Synthetic pages */ diff --git a/lib/libvcc/vcc_action.c b/lib/libvcc/vcc_action.c index 9b1d22d..df66029 100644 --- a/lib/libvcc/vcc_action.c +++ b/lib/libvcc/vcc_action.c @@ -372,11 +372,18 @@ parse_return(struct vcc *tl) /*--------------------------------------------------------------------*/ static void -parse_rollback(struct vcc *tl) +parse_rollback_req(struct vcc *tl) { vcc_NextToken(tl); - Fb(tl, 1, "VRT_Rollback(ctx);\n"); + Fb(tl, 1, "VRT_Rollback_req(ctx);\n"); +} +static void +parse_rollback_bereq(struct vcc *tl) +{ + + vcc_NextToken(tl); + Fb(tl, 1, "VRT_Rollback_bereq(ctx);\n"); } /*--------------------------------------------------------------------*/ @@ -415,7 +422,10 @@ static struct action_table { { "hash_data", parse_hash_data, VCL_MET_HASH }, { "new", parse_new, VCL_MET_INIT}, { "return", parse_return }, - { "rollback", parse_rollback }, + { "rollback_bereq", parse_rollback_bereq, VCL_MET_MASK_BACKEND }, + { "rollback_req", parse_rollback_req, VCL_MET_MASK_CLIENT }, + /* XXX deprecated - remove with next MAJOR version bump */ + { "rollback", parse_rollback_req, VCL_MET_MASK_CLIENT }, { "set", parse_set }, { "synthetic", parse_synthetic, VCL_MET_SYNTH | VCL_MET_BACKEND_ERROR }, -- 2.0.1
_______________________________________________ varnish-dev mailing list [email protected] https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
