[master] 5a83b69 Rework how we infer types in the VCC expresion evaluator.
Poul-Henning Kamp
phk at FreeBSD.org
Tue Aug 26 23:04:17 CEST 2014
commit 5a83b69d8c5d6b08b6de2132df45531c8b7543f1
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Tue Aug 26 20:48:08 2014 +0000
Rework how we infer types in the VCC expresion evaluator.
This mainly (only ?) changes things when we ask for an expression
of type STRING or STTRING_LIST.
Previously addition and subtraction would follow the general rule,
"first argument determines intial type" and that would make an
expression like this fail:
set resp.http.server_port_foo = std.port(server.ip) + "_foo";
Because the addition tries to evaluate "INT + STRING".
The idea was that people would rewrite this as:
set resp.http.server_port_foo = "" + std.port(server.ip) + "_foo";
To make the first argument STRING and everything will then just work.
However, this is needlessly strict in the case where we are actively
desiring the expression to evaluate to STRING -- like above where
resp.http.* has type STRING.
With the new code, when an impossible addition is encountered, it
will look at the type requested of the expression, and if it is
STRING, convert the current subexpression to STRING and continue
adding.
A large number of subtests which are designed to fail started working
with this change, they have been fixed by not using *.http.* variables
as scaffolding..
You can still get into some weird corner-cases like the difference
between:
set resp.http.foobar = req.ttl + 1s + "_" + req.ttl + 1s;
and
set resp.http.foobar = req.ttl + 1s + "_" + req.ttl + 1;
and
set resp.http.foobar = req.ttl + 1s + "_" + (req.ttl + 1s);
(Hint: The last one is the only one which does what you think)
Fixes #1579
diff --git a/bin/varnishtest/tests/v00020.vtc b/bin/varnishtest/tests/v00020.vtc
index e63be3f..f9be7ec 100644
--- a/bin/varnishtest/tests/v00020.vtc
+++ b/bin/varnishtest/tests/v00020.vtc
@@ -74,7 +74,7 @@ varnish v1 -errvcl {Operator * not possible on type STRING.} {
varnish v1 -errvcl {DURATION + INT not possible.} {
sub vcl_backend_response {
- set req.http.foo = req.ttl + beresp.status;
+ set req.ttl = req.ttl + beresp.status;
}
}
@@ -85,7 +85,7 @@ varnish v1 -errvcl {'!' must be followed by BOOL, found DURATION.} {
}
}
-varnish v1 -errvcl {Operator + not possible on type BOOL.} {
+varnish v1 -errvcl {BOOL + BOOL not possible.} {
sub vcl_backend_response {
if (beresp.do_gzip + beresp.do_gunzip) {
}
@@ -111,7 +111,7 @@ varnish v1 -vcl {
}
# XXX: not the most clear error message
-varnish v1 -errvcl {Expected ';' got '-'} {
+varnish v1 -errvcl {STRING - STRING not possible.} {
backend b { .host = "127.0.0.1"; }
sub vcl_recv {
set req.http.foo = "foo" - "bar";
@@ -121,14 +121,14 @@ varnish v1 -errvcl {Expected ';' got '-'} {
varnish v1 -errvcl {TIME + STRING not possible.} {
backend b { .host = "127.0.0.1"; }
sub vcl_recv {
- set req.http.foo = now + "foo";
+ set req.ttl = now + "foo";
}
}
varnish v1 -errvcl {TIME + TIME not possible.} {
backend b { .host = "127.0.0.1"; }
sub vcl_recv {
- set req.http.foo = now + now;
+ set req.ttl = now + now;
}
}
@@ -142,15 +142,15 @@ varnish v1 -errvcl {Expected ID got ';'} {
varnish v1 -errvcl {INT + STRING not possible.} {
backend b { .host = "127.0.0.1"; }
- sub vcl_recv {
- set req.http.foo = 1 + "foo";
+ sub vcl_backend_response {
+ set beresp.status = 1 + "foo";
}
}
varnish v1 -errvcl {INT + TIME not possible.} {
backend b { .host = "127.0.0.1"; }
- sub vcl_recv {
- set req.http.foo = 1 + now;
+ sub vcl_backend_response {
+ set beresp.status = 1 + now;
}
}
diff --git a/bin/varnishtest/tests/v00025.vtc b/bin/varnishtest/tests/v00025.vtc
index 67ac41a..2ac4b9d 100644
--- a/bin/varnishtest/tests/v00025.vtc
+++ b/bin/varnishtest/tests/v00025.vtc
@@ -15,6 +15,9 @@ varnish v1 -arg "-i J.F.Nobody" -vcl+backend {
sub vcl_deliver {
set resp.http.server_port = std.port(server.ip);
+ set resp.http.server_port_foo = std.port(server.ip) + "_foo";
+ set resp.http.ttl1 = (req.ttl + 10s);
+ set resp.http.ttl2 = req.ttl + 10s;
set resp.http.id = server.identity;
set resp.http.esi = req.esi;
set resp.http.be = req.backend_hint;
diff --git a/lib/libvcc/vcc_expr.c b/lib/libvcc/vcc_expr.c
index c2f9818..c6c75fe 100644
--- a/lib/libvcc/vcc_expr.c
+++ b/lib/libvcc/vcc_expr.c
@@ -660,6 +660,7 @@ vcc_Eval_SymFunc(struct vcc *tl, struct expr **e, const struct symbol *sym)
* SYNTAX:
* Expr4:
* '(' Expr0 ')'
+ * symbol
* CNUM
* CSTR
*/
@@ -704,6 +705,11 @@ vcc_expr4(struct vcc *tl, struct expr **e, enum var_type fmt)
AN(sym->eval);
AZ(*e);
sym->eval(tl, e, sym);
+ /* Unless asked for a HEADER, fold to string here */
+ if (*e && fmt != HEADER && (*e)->fmt == HEADER) {
+ vcc_expr_tostring(tl, e, STRING);
+ ERRCHK(tl);
+ }
return;
default:
break;
@@ -822,16 +828,21 @@ vcc_expr_mul(struct vcc *tl, struct expr **e, enum var_type fmt)
*/
static void
-vcc_expr_string_add(struct vcc *tl, struct expr **e)
+vcc_expr_string_add(struct vcc *tl, struct expr **e, struct expr *e2)
{
- struct expr *e2;
enum var_type f2;
+ AN(e);
+ AN(*e);
+ AN(e2);
f2 = (*e)->fmt;
+ assert (f2 == STRING || f2 == STRING_LIST);
- while (tl->t->tok == '+') {
- vcc_NextToken(tl);
- vcc_expr_mul(tl, &e2, STRING);
+ while (e2 != NULL || tl->t->tok == '+') {
+ if (e2 == NULL) {
+ vcc_NextToken(tl);
+ vcc_expr_mul(tl, &e2, STRING);
+ }
ERRCHK(tl);
if (e2->fmt != STRING && e2->fmt != STRING_LIST) {
vcc_expr_tostring(tl, &e2, f2);
@@ -858,6 +869,7 @@ vcc_expr_string_add(struct vcc *tl, struct expr **e)
*e = vcc_expr_edit(STRING_LIST, "\v1,\n\v2", *e, e2);
(*e)->constant = EXPR_VAR;
}
+ e2 = NULL;
}
}
@@ -873,49 +885,36 @@ vcc_expr_add(struct vcc *tl, struct expr **e, enum var_type fmt)
ERRCHK(tl);
f2 = (*e)->fmt;
- /* Unless we specifically ask for a HEADER, fold them to string here */
- if (fmt != HEADER && f2 == HEADER) {
- vcc_expr_tostring(tl, e, STRING);
- ERRCHK(tl);
- f2 = (*e)->fmt;
- assert(f2 == STRING);
- }
-
- if (tl->t->tok != '+' && tl->t->tok != '-')
- return;
-
- switch(f2) {
- case STRING:
- case STRING_LIST:
- vcc_expr_string_add(tl, e);
- return;
- case INT: break;
- case TIME: break;
- case DURATION: break;
- case BYTES: break;
- default:
- VSB_printf(tl->sb, "Operator %.*s not possible on type %s.\n",
- PF(tl->t), vcc_Type(f2));
- vcc_ErrWhere(tl, tl->t);
- return;
- }
-
while (tl->t->tok == '+' || tl->t->tok == '-') {
- if (f2 == TIME)
- f2 = DURATION;
tk = tl->t;
vcc_NextToken(tl);
- vcc_expr_mul(tl, &e2, f2);
+ if (f2 == TIME)
+ vcc_expr_mul(tl, &e2, DURATION);
+ else
+ vcc_expr_mul(tl, &e2, f2);
ERRCHK(tl);
if (tk->tok == '-' && (*e)->fmt == TIME && e2->fmt == TIME) {
/* OK */
} else if ((*e)->fmt == TIME && e2->fmt == DURATION) {
f2 = TIME;
/* OK */
- } else if (tk->tok == '-' &&
- (*e)->fmt == BYTES && e2->fmt == BYTES) {
+ } else if ((*e)->fmt == BYTES && e2->fmt == BYTES) {
+ /* OK */
+ } else if ((*e)->fmt == INT && e2->fmt == INT) {
+ /* OK */
+ } else if ((*e)->fmt == DURATION && e2->fmt == DURATION) {
/* OK */
- } else if (e2->fmt != f2) {
+ } else if (tk->tok == '+' &&
+ (*e)->fmt == STRING && e2->fmt == STRING) {
+ vcc_expr_string_add(tl, e, e2);
+ return;
+ } else if (tk->tok == '+' &&
+ (fmt == STRING || fmt == STRING_LIST)) {
+ /* Time to fold and add as string */
+ vcc_expr_tostring(tl, e, STRING);
+ vcc_expr_string_add(tl, e, e2);
+ return;
+ } else {
VSB_printf(tl->sb, "%s %.*s %s not possible.\n",
vcc_Type((*e)->fmt), PF(tk), vcc_Type(e2->fmt));
vcc_ErrWhere2(tl, tk, tl->t);
More information about the varnish-commit
mailing list