Skip to content

Commit

Permalink
varnishncsa: Change formats matching rules to better reflect reality
Browse files Browse the repository at this point in the history
With this change, [Be]resp/[Be]req formats should reflect better what was
received/sent from/to the backend/client without taking irrelevant VCL
changes into account. There is however still an exception for the changes
performed by the core code before vcl_recv for req and before
vcl_backend_response for beresp that cannot be distinguished from the
original headers.

Refs: varnishcache#3528
  • Loading branch information
walid-git committed Oct 31, 2024
1 parent 96384b2 commit ddb6951
Show file tree
Hide file tree
Showing 2 changed files with 224 additions and 25 deletions.
86 changes: 61 additions & 25 deletions bin/varnishncsa/varnishncsa.c
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,42 @@ static struct ctx {
int recv_compl;
} CTX;

enum format_policy {
FMTPOL_FIRST = 0,
FMTPOL_REQ,
FMTPOL_RESP,
};

static void parse_format(const char *format);

static unsigned
process_format_policy(enum format_policy fp, unsigned first)
{
unsigned policy;

if (fp == FMTPOL_FIRST)
return (first);

#define BACKEND_FLAG 2<<8
policy = fp | ((*CTX.side == 'c') ? 0 : BACKEND_FLAG);

switch (policy) {
case FMTPOL_REQ:
case FMTPOL_RESP | BACKEND_FLAG:
if (CTX.recv_compl)
return (0);
return (first);
case FMTPOL_REQ | BACKEND_FLAG:
if (CTX.recv_compl)
return (0);
return (1);
case FMTPOL_RESP:
return (1);
default:
WRONG("Illegal enum value");
}
}

static void
openout(int append)
{
Expand Down Expand Up @@ -851,7 +885,7 @@ isprefix(const char *prefix, size_t len, const char *b,
}

static void
frag_fields(int force, const char *b, const char *e, ...)
frag_fields(enum format_policy fp, const char *b, const char *e, ...)
{
va_list ap;
const char *p, *q;
Expand Down Expand Up @@ -883,7 +917,7 @@ frag_fields(int force, const char *b, const char *e, ...)
assert(p != NULL && q != NULL);
if (p >= e || q <= p)
continue;
if (frag->gen != CTX.gen || force) {
if (process_format_policy(fp, frag->gen != CTX.gen)) {
/* We only grab the same matching field once */
frag->gen = CTX.gen;
frag->b = p;
Expand All @@ -894,10 +928,11 @@ frag_fields(int force, const char *b, const char *e, ...)
}

static void
frag_line(int force, const char *b, const char *e, struct fragment *f)
frag_line(enum format_policy fp, const char *b, const char *e,
struct fragment *f)
{

if (f->gen == CTX.gen && !force)
if (!process_format_policy(fp, f->gen != CTX.gen))
/* We only grab the same matching record once */
return;

Expand All @@ -915,7 +950,8 @@ frag_line(int force, const char *b, const char *e, struct fragment *f)
}

static void
process_hdr(const struct watch_head *head, const char *b, const char *e)
process_hdr(enum format_policy fp, const struct watch_head *head, const char *b,
const char *e)
{
struct watch *w;
const char *p;
Expand All @@ -924,7 +960,7 @@ process_hdr(const struct watch_head *head, const char *b, const char *e)
CHECK_OBJ_NOTNULL(w, WATCH_MAGIC);
if (!isprefix(w->key, w->keylen, b, e, &p))
continue;
frag_line(1, p, e, &w->frag);
frag_line(fp, p, e, &w->frag);
}
}

Expand All @@ -943,9 +979,9 @@ process_vsl(const struct vsl_watch_head *head, enum VSL_tag_e tag,
!isprefix(w->prefix, w->prefixlen, b, e, &p))
continue;
if (w->idx == 0)
frag_line(0, p, e, &w->frag);
frag_line(FMTPOL_FIRST, p, e, &w->frag);
else
frag_fields(0, p, e, w->idx, &w->frag, 0, NULL);
frag_fields(FMTPOL_FIRST, p, e, w->idx, &w->frag, 0, NULL);
}
}

Expand Down Expand Up @@ -995,44 +1031,44 @@ dispatch_f(struct VSL_data *vsl, struct VSL_transaction * const pt[],
skip = 1;
break;
case SLT_PipeAcct:
frag_fields(0, b, e,
frag_fields(FMTPOL_FIRST, b, e,
3, &CTX.frag[F_I],
4, &CTX.frag[F_O],
0, NULL);
break;
case SLT_BackendOpen:
frag_fields(1, b, e,
frag_fields(FMTPOL_FIRST, b, e,
3, &CTX.frag[F_h],
0, NULL);
break;
case SLT_ReqStart:
frag_fields(0, b, e,
frag_fields(FMTPOL_FIRST, b, e,
1, &CTX.frag[F_h],
0, NULL);
break;
case SLT_BereqMethod:
case SLT_ReqMethod:
frag_line(0, b, e, &CTX.frag[F_m]);
frag_line(FMTPOL_REQ, 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(0, b, p, &CTX.frag[F_U]);
frag_line(0, p, e, &CTX.frag[F_q]);
frag_line(FMTPOL_REQ, b, p, &CTX.frag[F_U]);
frag_line(FMTPOL_REQ, p, e, &CTX.frag[F_q]);
break;
case SLT_BereqProtocol:
case SLT_ReqProtocol:
frag_line(0, b, e, &CTX.frag[F_H]);
frag_line(FMTPOL_REQ, b, e, &CTX.frag[F_H]);
break;
case SLT_BerespStatus:
case SLT_RespStatus:
frag_line(1, b, e, &CTX.frag[F_s]);
frag_line(FMTPOL_RESP, b, e, &CTX.frag[F_s]);
break;
case SLT_BereqAcct:
case SLT_ReqAcct:
frag_fields(0, b, e,
frag_fields(FMTPOL_FIRST, b, e,
3, &CTX.frag[F_I],
5, &CTX.frag[F_b],
6, &CTX.frag[F_O],
Expand All @@ -1041,37 +1077,37 @@ dispatch_f(struct VSL_data *vsl, struct VSL_transaction * const pt[],
case SLT_Timestamp:
#define ISPREFIX(a, b, c, d) isprefix(a, strlen(a), b, c, d)
if (ISPREFIX("Start:", b, e, &p)) {
frag_fields(0, p, e, 1,
frag_fields(FMTPOL_FIRST, p, e, 1,
&CTX.frag[F_tstart], 0, NULL);

} else if (ISPREFIX("Resp:", b, e, &p) ||
ISPREFIX("PipeSess:", b, e, &p) ||
ISPREFIX("BerespBody:", b, e, &p)) {
frag_fields(0, p, e, 1,
frag_fields(FMTPOL_FIRST, p, e, 1,
&CTX.frag[F_tend], 0, NULL);

} else if (ISPREFIX("Process:", b, e, &p) ||
ISPREFIX("Pipe:", b, e, &p) ||
ISPREFIX("Beresp:", b, e, &p)) {
frag_fields(0, p, e, 2,
frag_fields(FMTPOL_FIRST, p, e, 2,
&CTX.frag[F_ttfb], 0, NULL);
}
break;
case SLT_BereqHeader:
case SLT_ReqHeader:
process_hdr(&CTX.watch_reqhdr, b, e);
process_hdr(FMTPOL_REQ, &CTX.watch_reqhdr, b, e);
if (ISPREFIX("Authorization:", b, e, &p) &&
ISPREFIX("basic ", p, e, &p))
frag_line(0, p, e,
frag_line(FMTPOL_REQ, p, e,
&CTX.frag[F_auth]);
else if (ISPREFIX("Host:", b, e, &p))
frag_line(0, p, e,
frag_line(FMTPOL_REQ, p, e,
&CTX.frag[F_host]);
#undef ISPREFIX
break;
case SLT_BerespHeader:
case SLT_RespHeader:
process_hdr(&CTX.watch_resphdr, b, e);
process_hdr(FMTPOL_RESP, &CTX.watch_resphdr, b, e);
break;
case SLT_VCL_call:
if (!strcasecmp(b, "recv")) {
Expand Down Expand Up @@ -1111,7 +1147,7 @@ dispatch_f(struct VSL_data *vsl, struct VSL_transaction * const pt[],
strncmp(b, w->key, w->keylen))
continue;
p = b + w->keylen;
frag_line(0, p, e, &w->frag);
frag_line(FMTPOL_FIRST, p, e, &w->frag);
}
break;
default:
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 ddb6951

Please sign in to comment.