The documentation of mod_firewall currently says for IN_ROSTER: > Performance note: this check can potentially cause storage access (especially if the recipient is currently offline), so you may want to limit its use in high-traffic situations, and place it below other checks (such as a rate limiter). However, that doesn't actually work. For example, a filter of: KIND: message TYPE: chat IN_ROSTER: no LOG=Bouncing spam message. BOUNCE. Results in the generated code: do -- File /etc/prosody/blocklist.pfw ---- Chain deliver chain_1_deliver = function (definitions, fire_event, log) local rates = definitions.RATE; local zones = definitions.ZONE; local rostermanager = require "core.rostermanager"; local jid_split = require "util.jid".split; local jid_bare = require "util.jid".bare; local st = require "util.stanza"; local db = require 'util.debug'; return function (event) local stanza, session = event.stanza, event.origin; -- name local name = stanza.name; -- type local type = stanza.attr.type; -- to local to = stanza.attr.to or jid_bare(session.full_jid); -- split_to local to_node, to_host, to_resource = jid_split(to); -- from local from = stanza.attr.from; -- bare_from local bare_from = jid_bare(from) -- roster_entry local roster_entry = (to_node and rostermanager.load_roster(to_node, to_host) or {})[bare_from]; if (name == "message") and ((type or (name == 'message' and 'normal') or (name == 'presence' and 'available')) == "chat") and (not roster_entry) then -- LOG=Bouncing spam message. (session.log or log)("info", "Bouncing spam message."); -- BOUNCE. if type == "error" or (name == "iq" and type == "result") then return true; end -- Don't reply to 'error' stanzas, or iq results local newstanza = st.error_reply(stanza, "cancel", "service-unavailable", nil); session.send(newstanza); return true; end end; end; ---- End of chain deliver end -- End of file /etc/prosody/blocklist.pfw Note that the load_roster call is before the check (name == "message") and ((type or (name == 'message' and 'normal') or (name == 'presence' and 'available')) == "chat"). Therefore the call will be done for every stanza. If load_roster was part of the 'if' conditional, it would not be invoked as the first "false" for the chain of "and"s would terminate it. (It might be possible to do this efficiently using chains or marking sessions, but I think that even the simple case should be optimized.)

I wonder if compiling into a pyramid like this would be better: dependencies for first condition here if (first condition) then dependencies for second condition here if (second condition) then dependencies for third condition if (third condition) then etc ... until the actual action end end end

I believe Matthew considered this kind of issue to be of low priority.

• tags Priority-Low
• owner MattJ

• tags Component-Community