Commit a45e1f4e authored by Martin Blix Grydeland's avatar Martin Blix Grydeland Committed by Reza Naghibi

Fail fetch retries when uncached request body has been released

Currently we allow fetch retries with body even after we have released the
request that initiated the fetch, and the request body with it. The
attached test case demonstrates this, where s2 on the retry attempt gets
stuck waiting for 3 bytes of body data that is never sent.

Fix this by keeping track of what the initial request body status was, and
failing the retry attempt if the request was already released
(BOS_REQ_DONE) and the request body was not cached.

Conflicts:
    bin/varnishd/cache/cache.h
parent 346475f4
......@@ -390,7 +390,8 @@ struct busyobj {
* All fields from retries and down are zeroed when the busyobj
* is recycled.
*/
int retries;
unsigned retries;
enum req_body_state_e initial_req_body_status;
struct req *req;
struct sess *sp;
struct worker *wrk;
......
......@@ -242,6 +242,7 @@ vbf_stp_mkbereq(struct worker *wrk, struct busyobj *bo)
bo->ws_bo = WS_Snapshot(bo->ws);
HTTP_Copy(bo->bereq, bo->bereq0);
bo->initial_req_body_status = bo->req->req_body_status;
if (bo->req->req_body_status == REQ_BODY_NONE) {
bo->req = NULL;
ObjSetState(bo->wrk, bo->fetch_objcore, BOS_REQ_DONE);
......@@ -262,6 +263,15 @@ vbf_stp_retry(struct worker *wrk, struct busyobj *bo)
assert(bo->fetch_objcore->boc->state <= BOS_REQ_DONE);
if (bo->fetch_objcore->boc->state == BOS_REQ_DONE &&
bo->initial_req_body_status != REQ_BODY_NONE &&
bo->bereq_body == NULL) {
/* We have already released the req and there was a
* request body that was not cached. Too late to retry. */
VSLb(bo->vsl, SLT_Error, "req.body already consumed");
return (F_STP_FAIL);
}
VSLb_ts_busyobj(bo, "Retry", W_TIM_real(wrk));
/* VDI_Finish (via vbf_cleanup) must have been called before */
......
varnishtest "r03093 - fail retry on missing req body"
barrier b1 sock 2
server s1 {
rxreq
expect req.method == POST
expect req.body == foo
txresp -nolen -hdr "Content-Length: 3"
barrier b1 sync
} -start
# In this test s2 should not be called. The attempt to retry should fail because
# the request was already released from the fetch thread in the first attempt.
server s2 {
rxreq
expect req.method == POST
expect req.body == foo
txresp -body bar
} -start
varnish v1 -arg "-p debug=+syncvsl" -vcl+backend {
import vtc;
sub vcl_backend_fetch {
set bereq.http.retries = bereq.retries;
if (bereq.retries == 1) {
set bereq.backend = s2;
}
}
sub vcl_backend_response {
if (bereq.http.retries == "0") {
vtc.barrier_sync("${b1_sock}");
}
set beresp.do_stream = false;
}
sub vcl_backend_error {
return (retry);
}
} -start
client c1 {
txreq -req POST -body foo
rxresp
expect resp.status == 503
} -run
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment