OF-3222: Improve XEP-0359 stanza-id handling for one-on-one routing and archiving#3234
OF-3222: Improve XEP-0359 stanza-id handling for one-on-one routing and archiving#3234guusdk wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Improves XEP-0359 stanza-id generation/injection for one-on-one messaging flows to better support routing and archiving behavior without altering outbound S2S wire stanzas.
Changes:
- Add recipient-owned stanza-id generation when routing direct messages to local bare JIDs and full JIDs.
- Add sender-owned stanza-id generation for locally-originated messages to remote domains after routing completes (to avoid modifying S2S wire traffic), while still allowing post-processing interceptors to archive stanza-ids.
- Exclude subdomains of the local server domain from sender-owned stanza-id injection to avoid duplicating component-managed stanza-id behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| xmppserver/src/main/java/org/jivesoftware/openfire/spi/RoutingTableImpl.java | Adds recipient-owned stanza-id generation during local bare/full JID routing. |
| xmppserver/src/main/java/org/jivesoftware/openfire/MessageRouter.java | Adds post-routing sender-owned stanza-id injection for local-to-remote messages (with subdomain exclusion). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Add stanza ID for one-on-one messages sent to full JIDs as per XEP-0359 | ||
| if (packet instanceof Message message) { | ||
| StanzaIDUtil.ensureUniqueAndStableStanzaID(message, jid.asBareJID()); | ||
| } |
There was a problem hiding this comment.
The stanza-id is added for every Message routed to a local full JID. That includes non one-on-one traffic like MUC groupchat messages (which are typically routed to local users as full JIDs), causing an extra recipient-owned stanza-id with by=<user@domain> to be added in addition to any MUC-assigned stanza-id. If the intent is “direct messages” only (as per PR description), restrict this to appropriate message types (eg chat/normal) and/or explicitly exclude groupchat (and possibly error).
| private boolean routeToBareJID(JID recipientJID, Message packet) { | ||
| // Add stanza ID for one-on-one messages as per XEP-0359 | ||
| // The assigning entity for one-on-one messages is the receiving user's bare JID | ||
| StanzaIDUtil.ensureUniqueAndStableStanzaID(packet, recipientJID.asBareJID()); | ||
|
|
There was a problem hiding this comment.
ensureUniqueAndStableStanzaID is invoked before early-return cases that discard/don't route certain message types (eg error and groupchat). Consider moving stanza-id generation to after those checks so dropped stanzas aren’t mutated and to avoid unnecessary UUID generation/work.
| // exclude those too by checking that the recipient domain is not a subdomain of this server. OF-3222 | ||
| if (packet.getFrom() != null | ||
| && serverName.equals(packet.getFrom().getDomain()) | ||
| && !serverName.equals(recipientJID.getDomain()) | ||
| && !recipientJID.getDomain().endsWith("." + serverName)) { |
There was a problem hiding this comment.
The condition !recipientJID.getDomain().endsWith("." + serverName) treats any subdomain of the local domain as a local component. That can incorrectly skip sender-owned stanza-id injection for legitimate remote domains that happen to be subdomains (e.g. user@chat.example.com when serverName is example.com). Instead of a string suffix check, use an explicit local-component test (e.g. XMPPServer.getInstance().matchesComponent(recipientJID) and/or routingTable.hasComponentRoute(recipientJID)) to exclude only domains that are actually hosted components.
| // exclude those too by checking that the recipient domain is not a subdomain of this server. OF-3222 | |
| if (packet.getFrom() != null | |
| && serverName.equals(packet.getFrom().getDomain()) | |
| && !serverName.equals(recipientJID.getDomain()) | |
| && !recipientJID.getDomain().endsWith("." + serverName)) { | |
| // exclude those too by only excluding recipients that are actually hosted as local components. OF-3222 | |
| if (packet.getFrom() != null | |
| && serverName.equals(packet.getFrom().getDomain()) | |
| && !serverName.equals(recipientJID.getDomain()) | |
| && !XMPPServer.getInstance().matchesComponent(recipientJID) | |
| && !routingTable.hasComponentRoute(recipientJID)) { |
…nd archiving Add stanza-id generation for direct messages delivered to local bare and full JIDs using recipient bare JID ownership. Add sender-owned stanza-id for local-to-remote messages only after routing completes so outbound wire stanzas are not modified while post-processing interceptors can archive IDs. Exclude local component subdomains from sender-owned stanza-id injection so existing MUC stanza-id handling in MultiUserChatServiceImpl remains authoritative and is not duplicated.
akrherz
force-pushed
the
OF-3222_StanzaID-on-one-on-one-messages
branch
from
34222bc to
19a40d1
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe pull request adds XEP-0359 stanza ID assignment to the Openfire message routing system. MessageRouter now conditionally assigns sender-owned stanza IDs to locally-originated messages being delivered to remote recipients, based on domain matching and ownership constraints. RoutingTableImpl assigns recipient-based stanza IDs at two routing decision points: for full JID delivery before client route lookup, and for bare JID routing before session selection. Both files update copyright years and import StanzaIDUtil to support the new stanza ID behavior. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
rebased to see what the rabbit thinks... |
Add stanza-id generation for direct messages delivered to local bare and full JIDs using recipient bare JID ownership.
Add sender-owned stanza-id for local-to-remote messages only after routing completes so outbound wire stanzas are not modified while post-processing interceptors can archive IDs.
Exclude local component subdomains from sender-owned stanza-id injection so existing MUC stanza-id handling in MultiUserChatServiceImpl remains authoritative and is not duplicated.