MEDIUM: vars: make the ifexist variant of set-var only apply to the proc scope
When setting variables, there are currently two variants, one which will
always create the variable, and another one, "ifexist", which will only
create or update a variable if a similarly named variable in any scope
already existed before.
The goal was to limit the risk of injecting random names in the proc
scope, but it was achieved by making use of the somewhat limited name
indexing model, which explains the scope-agnostic restriction.
With this change, we're moving the check downwards in the chain, at the
variable level, and only variables under the scope "proc" will be subject
to the restriction. A new set of VF_* flags was added to adjust how
variables are set, and VF_UPDATEONLY is used to mention this restriction.
In this exact state of affairs, this is not completely exact, as if a
similar name was not known in any scope, the variable will continue to
be rejected like before, but this will change soon.
diff --git a/doc/SPOE.txt b/doc/SPOE.txt
index 4792963..2330c49 100644
--- a/doc/SPOE.txt
+++ b/doc/SPOE.txt
@@ -276,7 +276,8 @@
option force-set-var
By default, SPOE filter only register already known variables (mainly from
- parsing of the configuration). If you want that haproxy trusts the agent and
+ parsing of the configuration), and process-wide variables (those of scope
+ "proc") cannot be created. If you want that haproxy trusts the agent and
registers all variables (ex: can be useful for LUA workload), activate this
option.
diff --git a/doc/lua-api/index.rst b/doc/lua-api/index.rst
index d671b32..991dc6f 100644
--- a/doc/lua-api/index.rst
+++ b/doc/lua-api/index.rst
@@ -2023,8 +2023,9 @@
integer.
:param boolean ifexist: If this parameter is set to a truthy value the variable
will only be set if it was defined elsewhere (i.e. used
- within the configuration). It is highly recommended to
- always set this to true.
+ within the configuration). For global variables (using the
+ "proc" scope), they will only be updated and never created.
+ It is highly recommended to always set this to true.
.. js:function:: TXN.unset_var(TXN, var)
@@ -2831,7 +2832,9 @@
integer.
:param boolean ifexist: If this parameter is set to a truthy value the variable
will only be set if it was defined elsewhere (i.e. used
- within the configuration). It is highly recommended to
+ within the configuration). For global variables (using the
+ "proc" scope), they will only be updated and never created.
+ It is highly recommended to
always set this to true.
:see: :js:func:`AppletHTTP.unset_var`
:see: :js:func:`AppletHTTP.get_var`
@@ -2946,7 +2949,9 @@
integer.
:param boolean ifexist: If this parameter is set to a truthy value the variable
will only be set if it was defined elsewhere (i.e. used
- within the configuration). It is highly recommended to
+ within the configuration). For global variables (using the
+ "proc" scope), they will only be updated and never created.
+ It is highly recommended to
always set this to true.
:see: :js:func:`AppletTCP.unset_var`
:see: :js:func:`AppletTCP.get_var`
diff --git a/include/haproxy/vars-t.h b/include/haproxy/vars-t.h
index e1e0452..0e35ade 100644
--- a/include/haproxy/vars-t.h
+++ b/include/haproxy/vars-t.h
@@ -25,6 +25,9 @@
#include <haproxy/sample_data-t.h>
#include <haproxy/thread-t.h>
+/* flags used when setting/clearing variables */
+#define VF_UPDATEONLY 0x00000001 // SCOPE_PROC variables are only updated
+
enum vars_scope {
SCOPE_SESS = 0,
SCOPE_TXN,
diff --git a/src/vars.c b/src/vars.c
index 8a54c20..df26ac1 100644
--- a/src/vars.c
+++ b/src/vars.c
@@ -352,9 +352,14 @@
* and the stream may be NULL when scope is SCOPE_SESS. In case there wouldn't
* be enough memory to store the sample while the variable was already created,
* it would be changed to a bool (which is memory-less).
+ *
+ * Flags is a bitfield that may contain one of the following flags:
+ * - VF_UPDATEONLY: if the scope is SCOPE_PROC, the variable may only be
+ * updated but not created.
+ *
* It returns 0 on failure, non-zero on success.
*/
-static int var_set(const char *name, enum vars_scope scope, struct sample *smp)
+static int var_set(const char *name, enum vars_scope scope, struct sample *smp, uint flags)
{
struct vars *vars;
struct var *var;
@@ -383,6 +388,10 @@
-var->data.u.meth.str.data);
}
} else {
+ /* creation permitted for proc ? */
+ if (flags & VF_UPDATEONLY && scope == SCOPE_PROC)
+ goto unlock;
+
/* Check memory available. */
if (!var_accounting_add(vars, smp->sess, smp->strm, sizeof(struct var)))
goto unlock;
@@ -487,7 +496,7 @@
/* Returns 0 if fails, else returns 1. */
static int smp_conv_store(const struct arg *args, struct sample *smp, void *private)
{
- return var_set(args[0].data.var.name, args[0].data.var.scope, smp);
+ return var_set(args[0].data.var.name, args[0].data.var.scope, smp, 0);
}
/* Returns 0 if fails, else returns 1. */
@@ -541,7 +550,7 @@
if (!name)
return 0;
- return var_set(name, scope, smp);
+ return var_set(name, scope, smp, VF_UPDATEONLY);
}
@@ -557,7 +566,7 @@
if (!name)
return 0;
- return var_set(name, scope, smp);
+ return var_set(name, scope, smp, 0);
}
/* This function unset a variable if it was already defined.
@@ -717,7 +726,7 @@
smp_set_owner(&smp, px, sess, s, 0);
smp.data.type = SMP_T_STR;
smp.data.u.str = *fmtstr;
- var_set(rule->arg.vars.name, rule->arg.vars.scope, &smp);
+ var_set(rule->arg.vars.name, rule->arg.vars.scope, &smp, 0);
}
else {
/* an expression is used */
@@ -727,7 +736,7 @@
}
/* Store the sample, and ignore errors. */
- var_set(rule->arg.vars.name, rule->arg.vars.scope, &smp);
+ var_set(rule->arg.vars.name, rule->arg.vars.scope, &smp, 0);
free_trash_chunk(fmtstr);
return ACT_RET_CONT;
}