On Tue, Aug 31, 2021 at 04:41:16PM +0200, Tim Düsterhus wrote: > 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.
Ah yes, thank you, I forgot about this one! > > 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. Yeah that's the same now in 2.5-dev with the dedicated test. > 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. That's what I remembered as well, thanks for confirming! > 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. ... and without eating all the memory :-) > As such: Your patches LGTM, thanks. Please proceed :-) Will do, and reference the issue above and update the doc regarding ifexist, just mentioning that it's now ignored for legacy compatibility. Thanks! Willy
