[Test] Add unit tests for the new logger behaviourvstakhov-lua-logger-fixes

2 files changed, 99 insertions, 23 deletions

diff --git a/src/lua/lua_logger.c b/src/lua/lua_logger.c
index becfee85d..04ff81b6d 100644
--- 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 dc0120709..c28d8bb09 100644
--- 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