MEDIUM: vars: make the var() sample fetch function really return type ANY

A long-standing issue was reported in issue #1215.

In short, var() was initially internally declared as returning a string
because it was not possible by then to return "any type". As such, users
regularly get trapped thinking that when they're storing an integer there,
then the integer matching method automatically applies. Except that this
is not possible since this is related to the config parser and is decided
at boot time where the variable's type is not known yet.

As such, what is done is that the output being declared as type string,
the string match will automatically apply, and any value will first be
converted to a string. This results in several issues like:

    http-request set-var(txn.foo) int(-1)
    http-request deny if { var(txn.foo) lt 0 }

not working. This is because the string match on the second line will in
fact compare the string representation of the variable against strings
"lt" and "0", none of which matches.

The doc says that the matching method is mandatory, though that's not
the case in the code due to that default string type being permissive.
There's not even a warning when no explicit match is placed, because
this happens very deep in the expression evaluator and making a special
case just for "var" can reveal very complicated.

The set-var() converter already mandates a matching method, as the
following will be rejected:

    ... if { int(12),set-var(txn.truc) 12 }

  while this one will work:

    ... if { int(12),set-var(txn.truc) -m int 12 }

As such, this patch this modifies var() to match the doc, returning the
type "any", and mandating the matching method, implying that this bogus
config which does not work:

    http-request set-var(txn.foo) int(-1)
    http-request deny if { var(txn.foo) lt 0 }

  will need to be written like this:

    http-request set-var(txn.foo) int(-1)
    http-request deny if { var(txn.foo) -m int lt 0 }

This *will* break some configs (and even 3 of our regtests relied on
this), but except those which already match string exclusively, all
other ones are already broken and silently fail (and one of the 3
regtests, the one on FIX, was bogus regarding this).

In order to fix existing configs, one can simply append "-m str"
after a "var()" in an ACL or "if" expression:

    http-request deny unless { var(txn.jwt_alg) "ES" }

  must become:

    http-request deny unless { var(txn.jwt_alg) -m str "ES" }

Most commonly, patterns such as "le", "lt", "ge", "gt", "eq", "ne" in
front of a number indicate that the intent was to match an integer,
and in this case "-m int" would be desired:

    tcp-response content reject if ! { var(res.size) gt 3800 }

  ought to become:

    tcp-response content reject if ! { var(res.size) -m int gt 3800 }

This must not be backported, but if a solution is found to at least
detect this exact condition in the generic expression parser and
emit a warning, this could probably help spot configuration bugs.

Link: https://www.mail-archive.com/haproxy@formilux.org/msg41341.html
Cc: Christopher Faulet <cfaulet@haproxy.com>
Cc: Tim Düsterhus <tim@bastelstu.be>

diff --git a/reg-tests/converter/fix.vtc b/reg-tests/converter/fix.vtc
index f035445ac1bb08aac071858399dad3ba477c4544..6be46765ba6317f9052fe9682e80b39931cf2e2b 100644 (file)
--- a/reg-tests/converter/fix.vtc
+++ b/reg-tests/converter/fix.vtc
@@ -76,9 +76,9 @@ haproxy h1 -conf {
         tcp-request content set-var(req.fix_sender) req.payload(0,0),fix_tag_value(SenderCompID)
         tcp-request content set-var(req.fix_target) req.payload(0,0),fix_tag_value(TargetCompID)
         tcp-request content set-var(req.fix_chksum) req.payload(0,0),fix_tag_value(CheckSum)
-        tcp-request content reject if ! { var(req.fix_vsn) "FIX.4.4" } || ! { var(req.fix_len) "102" }
-        tcp-request content reject if ! { var(req.fix_type) "A" } || ! { var(req.fix_sender) "CLIENT1" }
-        tcp-request content reject if ! { var(req.fix_target) "EXECUTOR" } || ! { var(req.fix_chksum) "252" }
+        tcp-request content reject if ! { var(req.fix_vsn) -m str "FIX.4.4" }     || ! { var(req.fix_len) -m str "102" }
+        tcp-request content reject if ! { var(req.fix_type) -m str "A" }          || ! { var(req.fix_sender) -m str "CLIENT1" }
+        tcp-request content reject if ! { var(req.fix_target) -m str "EXECUTOR" } || ! { var(req.fix_chksum) -m str "252" }
         default_backend be2
 
     backend be1
@@ -96,9 +96,9 @@ haproxy h1 -conf {
         tcp-response content set-var(res.fix_sender) res.payload(0,0),fix_tag_value(49)
         tcp-response content set-var(res.fix_target) res.payload(0,0),fix_tag_value(56)
         tcp-response content set-var(res.fix_chksum) res.payload(0,0),fix_tag_value(10)
-        tcp-response content reject if ! { var(res.fix_vsn) "FIX.4.4" } || ! { var(res.fix_len) "79" }
-        tcp-response content reject if ! { var(res.fix_type) "A" } || ! { var(res.fix_sender) "EXECUTOR" }
-        tcp-response content reject if ! { var(res.fix_target) "CLIENT1" } || ! { var(res.fix_chksum) eq "038" }
+        tcp-response content reject if ! { var(res.fix_vsn) -m str "FIX.4.4" }    || ! { var(res.fix_len) -m str "79" }
+        tcp-response content reject if ! { var(res.fix_type) -m str "A" }         || ! { var(res.fix_sender) -m str "EXECUTOR" }
+        tcp-response content reject if ! { var(res.fix_target) -m str "CLIENT1" } || ! { var(res.fix_chksum) -m str "038" }
 } -start
 
 client c1_4_0 -connect ${h1_fe1_sock} {

diff --git a/reg-tests/converter/mqtt.vtc b/reg-tests/converter/mqtt.vtc
index ea2cbb4f89243a64e31f103661e7ac634ff4c435..d4e319bf28967e42f3bfefcd273125bc1bd9eac8 100644 (file)
--- a/reg-tests/converter/mqtt.vtc
+++ b/reg-tests/converter/mqtt.vtc
@@ -79,12 +79,12 @@ haproxy h1 -conf {
         tcp-request content set-var(req.reqpbinfo)    req.payload(0,0),mqtt_field_value(connect,23)
         tcp-request content set-var(req.ctype)        req.payload(0,0),mqtt_field_value(connect,3)
         tcp-request content set-var(req.willrsptopic) req.payload(0,0),mqtt_field_value(connect,8)
-        tcp-request content reject if ! { var(req.protoname) "MQTT" }      || ! { var(req.protovsn) "5" }
-        tcp-request content reject if ! { var(req.flags) "238" }           || ! { var(req.clientid) "test_sub" }
-        tcp-request content reject if ! { var(req.user) "test" }           || ! { var(req.pass) "passwd" }
-        tcp-request content reject if ! { var(req.willtopic) "willtopic" } || ! { var(req.willbody) "willpayload" }
-        tcp-request content reject if ! { var(req.maxpktsz) "20" }         || ! { var(req.reqpbinfo) "1" }
-        tcp-request content reject if ! { var(req.ctype) "text/plain" }    || ! { var(req.willrsptopic) "willrsptopic" }
+        tcp-request content reject if ! { var(req.protoname) -m str "MQTT" }      || ! { var(req.protovsn) -m str "5" }
+        tcp-request content reject if ! { var(req.flags) -m str "238" }           || ! { var(req.clientid) -m str "test_sub" }
+        tcp-request content reject if ! { var(req.user) -m str "test" }           || ! { var(req.pass) -m str "passwd" }
+        tcp-request content reject if ! { var(req.willtopic) -m str "willtopic" } || ! { var(req.willbody) -m str "willpayload" }
+        tcp-request content reject if ! { var(req.maxpktsz) -m str "20" }         || ! { var(req.reqpbinfo) -m str "1" }
+        tcp-request content reject if ! { var(req.ctype) -m str "text/plain" }    || ! { var(req.willrsptopic) -m str "willrsptopic" }
         default_backend be2
 
     backend be1
@@ -105,11 +105,11 @@ haproxy h1 -conf {
         tcp-response content set-var(res.retainavail)   res.payload(0,0),mqtt_field_value(connack,37)
         tcp-response content set-var(res.maxpktsz)      res.payload(0,0),mqtt_field_value(connack,39)
         tcp-response content set-var(res.topicaliasmax) res.payload(0,0),mqtt_field_value(connack,34)
-        tcp-response content reject if ! { var(res.protovsn) "5" }    || ! { var(res.flags) "0" }
-        tcp-response content reject if ! { var(res.rcode) "0" }       || ! { var(res.sessexpint) "120" }
-        tcp-response content reject if ! { var(res.recvmax) "32767" } || ! { var(res.maxqos) "1" }
-        tcp-response content reject if ! { var(res.retainavail) "1" } || ! { var(res.maxpktsz) "65535" }
-        tcp-response content reject if ! { var(res.topicaliasmax) "10" }
+        tcp-response content reject if ! { var(res.protovsn) -m str "5" }    || ! { var(res.flags) -m str "0" }
+        tcp-response content reject if ! { var(res.rcode) -m str "0" }       || ! { var(res.sessexpint) -m str "120" }
+        tcp-response content reject if ! { var(res.recvmax) -m str "32767" } || ! { var(res.maxqos) -m str "1" }
+        tcp-response content reject if ! { var(res.retainavail) -m str "1" } || ! { var(res.maxpktsz) -m str "65535" }
+        tcp-response content reject if ! { var(res.topicaliasmax) -m str "10" }
 } -start
 
 client c1_311_1 -connect ${h1_fe1_sock} {

diff --git a/reg-tests/jwt/jws_verify.vtc b/reg-tests/jwt/jws_verify.vtc
index c7913349d236a4637d83c798a9ec90dbd0d5dd43..91ead4af6f283239922752516c01042d573d1814 100644 (file)
--- a/reg-tests/jwt/jws_verify.vtc
+++ b/reg-tests/jwt/jws_verify.vtc
@@ -52,9 +52,9 @@ haproxy h1 -conf {
         http-response set-header x-jwt-token %[var(txn.bearer)]
         http-response set-header x-jwt-alg %[var(txn.jwt_alg)]
 
-        http-response set-header x-jwt-verify-HS256 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"hmac key hs256")] if { var(txn.jwt_alg) "HS256" }
-        http-response set-header x-jwt-verify-HS384 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"hmac key hs384")] if { var(txn.jwt_alg) "HS384" }
-        http-response set-header x-jwt-verify-HS512 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"hmac key hs512")] if { var(txn.jwt_alg) "HS512" }
+        http-response set-header x-jwt-verify-HS256 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"hmac key hs256")] if { var(txn.jwt_alg) -m str "HS256" }
+        http-response set-header x-jwt-verify-HS384 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"hmac key hs384")] if { var(txn.jwt_alg) -m str "HS384" }
+        http-response set-header x-jwt-verify-HS512 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"hmac key hs512")] if { var(txn.jwt_alg) -m str "HS512" }
         server s1 ${s1_addr}:${s1_port}
 
     backend rsXXX_be
@@ -66,9 +66,9 @@ haproxy h1 -conf {
         http-response set-header x-jwt-token %[var(txn.bearer)]
         http-response set-header x-jwt-alg %[var(txn.jwt_alg)]
 
-        http-response set-header x-jwt-verify-RS256 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"${testdir}/rsa-public.pem")] if { var(txn.jwt_alg) "RS256" }
-        http-response set-header x-jwt-verify-RS384 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"${testdir}/rsa-public.pem")] if { var(txn.jwt_alg) "RS384" }
-        http-response set-header x-jwt-verify-RS512 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"${testdir}/rsa-public.pem")] if { var(txn.jwt_alg) "RS512" }
+        http-response set-header x-jwt-verify-RS256 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"${testdir}/rsa-public.pem")] if { var(txn.jwt_alg) -m str "RS256" }
+        http-response set-header x-jwt-verify-RS384 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"${testdir}/rsa-public.pem")] if { var(txn.jwt_alg) -m str "RS384" }
+        http-response set-header x-jwt-verify-RS512 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"${testdir}/rsa-public.pem")] if { var(txn.jwt_alg) -m str "RS512" }
         server s1 ${s1_addr}:${s1_port}
 
     backend esXXX_be
@@ -80,9 +80,9 @@ haproxy h1 -conf {
         http-response set-header x-jwt-token %[var(txn.bearer)]
         http-response set-header x-jwt-alg %[var(txn.jwt_alg)]
 
-        http-response set-header x-jwt-verify-ES256 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"${testdir}/es256-public.pem")] if { var(txn.jwt_alg) "ES256" }
-        http-response set-header x-jwt-verify-ES384 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"${testdir}/es384-public.pem")] if { var(txn.jwt_alg) "ES384" }
-        http-response set-header x-jwt-verify-ES512 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"${testdir}/es512-public.pem")] if { var(txn.jwt_alg) "ES512" }
+        http-response set-header x-jwt-verify-ES256 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"${testdir}/es256-public.pem")] if { var(txn.jwt_alg) -m str "ES256" }
+        http-response set-header x-jwt-verify-ES384 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"${testdir}/es384-public.pem")] if { var(txn.jwt_alg) -m str "ES384" }
+        http-response set-header x-jwt-verify-ES512 %[var(txn.bearer),jwt_verify(txn.jwt_alg,"${testdir}/es512-public.pem")] if { var(txn.jwt_alg) -m str "ES512" }
         server s1 ${s1_addr}:${s1_port}
 
 

diff --git a/src/vars.c b/src/vars.c
index 999b05c85f4244cc2c3e25c514385a18fe1fcd43..0de90d86fabca30c82de2c9c46456b87069d6f6a 100644 (file)
--- a/src/vars.c
+++ b/src/vars.c
@@ -1199,7 +1199,7 @@ INITCALL0(STG_PREPARE, vars_init);
 
 static struct sample_fetch_kw_list sample_fetch_keywords = {ILH, {
 
-       { "var", smp_fetch_var, ARG2(1,STR,STR), smp_check_var, SMP_T_STR, SMP_USE_CONST },
+       { "var", smp_fetch_var, ARG2(1,STR,STR), smp_check_var, SMP_T_ANY, SMP_USE_CONST },
        { /* END */ },
 }};