# Software version: Debian GNU/Linux 8.2 (jessie) Prosody: 0.9.7 Lua: 5.1 hg identify of prosody-modules: 4b838e61c68e tip # Noteable aktive modules: carbons, smacks, compression, csi, filter_chatstates, throttle_presence # Log: ``` Sep 17 19:02:10 s2sinee5b10 debug Received[s2sin]: <presence to='[hidden.user1]@domain.example/tablet' from='conversations@conference.siacs.eu/User2'> Sep 17 19:02:10 domain.example.de:throttle_presence debug Buffering presence stanza from conversations@conference.siacs.eu/User2 to [hidden.user1]@domain.example.de/tablet Sep 17 19:02:10 s2sinee5b10 debug Received[s2sin]: <presence to='[hidden.user1]@domain.example.de/Gajim' from='conversations@conference.siacs.eu/User2'> Sep 17 19:02:10 c2s139e3c0 debug #queue = 1 Sep 17 19:02:10 c2s139e3c0 debug Received[c2s]: <a xmlns='urn:xmpp:sm:2' h='979'> Sep 17 19:02:10 c2s139e3c0 debug #queue = 0 Sep 17 19:02:15 s2sinee5b10 debug Received[s2sin]: <message type='groupchat' to='[hidden.user1]@domain.example.de/tablet' from='conversations@conference.siacs.eu/Hidden user' id='hidden-id1' xml:lang='fr'> Sep 17 19:02:15 mod_s2s error Traceback[s2s]: ...ules/mod_throttle_presence/mod_throttle_presence.lua:12: attempt to index field 'attr' (a nil value) stack traceback: ...ules/mod_throttle_presence/mod_throttle_presence.lua:12: in function '?' /usr/lib/prosody/util/filters.lua:24: in function 'filter' /usr/lib/prosody/core/sessionmanager.lua:35: in function 'send' /usr/lib/prosody/modules/mod_message.lua:71: in function '?' /usr/lib/prosody/util/events.lua:67: in function 'fire_event' /usr/lib/prosody/core/stanza_router.lua:187: in function 'core_post_stanza' /usr/lib/prosody/core/stanza_router.lua:135: in function </usr/lib/prosody/core/stanza_router.lua:56> (tail call): ? [C]: in function 'xpcall' /usr/lib/prosody/modules/mod_s2s/mod_s2s.lua:445: in function 'cb_handlestanza' /usr/lib/prosody/util/xmppstream.lua:187: in function </usr/lib/prosody/util/xmppstream.lua:167> [C]: in function 'parse' /usr/lib/prosody/util/xmppstream.lua:255: in function 'feed' /usr/lib/prosody/modules/mod_s2s/mod_s2s.lua:545: in function 'data' /usr/lib/prosody/modules/mod_s2s/mod_s2s.lua:605: in function </usr/lib/prosody/modules/mod_s2s/mod_s2s.lua:602> (tail call): ? /usr/lib/prosody/net/server_select.lua:854: in function </usr/lib/prosody/net/server_select.lua:836> [C]: in function 'xpcall' /usr/bin/prosody:376: in function 'loop' /usr/bin/prosody:407: in main chunk [C]: ? Sep 17 19:02:15 s2sinee5b10 debug Received[s2sin]: <message type='groupchat' to='[hidden.user1]@domain.example.de/Gajim' from='conversations@conference.siacs.eu/Hidden user' id='hidden-id1' xml:lang='fr'> Sep 17 19:02:15 c2s139e3c0 debug #queue = 1 Sep 17 19:02:15 c2s139e3c0 debug Received[c2s]: <a xmlns='urn:xmpp:sm:2' h='980'> Sep 17 19:02:15 c2s139e3c0 debug #queue = 0 ```

# Changes * `stanza._flush` renamed to `stanza._mod_throttle_presence_flush` to prevent name conflicts with other modules * Simplified `if` in Line 13 (http://hg.prosody.im/prosody-modules/file/991a5f74f848/mod_throttle_presence/mod_throttle_presence.lua) * Prevent the module to handle invalid and server stanzas (the actual fix) # Start Diff diff -r 4b838e61c68e mod_throttle_presence/mod_throttle_presence.lua --- a/mod_throttle_presence/mod_throttle_presence.lua Mon Sep 14 13:38:02 2015 +0200 +++ b/mod_throttle_presence/mod_throttle_presence.lua Mon Sep 21 09:18:24 2015 +0200 @@ -4,25 +4,36 @@ module:depends("csi"); local function presence_filter(stanza, session) - if stanza._flush then - stanza._flush = nil; + -- Ignore internal and invalid stanzas + if not stanza or not stanza.attr or not stanza.attr.from then return stanza; end + local from = stanza.attr.from; + + -- Only handle presence stanzas + if stanza.name == "presence" then + -- Ignore flushed (previously cached) stanzas + if stanza._mod_throttle_presence_flush then + stanza._mod_throttle_presence_flush = nil; + return stanza; + end + + -- cache unavailable + if stanza.attr.type and stanza.attr.type == "unavailable" then + module:log("debug", "Buffering unavailable presence stanza from %s to %s", from, session.full_jid); + session.presence_buffer[from] = st.clone(stanza); + return nil; -- Drop this stanza (we've stored it for later) + end + end + + --flush cached presence local buffer = session.presence_buffer; - local from = stanza.attr.from; - if stanza.name ~= "presence" or (stanza.attr.type and stanza.attr.type ~= "unavailable") then - local cached_presence = buffer[stanza.attr.from]; - if cached_presence then - module:log("debug", "Important stanza for %s from %s, flushing presence", session.full_jid, from); - stanza._flush = true; - cached_presence._flush = true; - session.send(cached_presence); - buffer[stanza.attr.from] = nil; - end - else - module:log("debug", "Buffering presence stanza from %s to %s", stanza.attr.from, session.full_jid); - buffer[stanza.attr.from] = st.clone(stanza); - return nil; -- Drop this stanza (we've stored it for later) + local cached_presence = buffer[from]; + if cached_presence then + module:log("debug", "Important stanza for %s from %s, flushing presence", session.full_jid, from); + cached_presence._mod_throttle_presence_flush = true; + session.send(cached_presence); + buffer[from] = nil; end return stanza; end # END Diff

It would have been nicer to have one patch per change. > * `stanza._flush` renamed to `stanza._mod_throttle_presence_flush` to prevent name conflicts with other modules What does it conflict with? > * Simplified `if` in Line 13 (http://hg.prosody.im/prosody-modules/file/991a5f74f848/mod_throttle_presence/mod_throttle_presence.lua) This patch adds 11 more lines than it removes, explain how it is simpler. Why is it only buffering unavailable presence? This removes the benefit of the module, removing duplicated presence updates while the user in the 'inactive' mode. > * Prevent the module to handle invalid and server stanzas (the actual fix) How? What triggers the traceback? Strings?

• tag Status-Accepted

> It would have been nicer to have one patch per change. Yeah, you are right. > > * `stanza._flush` renamed to `stanza._mod_throttle_presence_flush` to > > prevent name conflicts with other modules > What does it conflict with? Currently with none, but adding attributes to global objects without distinct namespaces can easily lead to hard to debug errors (If there is a conflict later). > > * Simplified `if` in Line 13 > > (http://hg.prosody.im/prosody-modules/file/991a5f74f848/mod_throttle_pres > > ence/mod_throttle_presence.lua) > This patch adds 11 more lines than it removes, explain how it is simpler. The `if` is not easy to understand! (See below) > Why is it only buffering unavailable presence? Because the current module does only buffer unavailable presence. Say stanza.name = "presence" and stanza.attr.type = "probe": Computing the if: 1. if stanza.name ~= "presence" or (stanza.attr.type and stanza.attr.type ~= "unavailable") 2. if false or (stanza.attr.type and stanza.attr.type ~= "unavailable") 3. if stanza.attr.type and stanza.attr.type ~= "unavailable" 4. if true and stanza.attr.type ~= "unavailable" 5. if stanza.attr.type ~= "unavailable" 4. if true => Flush the buffer > > * Prevent the module to handle invalid and server stanzas (the actual fix) > > How? Only these lines are relevant for the fix: if not stanza or not stanza.attr or not stanza.attr.from then return stanza; end > What triggers the traceback? Strings? Example stanzas: * <iq id='[...]' type='result' to='[...]'><blocklist xmlns='urn:xmpp:blocking'/></iq> * <a h='11' xmlns='urn:xmpp:sm:3'/> * And empty stanzas (tostring(stanza) returns nothing), I assume they come from http://hg.prosody.im/prosody-modules/file/991a5f74f848/mod_filter_chatstates/mod_filter_chatstates.lua#l15

I have to correct myself. (@Example stanza) The `iq` and `a` stanzas didn't lead to the crash! I had them just in mind because they lead to a crash in a failed fix attempt. So they have nothing to do with the original issue.

If we invert the logic, the condition is like this if stanza is <presence> or <presence type="unavailable"> then buffer it else flush buffer end Anyways, can you describe how to reproduce the traceback?

> Can you describe how to reproduce the traceback? Active modules: csi, filter_chatstates, throttle_presence 1. Join a Muc 2. Switch to csi-client-inactive mode 3. Other user starts and stops typing in the same muc. (Importen is that the client sends is-typing and stoped-typing messages) My first user (in csi-client-inactive mode) is a Conversations client. My second user a telepathy client.

It is indeed the "" from mod_filter_chatstates that causes it. However fixing things bring back the reason for this hack, that prosody thinks sending the thing failed and returns an error, which is not what you want. Especially not with MUCs, because that gets you kicked out of the room.

These commits should have fixed this. https://hg.prosody.im/prosody-modules/rev/e48dbb640408 https://hg.prosody.im/0.9/rev/caee8a32983a

• tags Status-Fixed