fix(sm): handle SASL2 inline SM resumption and suppress duplicate <enable>#380
Merged
fix(sm): handle SASL2 inline SM resumption and suppress duplicate <enable>#380
Conversation
…able>
Two bugs observed in XMPP log when ejabberd uses SASL2+bind2+FAST:
1. SASL2 inline SM resumption: when <resumed h="N"/> is embedded inside
the SASL2 <success> element, xmpp.js never emits a top-level <resumed/>
nonza and never emits 'online' (entity._ready(true) skips it). Only
sm.emit("resumed") fires on the SM plugin. Connection.ts was not
listening for this event, causing connect() to hang until the 30-second
timeout.
Fix: register sm.on('resumed', ...) alongside the existing sm.on('fail')
listener. The handler calls handleResult(true) to resolve the pending
connection promise and persists the SM state. The nonza-based path is
unaffected (resolved=true guard makes the sm handler a no-op there).
2. Duplicate <enable> after bind2: ejabberd sends a second <stream:features>
containing <sm> after the SASL2+bind2 handshake. xmpp.js's
setupStreamFeature middleware sees it and sends a duplicate <enable>
stanza. The server rejects this with <failed> "Stream management is
already enabled", and the xmpp.js catch block silently clears sm.enabled.
Fix: register a prependListener on 'element' (runs before middleware)
that strips <sm> from post-auth <stream:features> when bind2 is
negotiating SM inline. A bind2SmEnabling flag gates the removal and
self-resets after the first matching features element so only one
<stream:features> is affected.
Regression tests added for both paths in Connection.test.ts. Also adds
prependListener: vi.fn() to createMockXmppClient so the guard
(typeof prependListener === 'function') passes in tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two bugs triggered when ejabberd uses SASL2 (XEP-0388) + bind2 (XEP-0386) + FAST (XEP-0484), observed in a real connection log.
Bug 1 — SASL2 inline SM resumption hangs (
connect()times out after 30s)When
<resumed h="N"/>is embedded inside the SASL2<success>element (XEP-0198 inline resumption), xmpp.js processes it entirely inside the SM plugin: it callssm.emit("resumed")and thenentity._ready(true). The_ready(true)path setsthis.status = "online"without emitting theonlineevent, and no top-level<resumed/>nonza is dispatched either. Connection.ts was not listening tosm.emit("resumed"), soconnect()never resolved.Fix: add
sm.on('resumed', ...)alongside the existingsm.on('fail')listener. The handler callshandleResult(true)(resolves the pending promise as a resumption) and persists SM state. Theresolvedguard makes it a no-op when the traditional nonza path already fired.Bug 2 — Duplicate
<enable>silently disables SMAfter the bind2 handshake completes, ejabberd sends a second
<stream:features>that still includes<sm>. xmpp.js'ssetupStreamFeaturemiddleware sees the<sm>element and sends a duplicate<enable>. The server rejects it with<failed> "Stream management is already enabled", and the xmpp.js catch block clearssm.enabledfor the session.Fix: register a
prependListeneron'element'(fires before middleware) that strips<sm>from the post-auth<stream:features>when bind2 is negotiating SM inline. Abind2SmEnablingflag gates the removal and self-resets after the first matching element so only one<stream:features>is affected.