[Minor] Lua_util: Rework and add tests for callback_from_string

3 files changed, 61 insertions, 12 deletions

diff --git a/lualib/lua_dkim_tools.lua b/lualib/lua_dkim_tools.lua
index f61e38ae9..90bff13d5 100644
--- a/lualib/lua_dkim_tools.lua
+++ b/lualib/lua_dkim_tools.lua
@@ -699,7 +699,12 @@ exports.process_signing_settings = function(N, settings, opts)
     elseif k == 'vault_domains' then
       settings[k] = lua_maps.map_add(N, k, 'glob', 'DKIM signing domains in vault')
     elseif k == 'sign_condition' then
-      settings[k] = lua_util.callback_from_string(v)
+      local ret,f = lua_util.callback_from_string(v)
+      if ret then
+        settings[k] = f
+      else
+        logger.errx(rspamd_config, 'cannot load sign condition %s: %s', v, f)
+      end
     else
       settings[k] = v
     end
diff --git a/lualib/lua_util.lua b/lualib/lua_util.lua
index 4aa39db9b..6a43294c6 100644
--- a/lualib/lua_util.lua
+++ b/lualib/lua_util.lua
@@ -1071,23 +1071,37 @@ end
 
 ---[[[
 -- @function lua_util.callback_from_string(str)
--- Converts a string like `return function(...) end` to lua function or emits error using
--- `rspamd_config` superglobal
--- @return function object or nil
+-- Converts a string like `return function(...) end` to lua function and return true and this function
+-- or returns false + error message
+-- @return status code and function object or an error message
 --]]]
-exports.callback_from_string = function(str)
+exports.callback_from_string = function(s)
   local loadstring = loadstring or load
-  local ret, res_or_err = pcall(loadstring(str))
 
-  if not ret or type(res_or_err) ~= 'function' then
-    local rspamd_logger = require "rspamd_logger"
-    rspamd_logger.errx(rspamd_config, 'invalid callback (%s) - must be a function',
-        res_or_err)
+  if not s or #s == 0 then
+    return false,'invalid or empty string'
+  end
 
-    return nil
+  s = exports.rspamd_str_trim(s)
+  local inp
+
+  if s:match('^return%s*function') then
+    -- 'return function', can be evaluated directly
+    inp = s
+  elseif s:match('^function%s*%(') then
+    inp = 'return ' .. s
+  else
+    -- Just a plain sequence
+    inp = 'return function(...)\n' .. s .. '; end'
+  end
+
+  local ret, res_or_err = pcall(loadstring(inp))
+
+  if not ret or type(res_or_err) ~= 'function' then
+    return false,res_or_err
   end
 
-  return res_or_err
+  return ret,res_or_err
 end
 
 ---[[[
diff --git a/test/lua/unit/lua_util.misc.lua b/test/lua/unit/lua_util.misc.lua
new file mode 100644
index 000000000..b19b4d6f1
--- /dev/null
+++ b/test/lua/unit/lua_util.misc.lua
@@ -0,0 +1,30 @@
+local util  = require 'lua_util'
+
+context("Lua util - callback_from_string", function()
+  local cases = {
+    {'return function', 'return function(a, b) return a + b end'},
+    {'function', 'function(a, b) return a + b end'},
+    {'plain ops', 'local c = select(1, ...)\nreturn c + select(2, ...)'},
+  }
+  local fail_cases = {
+    nil,
+    '',
+    'return function(a, b) ( end',
+    'function(a, b) ( end',
+    'return a + b'
+  }
+
+  for _,c in ipairs(cases) do
+    test('Success case: ' .. c[1], function()
+      local ret,f = util.callback_from_string(c[2])
+      assert_true(ret, f)
+      assert_equal(f(2, 2), 4)
+    end)
+  end
+  for i,c in ipairs(fail_cases) do
+    test('Failure case: ' .. tostring(i), function()
+      local ret,f = util.callback_from_string(c)
+      assert_false(ret)
+    end)
+  end
+end)
\ No newline at end of file