Hi all,
Among the current pending issues, Christopher and I have been scratching
our heads on an issue involving the var() sample fetch function. It
started in issue 1215 but is more generalized:
https://github.com/haproxy/haproxy/issues/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.
I had a look at some prod configs from our internal issue tracker and
could also find a few occurrences of the exact same issue. It's even worse
in fact, I found that the vast majority of tests performed on variables
are integer based, and when they work it's only because of exact match,
or inclusive operators such as "ge 1" which will match value 1 but none
above due to the string conversion. And regarding the exact matches they
can even get wrong sometimes, as setting string "-0" will not match string
"0" for example.
Christopher found that 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. We thought that at the very least we could emit a
warning when no explicit match is placed, but this happens very deep in
the expression evaluator and making a special case just for "var" can
reveal very complicated (but probably not impossible).
Christopher also found that 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) eq 12 }
So the question is (as you see me coming), is it acceptable to fix this in
2.5+ by making var() match the doc, returning the type "any", and mandathing
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 }
My lock checks showing that roughly 30% of the configs not using -m are
bogus does not even make me want to continue to support this without
telling users their config will not work. And any config fixed with this
will also be fixed and start to work for earlier versions.
I would love to have the option to emit a warning for older ones but I
really cannot promise anything. I expect that 2.5 users will be experienced
users and that it will not be a big deal to rely on this to fix their
configs once for all. I'd rather not do this during an odd->even switch
however.
I'd like to get some opinions quickly on this as while the error will
just be a one-liner fix, the warnings are way more complicated and will
take some time.
Thanks,
Willy
