[Minor] lua_scanners - fix need_check, lua warnings #2, use lua_util

11 files changed, 204 insertions, 168 deletions

diff --git a/lualib/lua_scanners/clamav.lua b/lualib/lua_scanners/clamav.lua
index 7771261e1..90cd67cef 100644
--- a/lualib/lua_scanners/clamav.lua
+++ b/lualib/lua_scanners/clamav.lua
@@ -167,7 +167,9 @@ local function clamav_check(task, content, digest, rule)
     })
   end
 
-  if common.need_check(task, content, rule, digest) then
+  if common.need_check(task, content, rule, digest, clamav_check_uncached) then
+    return
+  else
     clamav_check_uncached()
   end
 
diff --git a/lualib/lua_scanners/common.lua b/lualib/lua_scanners/common.lua
index 64f67dcc8..2869cc2ed 100644
--- a/lualib/lua_scanners/common.lua
+++ b/lualib/lua_scanners/common.lua
@@ -186,14 +186,16 @@ local function dynamic_scan(task, rule)
   end
 end
 
-local function check_cache(task, digest, rule, fn)
+local function need_check(task, content, rule, digest, fn)
+
+  local uncached = true
   local key = digest
 
   local function redis_av_cb(err, data)
     if data and type(data) == 'string' then
       -- Cached
-      data = rspamd_str_split(data, '\t')
-      local threat_string = rspamd_str_split(data[1], '\v')
+      data = lua_util.str_split(data, '\t')
+      local threat_string = lua_util.str_split(data[1], '\v')
       local score = data[2] or rule.default_score
       if threat_string[1] ~= 'OK' then
         lua_util.debugm(rule.name, task, '%s: got cached threat result for %s: %s - score: %s',
@@ -203,12 +205,28 @@ local function check_cache(task, digest, rule, fn)
         lua_util.debugm(rule.name, task, '%s: got cached negative result for %s: %s',
           rule.log_prefix, key, threat_string[1])
       end
+      uncached = false
     else
       if err then
         rspamd_logger.errx(task, 'got error checking cache: %s', err)
       end
-      return true
     end
+
+    local f_message_not_too_large = message_not_too_large(task, content, rule) or true
+    local f_message_not_too_small = message_not_too_small(task, content, rule) or true
+    local f_message_min_words = message_min_words(task, rule) or true
+    local f_dynamic_scan = dynamic_scan(task, rule) or true
+
+    if uncached and
+      f_message_not_too_large and
+      f_message_not_too_small and
+      f_message_min_words and
+      f_dynamic_scan then
+
+      fn()
+
+    end
+
   end
 
   if rule.redis_params then
@@ -228,14 +246,7 @@ local function check_cache(task, digest, rule, fn)
   end
 
   return false
-end
 
-local function need_check(task, content, rule, digest)
-  return check_cache(task, digest, rule) and
-    message_not_too_large(task, content, rule) and
-    message_not_too_small(task, content, rule) and
-    message_min_words(task, rule) and
-    dynamic_scan(task, rule)
 end
 
 local function save_cache(task, digest, rule, to_save, dyn_weight)
@@ -248,8 +259,8 @@ local function save_cache(task, digest, rule, to_save, dyn_weight)
       rspamd_logger.errx(task, 'failed to save %s cache for %s -> "%s": %s',
           rule.detection_category, to_save, key, err)
     else
-      lua_util.debugm(rule.name, task, '%s: saved cached result for %s: %s - score %s',
-        rule.log_prefix, key, to_save, dyn_weight)
+      lua_util.debugm(rule.name, task, '%s: saved cached result for %s: %s - score %s - ttl %s',
+        rule.log_prefix, key, to_save, dyn_weight, rule.cache_expire)
     end
   end
 
@@ -322,7 +333,7 @@ end
 -- ext is the last extension, LOWERCASED
 -- ext2 is the one before last extension LOWERCASED
 local function gen_extension(fname)
-  local filename_parts = rspamd_str_split(fname, '.')
+  local filename_parts = lua_util.str_split(fname, '.')
 
   local ext = {}
   for n = 1, 2 do
diff --git a/lualib/lua_scanners/dcc.lua b/lualib/lua_scanners/dcc.lua
index 85d72dd8b..db1ac4497 100644
--- a/lualib/lua_scanners/dcc.lua
+++ b/lualib/lua_scanners/dcc.lua
@@ -298,7 +298,9 @@ local function dcc_check(task, content, digest, rule)
     })
   end
 
-  if common.need_check(task, content, rule, digest) then
+  if common.need_check(task, content, rule, digest, dcc_check_uncached) then
+    return
+  else
     dcc_check_uncached()
   end
 
diff --git a/lualib/lua_scanners/fprot.lua b/lualib/lua_scanners/fprot.lua
index 30d179ec6..4061251cb 100644
--- a/lualib/lua_scanners/fprot.lua
+++ b/lualib/lua_scanners/fprot.lua
@@ -160,7 +160,9 @@ local function fprot_check(task, content, digest, rule)
     })
   end
 
-  if common.need_check(task, content, rule, digest) then
+  if common.need_check(task, content, rule, digest, fprot_check_uncached) then
+    return
+  else
     fprot_check_uncached()
   end
 
diff --git a/lualib/lua_scanners/icap.lua b/lualib/lua_scanners/icap.lua
index 92187fb76..1f3ada5c9 100644
--- a/lualib/lua_scanners/icap.lua
+++ b/lualib/lua_scanners/icap.lua
@@ -112,7 +112,7 @@ local function icap_check(task, content, digest, rule)
 
     local function icap_callback(err, conn)
 
-      local function icap_requery(error, info)
+      local function icap_requery(err_m, info)
         -- set current upstream to fail because an error occurred
         upstream:fail()
 
@@ -123,7 +123,7 @@ local function icap_check(task, content, digest, rule)
 
           lua_util.debugm(rule.name, task,
               '%s: %s Request Error: %s - retries left: %s',
-              rule.log_prefix, info, error, retransmits)
+              rule.log_prefix, info, err_m, retransmits)
 
           -- Select a different upstream!
           upstream = rule.upstreams:get_upstream_round_robin()
@@ -144,15 +144,14 @@ local function icap_check(task, content, digest, rule)
           })
         else
           rspamd_logger.errx(task, '%s: failed to scan, maximum retransmits '..
-            'exceed - err: %s', rule.log_prefix, error)
-          common.yield_result(task, rule, 'failed - err: ' .. error, 0.0, 'fail')
+            'exceed - error: %s', rule.log_prefix, err_m or '')
+          common.yield_result(task, rule, 'failed - error: ' .. err_m or '', 0.0, 'fail')
         end
       end
 
       local function get_respond_query()
-        table.insert(respond_headers, 1,
-            'RESPMOD icap://' .. addr:to_string() .. ':' .. addr:get_port() .. '/'
-          .. rule.scheme .. ' ICAP/1.0\r\n')
+        table.insert(respond_headers, 1, string.format(
+            'RESPMOD icap://%s:%s/%s ICAP/1.0\r\n', addr:to_string(), addr:get_port(), rule.scheme))
         table.insert(respond_headers, '\r\n')
         table.insert(respond_headers, size .. '\r\n')
         table.insert(respond_headers, content)
@@ -161,7 +160,9 @@ local function icap_check(task, content, digest, rule)
       end
 
       local function add_respond_header(name, value)
-        table.insert(respond_headers, name .. ': ' .. value .. '\r\n' )
+        if name and value then
+          table.insert(respond_headers, string.format('%s: %s\r\n', name, value))
+        end
       end
 
       local function icap_result_header_table(result)
@@ -242,7 +243,7 @@ local function icap_check(task, content, digest, rule)
               '%s: icap X-Virus-ID: %s', rule.log_prefix, icap_headers['X-Virus-ID'])
 
           if string.find(icap_headers['X-Virus-ID'], ', ') then
-            local vnames = lua_util.rspamd_str_split(string.gsub(icap_headers['X-Virus-ID'], "%s", ""), ',') or {}
+            local vnames = lua_util.str_split(string.gsub(icap_headers['X-Virus-ID'], "%s", ""), ',') or {}
 
             for _,v in ipairs(vnames) do
               table.insert(threat_string, v)
@@ -267,7 +268,7 @@ local function icap_check(task, content, digest, rule)
               rule.log_prefix, infection_name, infected_filename)
 
           if string.find(infection_name, ', ') then
-            local vnames = lua_util.rspamd_str_split(infection_name, ',') or {}
+            local vnames = lua_util.str_split(infection_name, ',') or {}
 
             for _,v in ipairs(vnames) do
               table.insert(threat_string, v)
@@ -285,9 +286,9 @@ local function icap_check(task, content, digest, rule)
         end
       end
 
-      local function icap_r_respond_cb(error, data, connection)
-        if error or connection == nil then
-          icap_requery(err, "icap_r_respond_cb")
+      local function icap_r_respond_cb(err_m, data, connection)
+        if err_m or connection == nil then
+          icap_requery(err_m, "icap_r_respond_cb")
         else
           local result = tostring(data)
           conn:close()
@@ -315,17 +316,17 @@ local function icap_check(task, content, digest, rule)
         end
       end
 
-      local function icap_w_respond_cb(error, connection)
-        if error or connection == nil then
-          icap_requery(err, "icap_w_respond_cb")
+      local function icap_w_respond_cb(err_m, connection)
+        if err_m or connection == nil then
+          icap_requery(err_m, "icap_w_respond_cb")
         else
           connection:add_read(icap_r_respond_cb, '\r\n\r\n')
         end
       end
 
-      local function icap_r_options_cb(error, data, connection)
-        if error or connection == nil then
-          icap_requery(err, "icap_r_options_cb")
+      local function icap_r_options_cb(err_m, data, connection)
+        if err_m or connection == nil then
+          icap_requery(err_m, "icap_r_options_cb")
         else
           local icap_headers = icap_result_header_table(tostring(data))
 
@@ -343,7 +344,7 @@ local function icap_check(task, content, digest, rule)
                 local from = task:get_from('mime')
                 local rcpt_to = task:get_principal_recipient()
                 local client = task:get_from_ip()
-                add_respond_header('X-Client-IP', client:to_string())
+                if client then add_respond_header('X-Client-IP', client:to_string()) end
                 add_respond_header('X-Mail-From', from[1].addr)
                 add_respond_header('X-Rcpt-To', rcpt_to)
               end
@@ -384,7 +385,9 @@ local function icap_check(task, content, digest, rule)
     })
   end
 
-  if common.need_check(task, content, rule, digest) then
+  if common.need_check(task, content, rule, digest, icap_check_uncached) then
+    return
+  else
     icap_check_uncached()
   end
 
diff --git a/lualib/lua_scanners/kaspersky_av.lua b/lualib/lua_scanners/kaspersky_av.lua
index cb652f5b6..767ff2a94 100644
--- a/lualib/lua_scanners/kaspersky_av.lua
+++ b/lualib/lua_scanners/kaspersky_av.lua
@@ -178,7 +178,9 @@ local function kaspersky_check(task, content, digest, rule)
     })
   end
 
-  if common.need_check(task, content, rule, digest) then
+  if common.need_check(task, content, rule, digest, kaspersky_check_uncached) then
+    return
+  else
     kaspersky_check_uncached()
   end
 
diff --git a/lualib/lua_scanners/oletools.lua b/lualib/lua_scanners/oletools.lua
index 1f861f6a7..cc973d4d5 100644
--- a/lualib/lua_scanners/oletools.lua
+++ b/lualib/lua_scanners/oletools.lua
@@ -304,7 +304,9 @@ local function oletools_check(task, content, digest, rule)
 
   end
 
-  if common.need_check(task, content, rule, digest) then
+  if common.need_check(task, content, rule, digest, oletools_check_uncached) then
+    return
+  else
     oletools_check_uncached()
   end
 
diff --git a/lualib/lua_scanners/savapi.lua b/lualib/lua_scanners/savapi.lua
index 11f658da3..b36e6e148 100644
--- a/lualib/lua_scanners/savapi.lua
+++ b/lualib/lua_scanners/savapi.lua
@@ -247,7 +247,9 @@ local function savapi_check(task, content, digest, rule)
     })
   end
 
-  if common.need_check(task, content, rule, digest) then
+  if common.need_check(task, content, rule, digest, savapi_check_uncached) then
+    return
+  else
     savapi_check_uncached()
   end
 
diff --git a/lualib/lua_scanners/sophos.lua b/lualib/lua_scanners/sophos.lua
index ff47c262d..60a23c20b 100644
--- a/lualib/lua_scanners/sophos.lua
+++ b/lualib/lua_scanners/sophos.lua
@@ -167,7 +167,9 @@ local function sophos_check(task, content, digest, rule)
     })
   end
 
-  if common.need_check(task, content, rule, digest) then
+  if common.need_check(task, content, rule, digest, sophos_check_uncached) then
+    return
+  else
     sophos_check_uncached()
   end
 
diff --git a/lualib/lua_scanners/spamassassin.lua b/lualib/lua_scanners/spamassassin.lua
index dd4c914bb..06fcf5791 100644
--- a/lualib/lua_scanners/spamassassin.lua
+++ b/lualib/lua_scanners/spamassassin.lua
@@ -180,7 +180,7 @@ local function spamassassin_check(task, content, digest, rule)
             common.save_cache(task, digest, rule, symbols, spam_score)
           else
             local symbols_table = {}
-            symbols_table = rspamd_str_split(symbols, ",")
+            symbols_table = lua_util.str_split(symbols, ",")
             lua_util.debugm(rule.N, task, '%s: returned symbols as table: %s', rule.log_prefix, symbols_table)
 
             common.yield_result(task, rule, symbols_table, spam_score)
@@ -202,7 +202,9 @@ local function spamassassin_check(task, content, digest, rule)
     })
   end
 
-  if common.need_check(task, content, rule, digest) then
+  if common.need_check(task, content, rule, digest, spamassassin_check_uncached) then
+    return
+  else
     spamassassin_check_uncached()
   end
 
diff --git a/lualib/lua_scanners/vadesecure.lua b/lualib/lua_scanners/vadesecure.lua
index 0e539ef46..77a9e4dee 100644
--- a/lualib/lua_scanners/vadesecure.lua
+++ b/lualib/lua_scanners/vadesecure.lua
@@ -152,164 +152,170 @@ local function vade_config(opts)
 end
 
 local function vade_check(task, content, digest, rule)
-  local function vade_url(addr)
-    local url
-    if rule.use_https then
-      url = string.format('https://%s:%d%s', tostring(addr),
-          rule.default_port, rule.url)
-    else
-      url = string.format('http://%s:%d%s', tostring(addr),
-          rule.default_port, rule.url)
+  local function vade_check_uncached()
+    local function vade_url(addr)
+      local url
+      if rule.use_https then
+        url = string.format('https://%s:%d%s', tostring(addr),
+            rule.default_port, rule.url)
+      else
+        url = string.format('http://%s:%d%s', tostring(addr),
+            rule.default_port, rule.url)
+      end
+
+      return url
     end
 
-    return url
-  end
+    local upstream = rule.upstreams:get_upstream_round_robin()
+    local addr = upstream:get_addr()
+    local retransmits = rule.retransmits
 
-  local upstream = rule.upstreams:get_upstream_round_robin()
-  local addr = upstream:get_addr()
-  local retransmits = rule.retransmits
+    local url = vade_url(addr)
+    local hdrs = {}
 
-  local url = vade_url(addr)
-  local hdrs = {}
+    local helo = task:get_helo()
+    if helo then
+      hdrs['X-Helo'] = helo
+    end
+    local mail_from = task:get_from('smtp') or {}
+    if mail_from[1] and #mail_from[1].addr > 1 then
+      hdrs['X-Mailfrom'] = mail_from[1].addr
+    end
 
-  local helo = task:get_helo()
-  if helo then
-    hdrs['X-Helo'] = helo
-  end
-  local mail_from = task:get_from('smtp') or {}
-  if mail_from[1] and #mail_from[1].addr > 1 then
-    hdrs['X-Mailfrom'] = mail_from[1].addr
-  end
+    local rcpt_to = task:get_recipients('smtp')
+    if rcpt_to then
+      hdrs['X-Rcptto'] = {}
+      for _, r in ipairs(rcpt_to) do
+        table.insert(hdrs['X-Rcptto'], r.addr)
+      end
+    end
 
-  local rcpt_to = task:get_recipients('smtp')
-  if rcpt_to then
-    hdrs['X-Rcptto'] = {}
-    for _, r in ipairs(rcpt_to) do
-      table.insert(hdrs['X-Rcptto'], r.addr)
+    local fip = task:get_from_ip()
+    if fip and fip:is_valid() then
+      hdrs['X-Inet'] = tostring(fip)
     end
-  end
 
-  local fip = task:get_from_ip()
-  if fip and fip:is_valid() then
-    hdrs['X-Inet'] = tostring(fip)
-  end
+    local request_data = {
+      task = task,
+      url = url,
+      body = task:get_content(),
+      headers = hdrs,
+      timeout = rule.timeout,
+    }
 
-  local request_data = {
-    task = task,
-    url = url,
-    body = task:get_content(),
-    headers = hdrs,
-    timeout = rule.timeout,
-  }
+    local function vade_callback(http_err, code, body, headers)
 
-  local function vade_callback(http_err, code, body, headers)
+      local function vade_requery()
+        -- set current upstream to fail because an error occurred
+        upstream:fail()
 
-    local function vade_requery()
-      -- set current upstream to fail because an error occurred
-      upstream:fail()
+        -- retry with another upstream until retransmits exceeds
+        if retransmits > 0 then
 
-      -- retry with another upstream until retransmits exceeds
-      if retransmits > 0 then
+          retransmits = retransmits - 1
 
-        retransmits = retransmits - 1
+          lua_util.debugm(rule.name, task,
+              '%s: Request Error: %s - retries left: %s',
+              rule.log_prefix, http_err, retransmits)
 
-        lua_util.debugm(rule.name, task,
-            '%s: Request Error: %s - retries left: %s',
-            rule.log_prefix, http_err, retransmits)
+          -- Select a different upstream!
+          upstream = rule.upstreams:get_upstream_round_robin()
+          addr = upstream:get_addr()
+          url = vade_url(addr)
 
-        -- Select a different upstream!
-        upstream = rule.upstreams:get_upstream_round_robin()
-        addr = upstream:get_addr()
-        url = vade_url(addr)
+          lua_util.debugm(rule.name, task, '%s: retry IP: %s:%s',
+              rule.log_prefix, addr, addr:get_port())
+          request_data.url = url
 
-        lua_util.debugm(rule.name, task, '%s: retry IP: %s:%s',
-            rule.log_prefix, addr, addr:get_port())
-        request_data.url = url
+          http.request(request_data)
+        else
+          rspamd_logger.errx(task, '%s: failed to scan, maximum retransmits '..
+              'exceed', rule.log_prefix)
+          task:insert_result(rule['symbol_fail'], 0.0, 'failed to scan and '..
+              'retransmits exceed')
+        end
+      end
 
-        http.request(request_data)
+      if http_err then
+        vade_requery()
       else
-        rspamd_logger.errx(task, '%s: failed to scan, maximum retransmits '..
-            'exceed', rule.log_prefix)
-        task:insert_result(rule['symbol_fail'], 0.0, 'failed to scan and '..
-            'retransmits exceed')
-      end
-    end
+        -- Parse the response
+        if upstream then upstream:ok() end
+        if code ~= 200 then
+          rspamd_logger.errx(task, 'invalid HTTP code: %s, body: %s, headers: %s', code, body, headers)
+          task:insert_result(rule.symbol_fail, 1.0, 'Bad HTTP code: ' .. code)
+          return
+        end
+        local parser = ucl.parser()
+        local ret, err = parser:parse_string(body)
+        if not ret then
+          rspamd_logger.errx(task, 'vade: bad response body (raw): %s', body)
+          task:insert_result(rule.symbol_fail, 1.0, 'Parser error: ' .. err)
+          return
+        end
+        local obj = parser:get_object()
+        local verdict = obj.verdict
+        if not verdict then
+          rspamd_logger.errx(task, 'vade: bad response JSON (no verdict): %s', obj)
+          task:insert_result(rule.symbol_fail, 1.0, 'No verdict/unknown verdict')
+          return
+        end
+        local vparts = lua_util.str_split(verdict, ":")
+        verdict = table.remove(vparts, 1) or verdict
 
-    if http_err then
-      vade_requery()
-    else
-      -- Parse the response
-      if upstream then upstream:ok() end
-      if code ~= 200 then
-        rspamd_logger.errx(task, 'invalid HTTP code: %s, body: %s, headers: %s', code, body, headers)
-        task:insert_result(rule.symbol_fail, 1.0, 'Bad HTTP code: ' .. code)
-        return
-      end
-      local parser = ucl.parser()
-      local ret, err = parser:parse_string(body)
-      if not ret then
-        rspamd_logger.errx(task, 'vade: bad response body (raw): %s', body)
-        task:insert_result(rule.symbol_fail, 1.0, 'Parser error: ' .. err)
-        return
-      end
-      local obj = parser:get_object()
-      local verdict = obj.verdict
-      if not verdict then
-        rspamd_logger.errx(task, 'vade: bad response JSON (no verdict): %s', obj)
-        task:insert_result(rule.symbol_fail, 1.0, 'No verdict/unknown verdict')
-        return
-      end
-      local vparts = lua_util.rspamd_str_split(verdict, ":")
-      verdict = table.remove(vparts, 1) or verdict
+        local sym = rule.symbols[verdict]
+        if not sym then
+          sym = rule.symbols.other
+        end
 
-      local sym = rule.symbols[verdict]
-      if not sym then
-        sym = rule.symbols.other
-      end
+        if not sym.symbol then
+          -- Subcategory match
+          local lvl = 'low'
+          if vparts and vparts[1] then
+            lvl = vparts[1]
+          end
+
+          if sym[lvl] then
+            sym = sym[lvl]
+          else
+            sym = rule.symbols.other
+          end
+        end
 
-      if not sym.symbol then
-        -- Subcategory match
-        local lvl = 'low'
-        if vparts and vparts[1] then
-          lvl = vparts[1]
+        local opts = {}
+        if obj.score then
+          table.insert(opts, 'score=' .. obj.score)
+        end
+        if obj.elapsed then
+          table.insert(opts, 'elapsed=' .. obj.elapsed)
         end
 
-        if sym[lvl] then
-          sym = sym[lvl]
+        if rule.log_spamcause and obj.spamcause then
+          rspamd_logger.infox(task, 'vadesecure verdict="%s", score=%s, spamcause="%s", message-id="%s"',
+              verdict, obj.score, obj.spamcause, task:get_message_id())
         else
-          sym = rule.symbols.other
+          lua_util.debugm(rule.name, task, 'vadesecure returned verdict="%s", score=%s, spamcause="%s"',
+              verdict, obj.score, obj.spamcause)
         end
-      end
-
-      local opts = {}
-      if obj.score then
-        table.insert(opts, 'score=' .. obj.score)
-      end
-      if obj.elapsed then
-        table.insert(opts, 'elapsed=' .. obj.elapsed)
-      end
 
-      if rule.log_spamcause and obj.spamcause then
-        rspamd_logger.infox(task, 'vadesecure verdict="%s", score=%s, spamcause="%s", message-id="%s"',
-            verdict, obj.score, obj.spamcause, task:get_message_id())
-      else
-        lua_util.debugm(rule.name, task, 'vadesecure returned verdict="%s", score=%s, spamcause="%s"',
-            verdict, obj.score, obj.spamcause)
-      end
+        if #vparts > 0 then
+          table.insert(opts, 'verdict=' .. verdict .. ';' .. table.concat(vparts, ':'))
+        end
 
-      if #vparts > 0 then
-        table.insert(opts, 'verdict=' .. verdict .. ';' .. table.concat(vparts, ':'))
+        task:insert_result(sym.symbol, 1.0, opts)
       end
-
-      task:insert_result(sym.symbol, 1.0, opts)
     end
-  end
 
-  if common.need_check(task, content, rule, digest) then
     request_data.callback = vade_callback
     http.request(request_data)
   end
 
+  if common.need_check(task, content, rule, digest, vade_check_uncached) then
+    return
+  else
+    vade_check_uncached()
+  end
+
 end
 
 return {