[Test] Add unit tests for the new logger behaviour

diff --git a/src/lua/lua_logger.c b/src/lua/lua_logger.c
index becfee85d8b993812f7fa3a22ea3551841b522fb..04ff81b6ded5f70551dc2d55532f50542dc346d9 100644 (file)
--- a/src/lua/lua_logger.c
+++ b/src/lua/lua_logger.c
@@ -280,6 +280,7 @@ lua_logger_char_safe(int t, unsigned int esc_type)
        return true;
 }
 
+#define LUA_MAX_ARGS 32
 /* Gracefully handles argument mismatches by substituting missing args and noting extra args */
 static glong
 lua_logger_log_format_str(lua_State *L, int offset, char *logbuf, gsize remain,
@@ -289,11 +290,12 @@ lua_logger_log_format_str(lua_State *L, int offset, char *logbuf, gsize remain,
        const char *c;
        gsize r;
        int digit;
-
        char *d = logbuf;
        unsigned int arg_num, cur_arg = 0, arg_max = lua_gettop(L) - offset;
-       unsigned int max_arg_used = 0;
+       gboolean args_used[LUA_MAX_ARGS];
+       unsigned int used_args_count = 0;
 
+       memset(args_used, 0, sizeof(args_used));
        while (remain > 1 && *fmt) {
                if (*fmt == '%') {
                        ++fmt;
@@ -307,7 +309,7 @@ lua_logger_log_format_str(lua_State *L, int offset, char *logbuf, gsize remain,
                                while ((digit = g_ascii_digit_value(*fmt)) >= 0) {
                                        ++fmt;
                                        arg_num = arg_num * 10 + digit;
-                                       if (arg_num >= 100) {
+                                       if (arg_num >= LUA_MAX_ARGS) {
                                                /* Avoid ridiculously large numbers */
                                                fmt = c;
                                                break;
@@ -321,20 +323,17 @@ lua_logger_log_format_str(lua_State *L, int offset, char *logbuf, gsize remain,
                        }
 
                        if (fmt > c) {
-                               if (cur_arg < 1) {
-                                       /* Invalid argument number format */
-                                       r = rspamd_snprintf(d, remain, "<INVALID ARG NUMBER>");
-                               }
-                               else if (cur_arg > arg_max) {
+                               if (cur_arg < 1 || cur_arg > arg_max) {
                                        /* Missing argument - substitute placeholder */
                                        r = rspamd_snprintf(d, remain, "<MISSING ARGUMENT>");
                                }
                                else {
                                        /* Valid argument - output it */
                                        r = lua_logger_out(L, offset + cur_arg, d, remain, esc_type);
-                                       /* Track maximum argument used */
-                                       if (cur_arg > max_arg_used) {
-                                               max_arg_used = cur_arg;
+                                       /* Track which arguments are used */
+                                       if (cur_arg <= LUA_MAX_ARGS && !args_used[cur_arg - 1]) {
+                                               args_used[cur_arg - 1] = TRUE;
+                                               used_args_count++;
                                        }
                                }
 
@@ -353,8 +352,8 @@ lua_logger_log_format_str(lua_State *L, int offset, char *logbuf, gsize remain,
        }
 
        /* Check for extra arguments and append warning if any */
-       if (max_arg_used < arg_max && remain > 1) {
-               unsigned int extra_args = arg_max - max_arg_used;
+       if (used_args_count > 0 && used_args_count < arg_max && remain > 1) {
+               unsigned int extra_args = arg_max - used_args_count;
                r = rspamd_snprintf(d, remain, " <EXTRA %d ARGUMENTS>", (int) extra_args);
                remain -= r;
                d += r;
@@ -365,6 +364,8 @@ lua_logger_log_format_str(lua_State *L, int offset, char *logbuf, gsize remain,
        return d - logbuf;
 }
 
+#undef LUA_MAX_ARGS
+
 static gsize
 lua_logger_out_str(lua_State *L, int pos,
                                   char *outbuf, gsize len,

diff --git a/test/lua/unit/logger.lua b/test/lua/unit/logger.lua
index dc0120709ec089f5f11e17ddd9ab6844ece3cbcd..c28d8bb09bf5ea1d98c04d0c6ea4ca171092a546 100644 (file)
--- a/test/lua/unit/logger.lua
+++ b/test/lua/unit/logger.lua
@@ -3,17 +3,17 @@ context("Logger unit tests", function()
     local log = require "rspamd_logger"
 
     local cases = {
-      {'string', 'string'},
-      {'%1', 'string', 'string'},
-      {'%1', '1.1', 1.1},
-      {'%1', '1', 1},
-      {'%1', 'true', true},
-      {'%1', '{[1] = 1, [2] = test}', {1, 'test'}},
-       {'%1', '{[1] = 1, [2] = 2.1, [k2] = test}', {1, 2.1, k2='test'}},
-      {'%s', 'true', true},
+      { 'string', 'string' },
+      { '%1', 'string', 'string' },
+      { '%1', '1.1', 1.1 },
+      { '%1', '1', 1 },
+      { '%1', 'true', true },
+      { '%1', '{[1] = 1, [2] = test}', { 1, 'test' } },
+      { '%1', '{[1] = 1, [2] = 2.1, [k2] = test}', { 1, 2.1, k2 = 'test' } },
+      { '%s', 'true', true },
     }
 
-    for _,c in ipairs(cases) do
+    for _, c in ipairs(cases) do
       local s
       if c[3] then
         s = log.slog(c[1], c[3])
@@ -21,7 +21,82 @@ context("Logger unit tests", function()
         s = log.slog(c[1])
       end
       assert_equal(s, c[2], string.format("'%s' doesn't match with '%s'",
-        c[2], s))
+          c[2], s))
+    end
+  end)
+
+  test("Logger graceful error handling", function()
+    local log = require "rspamd_logger"
+
+    -- Test missing arguments
+    local missing_arg_cases = {
+      { '%1', '<MISSING ARGUMENT>' },
+      { '%0', '<MISSING ARGUMENT>' }, -- %0 is invalid since Lua args are 1-indexed
+      { '%2', '<MISSING ARGUMENT>', 'arg1' },
+      { '%1 %2', 'arg1 <MISSING ARGUMENT>', 'arg1' },
+      { 'prefix %1 %3 suffix', 'prefix arg1 <MISSING ARGUMENT> suffix', 'arg1' },
+    }
+
+    for _, c in ipairs(missing_arg_cases) do
+      local s
+      if c[3] then
+        s = log.slog(c[1], c[3])
+      else
+        s = log.slog(c[1])
+      end
+      assert_equal(s, c[2], string.format("Missing arg test: '%s' doesn't match with '%s'",
+          c[2], s))
+    end
+
+    -- Test extra arguments
+    local extra_arg_cases = {
+      { '%1', 'arg1 <EXTRA 1 ARGUMENTS>', 'arg1', 'extra1' },
+      { '%1', 'arg1 <EXTRA 2 ARGUMENTS>', 'arg1', 'extra1', 'extra2' },
+      { '%s', 'arg1 <EXTRA 1 ARGUMENTS>', 'arg1', 'extra1' },
+      { 'prefix %1 suffix', 'prefix arg1 suffix <EXTRA 1 ARGUMENTS>', 'arg1', 'extra1' },
+    }
+
+    for _, c in ipairs(extra_arg_cases) do
+      local s
+      if c[4] and c[5] then
+        s = log.slog(c[1], c[3], c[4], c[5])
+      elseif c[4] then
+        s = log.slog(c[1], c[3], c[4])
+      else
+        s = log.slog(c[1], c[3])
+      end
+      assert_equal(s, c[2], string.format("Extra arg test: '%s' doesn't match with '%s'",
+          c[2], s))
+    end
+
+    -- Test literal percent sequences (should pass through as-is)
+    local literal_cases = {
+      { '%-1', '%-1' },
+      { '%abc', '%abc' }, -- Should pass through as literal since it's not a valid number
+      { '%', '%' }, -- Single percent should pass through
+    }
+
+    for _, c in ipairs(literal_cases) do
+      local s = log.slog(c[1])
+      assert_equal(s, c[2], string.format("Literal test: '%s' doesn't match with '%s'",
+          c[2], s))
+    end
+
+    -- Test mixed scenarios
+    local mixed_cases = {
+      { '%1 %3', 'arg1 <MISSING ARGUMENT> <EXTRA 1 ARGUMENTS>', 'arg1', 'extra1' },
+      { '%2 %4', 'extra1 <MISSING ARGUMENT> <EXTRA 1 ARGUMENTS>', 'arg1', 'extra1' },
+    }
+
+    for _, c in ipairs(mixed_cases) do
+      local s
+      if c[4] then
+        s = log.slog(c[1], c[3], c[4])
+      else
+        s = log.slog(c[1], c[3])
+      end
+      assert_equal(s, c[2], string.format("Mixed test: '%s' doesn't match with '%s'",
+          c[2], s))
     end
   end)
 end)
\ No newline at end of file