Skip to content

OF-3222: Improve XEP-0359 stanza-id handling for one-on-one routing and archiving #3234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
guusdk wants to merge 1 commit into
base: main
Choose a base branch
from
+27 −2
Open

OF-3222: Improve XEP-0359 stanza-id handling for one-on-one routing and archiving #3234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion xmppserver/src/main/java/org/jivesoftware/openfire/MessageRouter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2005-2008 Jive Software, 2017-2025 Ignite Realtime Foundation. All rights reserved.
* Copyright (C) 2005-2008 Jive Software, 2017-2026 Ignite Realtime Foundation. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -23,6 +23,7 @@
import org.jivesoftware.openfire.interceptor.PacketRejectedException;
import org.jivesoftware.openfire.session.ClientSession;
import org.jivesoftware.openfire.session.LocalClientSession;
import org.jivesoftware.openfire.stanzaid.StanzaIDUtil;
import org.jivesoftware.openfire.user.UserManager;
import org.jivesoftware.util.JiveGlobals;
import org.slf4j.Logger;
Expand Down Expand Up @@ -131,6 +132,20 @@ public void route(Message packet) {
routingFailed(recipientJID, packet);
}

// For locally-originated messages to remote recipients, add sender-owned stanza-id to the packet
// AFTER routing has completed (so it was never present on the wire, satisfying XEP-0359 MUST-NOT),
// but BEFORE post-processing interceptors run (so archivers store XML that includes the stanza-id).
// For local-to-local, RoutingTableImpl already added a recipient-owned stanza-id during routePacket();
// no sender-owned ID is added to avoid dual ownership. For component-bound messages (e.g. MUC at
// conference.example.com), the component itself manages stanza-ids (e.g. MultiUserChatServiceImpl);
// 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)) {
Comment on lines +141 to +145
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
// 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)) {

Copilot uses AI. Check for mistakes.
StanzaIDUtil.ensureUniqueAndStableStanzaID(packet, packet.getFrom().asBareJID());
}

// Sent carbon copies to other resources of the sender:
if (session != null && Forwarded.isEligibleForCarbonsDelivery(packet)) {
List<JID> routes = routingTable.getRoutes(packet.getFrom().asBareJID(), null);
Expand Down
12 changes: 11 additions & 1 deletion xmppserver/src/main/java/org/jivesoftware/openfire/spi/RoutingTableImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2005-2008 Jive Software, 2016-2025 Ignite Realtime Foundation. All rights reserved.
* Copyright (C) 2005-2008 Jive Software, 2016-2026 Ignite Realtime Foundation. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -33,6 +33,7 @@
import org.jivesoftware.openfire.server.OutgoingSessionPromise;
import org.jivesoftware.openfire.server.RemoteServerManager;
import org.jivesoftware.openfire.session.*;
import org.jivesoftware.openfire.stanzaid.StanzaIDUtil;
import org.jivesoftware.util.JiveGlobals;
import org.jivesoftware.util.LocaleUtils;
import org.jivesoftware.util.TaskEngine;
Expand Down Expand Up @@ -364,6 +365,11 @@ private boolean routeToLocalDomain(JID jid, Packet packet)
}
else {
// RFC 6121 section 8.5.3. localpart@domainpart/resourcepart (Packet sent to a full JID of a user)
// 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());
}
Comment on lines +368 to +371
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.

ClientRoute clientRoute = getClientRouteForLocalUser(jid);
if (clientRoute != null) {
// RFC-6121 section 8.5.3.1. Resource Matches
Expand Down Expand Up @@ -624,6 +630,10 @@ private boolean routeToRemoteDomain(JID jid, Packet packet) {
* @return true if at least one target session was found
*/
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());

Comment on lines 632 to +636
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
List<ClientSession> sessions = new ArrayList<>();
// Get existing AVAILABLE sessions of this user or AVAILABLE to the sender of the packet
for (JID address : getRoutes(recipientJID, packet.getFrom())) {
Expand Down
Loading