OF-3359: Do not let unofferable FAST mechanisms shape the stream features - #3477
OF-3359: Do not let unofferable FAST mechanisms shape the stream features#3477guusdk wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthrough
Merge Risk: ⚪ Minimal · up to SASL stream features now advertise FAST mechanisms only when available for the session, avoiding incorrect mechanism and channel-binding advertisement. The covered SASL1 and SASL2 cases indicate no remaining merge-blocking risk. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🟢 Approval recommended
The implementation aligns offered/recorded SASL state with what is actually rendered on the wire, and the new tests cover the previously incorrect FAST-driven edge cases.
Pull request overview
This PR fixes how SASL stream features are computed/recorded when FAST (XEP-0484) mechanisms are eligible but not actually offerable (notably when SASL2 isn’t being advertised). It ensures that only mechanisms that will truly be offered influence (a) channel-binding capability advertisement/recording and (b) suppression of empty SASL1 <mechanisms/> elements.
Changes:
- Filter FAST mechanisms out of the “offered” set unless the SASL2 inline feature is actually advertised, preventing FAST-only mechanisms from affecting channel-binding type advertisement/recording.
- Adjust SASL1 empty-element suppression to consider only standard (non-FAST) mechanisms, as FAST mechanisms are never rendered as SASL1
<mechanism/>children. - Add/extend tests to cover FAST-only eligibility scenarios for SASL1/SASL2 and to verify channel-binding/FAST advertisement state is recorded consistently.
File summaries
| File | Description |
|---|---|
| xmppserver/src/main/java/org/jivesoftware/openfire/net/SaslStreamFeatures.java | Ensures only actually-offered mechanisms (and only SASL2 when applicable) influence rendered features and recorded advertised state. |
| xmppserver/src/test/java/org/jivesoftware/openfire/net/SaslStreamFeaturesTest.java | Adds regression coverage for FAST-only eligibility and for avoiding FAST-driven channel-binding advertisement when SASL2 is unavailable. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ures The FAST mechanisms were carried into two calculations that cannot use them: the channel-binding types, which were derived from HT-*-UNIQ/ENDP/EXPR variants even when the FAST inline feature was not being advertised, and the emptiness check that suppresses a SASL1 mechanisms element, which counted mechanisms that a SASL1 element never renders. appendSASLFeatures now passes on only what is actually offered, and the element builder distinguishes the two profiles rather than re-deciding whether FAST is enabled.
08f74f1 to
7f61fa5
Compare
|
Rebased |
The FAST mechanisms were carried into two calculations that cannot use them: the channel-binding types, which were derived from HT-*-UNIQ/ENDP/EXPR variants even when the FAST inline feature was not being advertised, and the emptiness check that suppresses a SASL1 mechanisms element, which counted mechanisms that a SASL1 element never renders.
appendSASLFeatures now passes on only what is actually offered, and the element builder distinguishes the two profiles rather than re-deciding whether FAST is enabled.