mailcow / mailcow-dockerized

mailcow: dockerized - 🐮 + 🐋 = 💕
https://mailcow.email
GNU General Public License v3.0
8.25k stars 1.12k forks source link

[Rspamd] Delete overriding obsolete rspamd plugin #5900

Closed realizelol closed 1 week ago

realizelol commented 3 weeks ago

After about 3 and a half years the rspamd plugin file /usr/share/rspamd/plugins/metadata_exporter.lua still gets overridden by data/Dockerfiles/rspamd/metadata_exporter.lua. By this commit https://github.com/mailcow/mailcow-dockerized/commit/d85241f this should be a temporary fix which also should be merged upstream by https://github.com/rspamd/rspamd/commit/8683c46.

The only thing is that you'll be informed by "unknown" instead empty '' on r.fuzzy:

upstream

  local fuzzy = task:get_mempool():get_variable("fuzzy_hashes", "fstrings")
  if fuzzy and #fuzzy > 0 then
    local fz = {}
    for _, h in ipairs(fuzzy) do
      table.insert(fz, h)
    end
    if not flatten then
      r.fuzzy = fz
    else
      r.fuzzy = table.concat(fz, ', ')
    end
  else
    if not flatten then
      r.fuzzy = {}
    else
      r.fuzzy = ''
    end
  end

local

  local fuzzy = task:get_mempool():get_variable("fuzzy_hashes", "fstrings")
  if fuzzy and #fuzzy > 0 then
    local fz = {}
    for _,h in ipairs(fuzzy) do
      table.insert(fz, h)
    end
    if not flatten then
      r.fuzzy = fz
    else
      r.fuzzy = table.concat(fz, ', ')
    end
  else
    r.fuzzy = 'unknown'
  end

But this should not be the reason of overriding improved code which is also still in maintenance.

dragoangel commented 3 weeks ago

@DerLinkman after merging if quarantine still works and get fuzzy hashes we can assume everything is okay.

We can add extra handling for unknown string to not display it in quarantine as well.

https://github.com/mailcow/mailcow-dockerized/blob/36b5cccd186090d726de62b6b00d1e842e67aacd/data/conf/rspamd/local.d/metadata_exporter.conf#L4

realizelol commented 3 weeks ago

We can add extra handling for unknown string to not display it in quarantine as well.

Maybe I unterstand you wrong. But the current code has the 'unknown' value on the upstream part it's ''|{} (empty). I don't know where exactly the variable r.fuzzy will be used. But if I'll give it a short review in the mentioned code snippet: This will fix the output of fuzzy_hashes "higher than 0" in the result value or array e.g. {1, 2, 3} and if this is zero or negative value it result in whether ''(single) or {}(array) emptiness instead of a 0 or a negative value.

And actually the quarantine will not being processed if the fuzzy_hashes has the unknown flag. So the same could be done if the value is '' or {} => empty ?