Willy, On 8/31/21 9:07 AM, Willy Tarreau wrote:
I've finally implemented the replacement of the global variables table
Okay, please refer to issue #624 in the commit: https://github.com/haproxy/haproxy/issues/624. I believe it should be resolved afterwards.
with a hash instead. However it now obviously breaks the "ifexist" argument that you added to Lua's set_var() that was designed to work around the growing table problem. Given that the limitation used to be made on the existence of a similarly named variable anywhere in the process (and not of the variable in the current list), I conclude that it was only used to preserve precious resources and never to conditionally set a variable in one's context. As such we could simply remove this test and silently ignore the ifexist argument now. Do you agree with this ? I'd really like it if we could definitely get rid of this old mess!
For the record this is my the repository I care about: https://github.com/TimWolla/haproxy-auth-request It includes tests based on VTest. I just ran the tests with patches applied:
$ vtest -Dhaproxy_version=2.5.0 -q -k -t 10 -C test/*.vtc # top TEST test/no_variable_leak.vtc FAILED (0.203) exit=2 1 tests failed, 0 tests skipped, 19 tests passed
Naturally the no_variable_leak.vtc test (https://github.com/TimWolla/haproxy-auth-request/blob/main/test/no_variable_leak.vtc) fails now, as it specifically tests that my detection of the ifexist parameter works as expected. I can simply exclude HAProxy 2.5 from that test and all is well.
In my case I only care about not bloating HAProxy's memory usage infinitely, in case the backend sends sends headers with randomly generated names (these are exposed as req.auth_response_header.*). The fact that variables are unavailable to Lua is a side-effect of this, not a feature.
It would certainly be preferable for the user if they could simply use the variables from Lua, without needing to reference them in the config.
As such: Your patches LGTM, thanks. Please proceed :-)
For reference I'm appending the current patch series.
Best regards Tim Düsterhus
