fix(lib): do not insert "nulls" as values for __VARARGS__ config arguments

[OverOrion]

@version: current

block destination second(...) {
      `__VARARGS__`
};

block destination first(...) {
      second(`__VARARGS__`);
};

log {
   destination{ first(some-param("")) };
};

After the fix:

[2024-10-15T09:49:23.089776] Unknown argument, adding it to __VARARGS__; argument='some_param', value='', reference='../install_dir/etc/empty.conf:12:17'
[2024-10-15T09:49:23.089776] Unknown argument, adding it to __VARARGS__; argument='some_param', value='', reference='../install_dir/etc/empty.conf:8:7'

Error parsing log statement: syntax error, unexpected LL_IDENTIFIER, expecting '}'

In ../install_dir/etc/empty.conf:4:7-4:17:
-1      
0       #Start Block block destination second() at ../install_dir/etc/empty.conf:3
1       ## some_param=
2       @line "../install_dir/etc/empty.conf" 3 31
3       
4----->       some_param("") 
4----->       ^^^^^^^^^^
5       
6       #End Block block destination second() at ../install_dir/etc/empty.conf:3
7       

Included from ../install_dir/etc/empty.conf:8:7-8:30:
3       
4       #Start Block block destination first() at ../install_dir/etc/empty.conf:7
5       ## some_param=
6       @line "../install_dir/etc/empty.conf" 7 30
7       
8----->       second(some_param("") );
8----->       ^^^^^^^^^^^^^^^^^^^^^^^
9       
10      #End Block block destination first() at ../install_dir/etc/empty.conf:7
11      

Included from ../install_dir/etc/empty.conf:12:17-12:38:
7       block destination first(...) {
8             second(`__VARARGS__`);
9       };
10      
11      log {
12---->    destination{ first(some-param("")) };
12---->                 ^^^^^^^^^^^^^^^^^^^^^
13      };

Fixes #244.

[OverOrion]

On a second thought this might not be a proper fix:

[2024-08-13T09:19:13.979827] Unknown argument, adding it to __VARARGS__; argument='some_param', value='', reference='/source/install_dir/etc/foo.conf:12:17'
[2024-08-13T09:19:13.979827] Unknown argument, adding it to __VARARGS__; argument='some_param', value='(null)', reference='/source/install_dir/etc/foo.conf:8:7'

The value gets lost somewhere 👀

[MrAnno]

As far as I remember this only happens through multi-level passes of __VARARGS__ with empty values, so the real fix might be somewhere else and will be something about how we handle the empty string parameter passed through these varargs.

[alltilla]

Another config that crashes at the same location:

@version: current

block destination a(filtr(def)) {
  channel {
    filter {
      `filtr`
      and 1 == 1};
  };
};

destination d { a(filtr()); };

[OverOrion]

Updated the PR to properly handle empty __VARARGS__ values instead.

[MrAnno]

This is getting closer, it fixes the crash with the above config snippet but changes the output if you type some-param() to some-param("") inside the block (but it crashes before that because of the same null-ptr deref issue).

I think the issue is in the lexer, it removes "" when passing parameters.
I'm investigating this and propose my patch soon.

[MrAnno]

<block>\({white}*\)        { yy_pop_state(yyscanner); yylval->cptr = NULL; return LL_BLOCK; }
<block>\({white}*\"\"{white}*\) { yy_pop_state(yyscanner); yylval->cptr = g_strdup(""); return LL_BLOCK; }
<block>\({white}*\'\'{white}*\) { yy_pop_state(yyscanner); yylval->cptr = g_strdup(""); return LL_BLOCK; }

These are the ones removing what we need.
The problem here is probably the context of where we use these:

• we need to remove empty quotes in the declaration of block params (that's how we detect "optional" and "mandatory")
• we should NOT remove quotes when we pass arguments to blocks (when we "call" them) it's more complicated than that.

[MrAnno]

I can't fix this without reimplementing f0650bb completely, which would be a lot more work.

syslog-ng/syslog-ng#2088 (comment) says the following:

As "" is not mapped into "" anymore, this is a problem needs to be addressed. Luckily, we could break the cycle here, do not need to find another symbol that would be mapped to "", of which again find a symbol that maps to the underlying symbol etc. This can be addressed by modifying such scl. I show an example for this:

Unfortunately, this is not so lucky with varargs. With the current implementation, it is impossible to pass an empty string parameter through varargs. With your current patch, it is possible, but then it breaks the fully empty case.

So in order to fix the crash (but without making it possible to pass empty strings properly), I think we could go with this (which is very similar to your original patch, except for the new-line ending):

diff --git a/lib/cfg-block-generator.c b/lib/cfg-block-generator.c
index 9374c58e8..00811f278 100644
--- a/lib/cfg-block-generator.c
+++ b/lib/cfg-block-generator.c
@@ -39,6 +39,7 @@ _report_generator_args(gpointer key, gpointer value, gpointer user_data)
 {
   GString *result = (GString *) user_data;
   g_string_append_printf(result, "## %s=", (gchar *) key);
+  value = value ? : "";
   for (const gchar *c = (const gchar *) value; *c; c++)
     {
       if (*c == '\n' && *(c + 1))

Because of this:

axosyslog/lib/cfg-lexer-subst.c

Line 257 in f074c0f

if (!_append_value(self, value ? : "", error))

And we can open a separate issue about the impossibility to pass empty strings through varargs.
@OverOrion Is that okay with you?