[master] ab7a5174d Re-Setup the response after rollback
Nils Goroll
nils.goroll at uplex.de
Wed Oct 30 15:07:08 UTC 2019
commit ab7a5174d5be93c60a1bd1dd84efc9459af5f11a
Author: Nils Goroll <nils.goroll at uplex.de>
Date: Mon Oct 7 18:09:19 2019 +0200
Re-Setup the response after rollback
Fixes #3083
to get there, also centralize req->resp setup for deliver and synth:
No semantic changes other than some reordering, which also fixes an odd
log line ordering as shown by the change to c00018.vtc
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index 6fdce85a2..977f57e39 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -104,6 +104,96 @@ cnt_transport(struct worker *wrk, struct req *req)
* Deliver an object to client
*/
+static int
+resp_setup_deliver(struct req *req)
+{
+ struct http *h;
+ struct objcore *oc;
+
+ CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
+ oc = req->objcore;
+ CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
+
+ h = req->resp;
+
+ HTTP_Setup(h, req->ws, req->vsl, SLT_RespMethod);
+
+ if (HTTP_Decode(h, ObjGetAttr(req->wrk, oc, OA_HEADERS, NULL)))
+ return (-1);
+
+ http_ForceField(h, HTTP_HDR_PROTO, "HTTP/1.1");
+
+ if (req->is_hit)
+ http_PrintfHeader(h, "X-Varnish: %u %u", VXID(req->vsl->wid),
+ ObjGetXID(req->wrk, oc));
+ else
+ http_PrintfHeader(h, "X-Varnish: %u", VXID(req->vsl->wid));
+
+ /*
+ * We base Age calculation upon the last timestamp taken during client
+ * request processing. This gives some inaccuracy, but since Age is only
+ * full second resolution that shouldn't matter. (Last request timestamp
+ * could be a Start timestamp taken before the object entered into cache
+ * leading to negative age. Truncate to zero in that case).
+ */
+ http_PrintfHeader(h, "Age: %.0f",
+ floor(fmax(0., req->t_prev - oc->t_origin)));
+
+ http_SetHeader(h, "Via: 1.1 varnish (Varnish/6.3)");
+
+ if (cache_param->http_gzip_support &&
+ ObjCheckFlag(req->wrk, oc, OF_GZIPED) &&
+ !RFC2616_Req_Gzip(req->http))
+ RFC2616_Weaken_Etag(h);
+
+ return (0);
+}
+
+static int
+resp_setup_synth(struct req *req)
+{
+ struct http *h;
+
+ CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
+
+ h = req->resp;
+
+ HTTP_Setup(h, req->ws, req->vsl, SLT_RespMethod);
+
+ AZ(req->objcore);
+ http_PutResponse(h, "HTTP/1.1", req->err_code, req->err_reason);
+
+ http_TimeHeader(h, "Date: ", W_TIM_real(req->wrk));
+ http_SetHeader(h, "Server: Varnish");
+ http_PrintfHeader(h, "X-Varnish: %u",
+ VXID(req->vsl->wid));
+
+ /*
+ * For late 100-continue, we suggest to VCL to close the connection to
+ * neither send a 100-continue nor drain-read the request. But VCL has
+ * the option to veto by removing Connection: close
+ */
+ if (req->want100cont)
+ http_SetHeader(h, "Connection: close");
+
+ return (0);
+}
+
+int
+Resp_Setup(struct req *req, unsigned method)
+{
+ switch (method) {
+ case VCL_MET_DELIVER:
+ return (resp_setup_deliver(req));
+ case VCL_MET_SYNTH:
+ return (resp_setup_synth(req));
+ default:
+ WRONG("vcl method");
+ }
+
+ return (0);
+}
+
static enum req_fsm_nxt
cnt_deliver(struct worker *wrk, struct req *req)
{
@@ -119,41 +209,12 @@ cnt_deliver(struct worker *wrk, struct req *req)
ObjTouch(req->wrk, req->objcore, req->t_prev);
- HTTP_Setup(req->resp, req->ws, req->vsl, SLT_RespMethod);
- if (HTTP_Decode(req->resp,
- ObjGetAttr(req->wrk, req->objcore, OA_HEADERS, NULL))) {
+ if (resp_setup_deliver(req)) {
(void)HSH_DerefObjCore(wrk, &req->objcore, HSH_RUSH_POLICY);
req->err_code = 500;
req->req_step = R_STP_SYNTH;
return (REQ_FSM_MORE);
}
- http_ForceField(req->resp, HTTP_HDR_PROTO, "HTTP/1.1");
-
- if (req->is_hit)
- http_PrintfHeader(req->resp,
- "X-Varnish: %u %u", VXID(req->vsl->wid),
- ObjGetXID(wrk, req->objcore));
- else
- http_PrintfHeader(req->resp,
- "X-Varnish: %u", VXID(req->vsl->wid));
-
- /*
- * We base Age calculation upon the last timestamp taken during
- * client request processing. This gives some inaccuracy, but
- * since Age is only full second resolution that shouldn't
- * matter. (Last request timestamp could be a Start timestamp
- * taken before the object entered into cache leading to negative
- * age. Truncate to zero in that case).
- */
- http_PrintfHeader(req->resp, "Age: %.0f",
- floor(fmax(0., req->t_prev - req->objcore->t_origin)));
-
- http_SetHeader(req->resp, "Via: 1.1 varnish (Varnish/6.3)");
-
- if (cache_param->http_gzip_support &&
- ObjCheckFlag(req->wrk, req->objcore, OF_GZIPED) &&
- !RFC2616_Req_Gzip(req->http))
- RFC2616_Weaken_Etag(req->resp);
VCL_deliver_method(req->vcl, wrk, req, NULL, NULL);
VSLb_ts_req(req, "Process", W_TIM_real(wrk));
@@ -221,8 +282,6 @@ cnt_vclfail(const struct worker *wrk, struct req *req)
static enum req_fsm_nxt
cnt_synth(struct worker *wrk, struct req *req)
{
- struct http *h;
- double now;
struct vsb *synth_body;
ssize_t sz, szl;
uint8_t *ptr;
@@ -235,27 +294,12 @@ cnt_synth(struct worker *wrk, struct req *req)
wrk->stats->s_synth++;
- now = W_TIM_real(wrk);
- VSLb_ts_req(req, "Process", now);
+ VSLb_ts_req(req, "Process", W_TIM_real(wrk));
if (req->err_code < 100)
req->err_code = 501;
- HTTP_Setup(req->resp, req->ws, req->vsl, SLT_RespMethod);
- h = req->resp;
- http_TimeHeader(h, "Date: ", now);
- http_SetHeader(h, "Server: Varnish");
- http_PrintfHeader(req->resp, "X-Varnish: %u", VXID(req->vsl->wid));
- http_PutResponse(h, "HTTP/1.1", req->err_code, req->err_reason);
-
- /*
- * For late 100-continue, we suggest to VCL to close the connection to
- * neither send a 100-continue nor drain-read the request. But VCL has
- * the option to veto by removing Connection: close
- */
- if (req->want100cont) {
- http_SetHeader(h, "Connection: close");
- }
+ (void) resp_setup_synth(req);
synth_body = VSB_new_auto();
AN(synth_body);
@@ -281,14 +325,14 @@ cnt_synth(struct worker *wrk, struct req *req)
* XXX: Should we reset req->doclose = SC_VCL_FAILURE
* XXX: If so, to what ?
*/
- HTTP_Setup(h, req->ws, req->vsl, SLT_RespMethod);
+ HTTP_Setup(req->resp, req->ws, req->vsl, SLT_RespMethod);
VSB_destroy(&synth_body);
req->req_step = R_STP_RESTART;
return (REQ_FSM_MORE);
}
assert(wrk->handling == VCL_RET_DELIVER);
- http_Unset(h, H_Content_Length);
+ http_Unset(req->resp, H_Content_Length);
http_PrintfHeader(req->resp, "Content-Length: %zd",
VSB_len(synth_body));
diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h
index 0fb461c61..6a1ace8aa 100644
--- a/bin/varnishd/cache/cache_varnishd.h
+++ b/bin/varnishd/cache/cache_varnishd.h
@@ -360,6 +360,8 @@ void VRB_Free(struct req *);
/* cache_req_fsm.c [CNT] */
+int Resp_Setup(struct req *, unsigned);
+
enum req_fsm_nxt {
REQ_FSM_MORE,
REQ_FSM_DONE,
diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index 4ec470ea0..0fa600f2a 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -747,6 +747,8 @@ VRT_Rollback(VRT_CTX, VCL_HTTP hp)
if (hp == ctx->http_req) {
CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC);
Req_Rollback(ctx->req);
+ if ((ctx->method & (VCL_MET_DELIVER | VCL_MET_SYNTH)) != 0)
+ Resp_Setup(ctx->req, ctx->method);
} else if (hp == ctx->http_bereq) {
CHECK_OBJ_NOTNULL(ctx->bo, BUSYOBJ_MAGIC);
Bereq_Rollback(ctx->bo);
diff --git a/bin/varnishtest/tests/c00018.vtc b/bin/varnishtest/tests/c00018.vtc
index 7a27755ff..10359f758 100644
--- a/bin/varnishtest/tests/c00018.vtc
+++ b/bin/varnishtest/tests/c00018.vtc
@@ -123,11 +123,13 @@ logexpect l1 -v v1 -g raw {
expect 0 1011 VCL_call {^HASH$}
expect 0 1011 VCL_return {^lookup$}
expect 0 1011 Timestamp {^Process:}
- expect 0 1011 RespHeader {^Date:}
- expect 0 1011 RespHeader {^Server: Varnish$}
- expect 0 1011 RespHeader {^X-Varnish: 1011$}
expect 0 1011 RespProtocol {^HTTP/1.1$}
expect 0 1011 RespStatus {^405$}
+ expect 0 1011 RespReason {^Method Not Allowed$}
+ # XXX dup RespReason
+ expect 1 1011 RespHeader {^Date:}
+ expect 0 1011 RespHeader {^Server: Varnish$}
+ expect 0 1011 RespHeader {^X-Varnish: 1011$}
} -start
More information about the varnish-commit
mailing list