Skip to content

Commit

Permalink
varnishncsa: Change matching rules to reflect reality
Browse files Browse the repository at this point in the history
With this change, [Be]rsp/[Be]req formats should match exactly what was
received/sent from/to the backend/client without taking irrelevant VCL
changes into account.

Note that this is a breaking change in varnishncsa's default behavior.
Refs: varnishcache#3528
  • Loading branch information
walid-git committed Oct 11, 2024
1 parent 7f5fed1 commit 7468e9a
Show file tree
Hide file tree
Showing 2 changed files with 195 additions and 10 deletions.
42 changes: 32 additions & 10 deletions bin/varnishncsa/varnishncsa.c
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,21 @@ static struct ctx {
} CTX;

enum update_mode {
UM_NEVER,
UM_IF_NOT_SET,
UM_ALWAYS
};

#define UPDATE_REQ_C(CTX) (CTX.vcl_recv_seen ? UM_NEVER : UM_ALWAYS)
#define UPDATE_REQ_B(CTX) (CTX.vcl_br_seen ? UM_NEVER : UM_ALWAYS)

#define UPDATE_REQ(CTX) (*CTX.side == 'b' ? UPDATE_REQ_B(CTX) : UPDATE_REQ_C(CTX))

#define UPDATE_RESP_C(CTX) (UM_ALWAYS)
#define UPDATE_RESP_B(CTX) (CTX.vcl_br_seen ? UM_NEVER : UM_ALWAYS)

#define UPDATE_RESP(CTX) (*CTX.side == 'b' ? UPDATE_RESP_B(CTX) : UPDATE_RESP_C(CTX))

static void parse_format(const char *format);

static void
Expand Down Expand Up @@ -889,7 +900,8 @@ frag_fields(enum update_mode um, const char *b, const char *e, ...)
assert(p != NULL && q != NULL);
if (p >= e || q <= p)
continue;
if (frag->gen != CTX.gen || um == UM_ALWAYS) {
if ((frag->gen != CTX.gen && um == UM_IF_NOT_SET)
|| um == UM_ALWAYS) {
/* We only grab the same matching field once */
frag->gen = CTX.gen;
frag->b = p;
Expand All @@ -902,6 +914,9 @@ frag_fields(enum update_mode um, const char *b, const char *e, ...)
static void
frag_line(enum update_mode um, const char *b, const char *e, struct fragment *f)
{
if (um == UM_NEVER)
return;

if (f->gen == CTX.gen && um == UM_IF_NOT_SET)
/* We only grab the same matching record once */
return;
Expand Down Expand Up @@ -1019,23 +1034,28 @@ dispatch_f(struct VSL_data *vsl, struct VSL_transaction * const pt[],
break;
case SLT_BereqMethod:
case SLT_ReqMethod:
frag_line(UM_IF_NOT_SET, b, e, &CTX.frag[F_m]);
frag_line(UPDATE_REQ(CTX),
b, e, &CTX.frag[F_m]);
break;
case SLT_BereqURL:
case SLT_ReqURL:
p = memchr(b, '?', e - b);
if (p == NULL)
p = e;
frag_line(UM_IF_NOT_SET, b, p, &CTX.frag[F_U]);
frag_line(UM_IF_NOT_SET, p, e, &CTX.frag[F_q]);
frag_line(UPDATE_REQ(CTX),
b, p, &CTX.frag[F_U]);
frag_line(UPDATE_REQ(CTX),
p, e, &CTX.frag[F_q]);
break;
case SLT_BereqProtocol:
case SLT_ReqProtocol:
frag_line(UM_IF_NOT_SET, b, e, &CTX.frag[F_H]);
frag_line(UPDATE_REQ(CTX),
b, e, &CTX.frag[F_H]);
break;
case SLT_BerespStatus:
case SLT_RespStatus:
frag_line(UM_ALWAYS, b, e, &CTX.frag[F_s]);
frag_line(UPDATE_RESP(CTX),
b, e, &CTX.frag[F_s]);
break;
case SLT_BereqAcct:
case SLT_ReqAcct:
Expand Down Expand Up @@ -1066,19 +1086,21 @@ dispatch_f(struct VSL_data *vsl, struct VSL_transaction * const pt[],
break;
case SLT_BereqHeader:
case SLT_ReqHeader:
process_hdr(&CTX.watch_reqhdr, b, e, UM_ALWAYS);
process_hdr(&CTX.watch_reqhdr, b, e,
UPDATE_REQ(CTX));
if (ISPREFIX("Authorization:", b, e, &p) &&
ISPREFIX("basic ", p, e, &p))
frag_line(UM_IF_NOT_SET, p, e,
frag_line(UPDATE_REQ(CTX), p, e,
&CTX.frag[F_auth]);
else if (ISPREFIX("Host:", b, e, &p))
frag_line(UM_IF_NOT_SET, p, e,
frag_line(UPDATE_REQ(CTX), p, e,
&CTX.frag[F_host]);
#undef ISPREFIX
break;
case SLT_BerespHeader:
case SLT_RespHeader:
process_hdr(&CTX.watch_resphdr, b, e, UM_ALWAYS);
process_hdr(&CTX.watch_resphdr, b, e,
UPDATE_RESP(CTX));
break;
case SLT_VCL_call:
if (!strcasecmp(b, "recv")) {
Expand Down
163 changes: 163 additions & 0 deletions bin/varnishtest/tests/u00020.vtc
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
varnishtest "new varnishncsa matching rules"

# Test things we send to the backend

server s1 {
rxreq
txresp
} -start

varnish v1 -vcl+backend {

sub vcl_backend_fetch {
set bereq.http.bereqhdr = "vbf-modified";
set bereq.method = "HEAD";
set bereq.url = "/vbf-url?q=vbfQuerry";
set bereq.http.Authorization = "basic dmJmOnBhc3M=";
}

sub vcl_backend_response {
set bereq.http.bereqhdr = "vbr-modified";
set bereq.http.notsent = "notsent";
set bereq.method = "CONNECT";
set bereq.url = "/vbr-url?q=vbrQuerry";
set bereq.http.Authorization = "basic dmJyOnBhc3M=";
}

} -start


client c1 {
txreq -url "/client-url?q=clientQuerry" -hdr "bereqhdr: client-header" -hdr "Authorization:basic Y2xpZW50OnBhc3M="
rxresp
} -run

shell {
varnishncsa -n ${v1_name} -d -b -F '%H %{bereqhdr}i %{notsent}i %m %q %U %u' > ncsa_sb.txt

cat >expected_sb.txt <<-EOF
HTTP/1.1 vbf-modified - HEAD ?q=vbfQuerry /vbf-url vbf
EOF
diff -u expected_sb.txt ncsa_sb.txt
}

varnish v1 -stop

# Test things we receive from the backend

server s1 {
rxreq
txresp -status 202 -hdr "beresp: origin"
} -start

varnish v1 -vcl+backend {

sub vcl_backend_response {
set beresp.http.beresp = "vbr-updated";
set beresp.status = 200;
}

} -start


client c1 {
txreq
rxresp
} -run


shell {
varnishncsa -n ${v1_name} -d -b -F '%s %{beresp}o' > ncsa_rb.txt

cat >expected_rb.txt <<-EOF
202 origin
EOF
diff -u expected_rb.txt ncsa_rb.txt
}

varnish v1 -stop

# Test things we send to the client

server s1 {
rxreq
txresp -status 202 -hdr "resp: origin"
} -start

varnish v1 -vcl+backend {

sub vcl_backend_response {
set beresp.http.resp = "vbr-updated";
set beresp.status = 200;
}

sub vcl_deliver {
set resp.http.resp = "deliver-updated";
set resp.status = 201;
set resp.http.added = "deliver";
}

} -start


client c1 {
txreq
rxresp
} -run

shell {
varnishncsa -n ${v1_name} -d -c -F '%s %{resp}o %{added}o' > ncsa_sc.txt

cat >expected_sc.txt <<-EOF
201 deliver-updated deliver
EOF
diff -u expected_sc.txt ncsa_sc.txt
}

varnish v1 -stop

# Test things we receive from the client

server s1 {
rxreq
txresp
} -start

varnish v1 -vcl+backend {

sub vcl_recv {
set req.http.reqhdr = "recv-modified";
set req.method = "HEAD";
set req.url = "/recv-url?q=recvQuerry";
set req.http.Authorization = "basic cmVjdjpwYXNz";
set req.http.notreceived = "recv";
}

sub vcl_hash {
set req.http.reqhdr = "hash-modified";
set req.method = "GET";
set req.url = "/hash-url?q=hashQuerry";
set req.http.Authorization = "basic aGFzaDpwYXNz";
set req.http.notreceived = "hash";
}

} -start


client c1 {
txreq -req "POST" -url "/client-url?q=clientQuerry" \
-hdr "reqhdr: client-header" \
-hdr "Authorization:basic Y2xpZW50OnBhc3M="
rxresp
} -run


shell {
varnishncsa -n ${v1_name} -d -c -F '%H %{reqhdr}i %{notreceived}i %m %q %U %u' > ncsa_rc.txt

cat >expected_rc.txt <<-EOF
HTTP/1.1 client-header - POST ?q=clientQuerry /client-url client
EOF
diff -u expected_rc.txt ncsa_rc.txt
}
# TODO: Handle Unset

0 comments on commit 7468e9a

Please sign in to comment.