From b58dffeb803d5132aaad79a747f8a53156a4cb6e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 13:59:17 +0000 Subject: [PATCH 1/4] Initial plan From 0450a454bb8c720a821021ff7c47d3749261f471 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 14:24:31 +0000 Subject: [PATCH 2/4] feat(muc): XEP-0045 compliance fixes and tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix: semi-anonymous room member list protection (XEP-0045 §5.2.1) Members SHOULD NOT retrieve the member list in semi-anonymous rooms since it exposes full JIDs. Only admins/owners are now permitted. (IQAdminHandler.java) - Add: muc#roomconfig_changesubject to extended disco info (XEP-0045 §6.4) The room's 'subject can be modified' setting is now advertised in the muc#roominfo data form returned during service discovery. (MultiUserChatServiceImpl.java, openfire_i18n.properties) - Tests: 38 new unit tests for join preconditions and self-presence status codes covering XEP-0045 §7.2.2/3/5/6/7/8/9/10/12 (MUCRoomJoinTest.java) Co-authored-by: akrherz <210858+akrherz@users.noreply.github.com> --- .../main/resources/openfire_i18n.properties | 1 + .../openfire/muc/spi/IQAdminHandler.java | 7 + .../muc/spi/MultiUserChatServiceImpl.java | 6 + .../openfire/muc/MUCRoomJoinTest.java | 842 ++++++++++++++++++ 4 files changed, 856 insertions(+) create mode 100644 xmppserver/src/test/java/org/jivesoftware/openfire/muc/MUCRoomJoinTest.java diff --git a/i18n/src/main/resources/openfire_i18n.properties b/i18n/src/main/resources/openfire_i18n.properties index 261236a47e..dafee6ee9d 100644 --- a/i18n/src/main/resources/openfire_i18n.properties +++ b/i18n/src/main/resources/openfire_i18n.properties @@ -319,6 +319,7 @@ muc.roomIsNowMembersOnly=This room is now members-only. muc.extended.info.desc=Description muc.extended.info.subject=Subject +muc.extended.info.changesubject=Subject can be modified muc.extended.info.occupants=Number of occupants muc.extended.info.creationdate=Creation date muc.extended.info.avatarhash=Avatar hash diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/IQAdminHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/IQAdminHandler.java index 3740bfe913..dc1a527358 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/IQAdminHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/IQAdminHandler.java @@ -180,6 +180,13 @@ private void handleItemsElement(@Nonnull final Affiliation senderAffiliation, @N && Affiliation.owner != senderAffiliation) { throw new ForbiddenException(); } + // XEP-0045 §5.2.1: In semi-anonymous rooms, members SHOULD NOT be allowed to + // retrieve the member list (as that list MUST contain the JID of each member). + if (!room.canAnyoneDiscoverJID() + && Affiliation.admin != senderAffiliation + && Affiliation.owner != senderAffiliation) { + throw new ForbiddenException(); + } for (JID jid : room.getMembers()) { if (GroupJID.isGroup(jid)) { try { diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/MultiUserChatServiceImpl.java b/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/MultiUserChatServiceImpl.java index 754d2fd5ca..c90dba6784 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/MultiUserChatServiceImpl.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/muc/spi/MultiUserChatServiceImpl.java @@ -3144,6 +3144,12 @@ public Set getExtendedInfos(String name, String node, JID senderJID) { fieldSubj.setLabel(LocaleUtils.getLocalizedString("muc.extended.info.subject", preferredLocale)); fieldSubj.addValue(room.getSubject()); + final FormField fieldChangeSubject = dataForm.addField(); + fieldChangeSubject.setVariable("muc#roomconfig_changesubject"); + fieldChangeSubject.setType(FormField.Type.boolean_type); + fieldChangeSubject.setLabel(LocaleUtils.getLocalizedString("muc.extended.info.changesubject", preferredLocale)); + fieldChangeSubject.addValue(room.canOccupantsChangeSubject() ? "1" : "0"); + final FormField fieldOcc = dataForm.addField(); fieldOcc.setVariable("muc#roominfo_occupants"); fieldOcc.setType(FormField.Type.text_single); diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/muc/MUCRoomJoinTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/muc/MUCRoomJoinTest.java new file mode 100644 index 0000000000..b0578d94f8 --- /dev/null +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/muc/MUCRoomJoinTest.java @@ -0,0 +1,842 @@ +/* + * Copyright (C) 2025 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jivesoftware.openfire.muc; + +import org.dom4j.Element; +import org.jivesoftware.openfire.XMPPServer; +import org.jivesoftware.openfire.group.ConcurrentGroupList; +import org.jivesoftware.openfire.group.ConcurrentGroupMap; +import org.jivesoftware.openfire.user.UserAlreadyExistsException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.xmpp.packet.JID; +import org.xmpp.packet.Presence; + +import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +/** + * Unit tests that verify the XEP-0045 compliance of {@link MUCRoom}'s join precondition checks + * and self-presence status code generation. + * + * References: + * + */ +@ExtendWith(MockitoExtension.class) +public class MUCRoomJoinTest { + + @Mock(strictness = Mock.Strictness.LENIENT) + private MultiUserChatService mockService; + + @Mock(strictness = Mock.Strictness.LENIENT) + private XMPPServer xmppServer; + + private MUCRoom room; + + private static final JID ROOM_JID = new JID("testroom", "conference.example.org", null); + private static final JID USER_A = new JID("usera@example.org/res1"); + private static final JID USER_B = new JID("userb@example.org/res1"); + + @BeforeEach + public void setup() throws Exception { + doReturn("conference").when(mockService).getServiceName(); + doReturn("conference.example.org").when(mockService).getServiceDomain(); + doReturn(null).when(mockService).getMUCDelegate(); + doReturn(false).when(mockService).isSysadmin(any(JID.class)); + doReturn(true).when(mockService).isPasswordRequiredForSysadminsToJoinRoom(); + + //noinspection deprecation + XMPPServer.setInstance(xmppServer); + + room = new MUCRoom(); + setField(room, "mucService", mockService); + setField(room, "name", "testroom"); + setField(room, "owners", new ConcurrentGroupList<>()); + setField(room, "admins", new ConcurrentGroupList<>()); + setField(room, "members", new ConcurrentGroupMap<>()); + setField(room, "outcasts", new ConcurrentGroupList<>()); + setField(room, "occupants", new CopyOnWriteArrayList<>()); + setField(room, "lockedTime", 0L); + setField(room, "maxUsers", 0); // 0 = unlimited + setField(room, "membersOnly", false); + setField(room, "password", null); + setField(room, "loginRestrictedToNickname", false); + setField(room, "canAnyoneDiscoverJID", true); + setField(room, "logEnabled", false); + setField(room, "isDestroyed", false); + } + + // ------------------------------------------------------------------------- + // XEP-0045 §7.2.7: Banned Users + // "If the user has been banned from the room (i.e., has an affiliation of + // 'outcast'), the service MUST deny access to the room and inform the user + // of the fact that they are banned; this is done by returning a presence + // stanza of type 'error' specifying a error condition" + // ------------------------------------------------------------------------- + + @Test + public void testJoinPrecondition_OutcastCannotJoin() throws Exception { + // XEP-0045 §7.2.7: A banned user (outcast) MUST be denied access. + assertThrowsPrecondition( + "checkJoinRoomPreconditionIsOutcast", + ForbiddenException.class, + USER_A, Affiliation.outcast + ); + } + + @Test + public void testJoinPrecondition_NonOutcastCanJoin() throws Exception { + // Non-outcasts MUST be allowed to pass the outcast check. + assertNoPreconditionException( + "checkJoinRoomPreconditionIsOutcast", + USER_A, Affiliation.none + ); + assertNoPreconditionException( + "checkJoinRoomPreconditionIsOutcast", + USER_A, Affiliation.member + ); + assertNoPreconditionException( + "checkJoinRoomPreconditionIsOutcast", + USER_A, Affiliation.admin + ); + assertNoPreconditionException( + "checkJoinRoomPreconditionIsOutcast", + USER_A, Affiliation.owner + ); + } + + // ------------------------------------------------------------------------- + // XEP-0045 §7.2.6: Members-Only Rooms + // "If the room is members-only but the user is not on the member list, the + // service MUST deny access to the room and inform the user that they are + // not allowed to enter the room; this is done by returning a presence + // stanza of type 'error' specifying a error" + // ------------------------------------------------------------------------- + + @Test + public void testJoinPrecondition_MembersOnlyRoom_NonMember_MustBeRejected() throws Exception { + // XEP-0045 §7.2.6: Unaffiliated user MUST NOT enter a members-only room. + setField(room, "membersOnly", true); + + assertThrowsPrecondition( + "checkJoinRoomPreconditionMemberOnly", + RegistrationRequiredException.class, + USER_A, Affiliation.none + ); + } + + @Test + public void testJoinPrecondition_MembersOnlyRoom_Member_CanJoin() throws Exception { + // XEP-0045 §7.2.6: A member MUST be allowed to enter a members-only room. + setField(room, "membersOnly", true); + + assertNoPreconditionException( + "checkJoinRoomPreconditionMemberOnly", + USER_A, Affiliation.member + ); + } + + @Test + public void testJoinPrecondition_MembersOnlyRoom_Admin_CanJoin() throws Exception { + // XEP-0045 §7.2.6: Admins MUST be allowed to enter a members-only room. + setField(room, "membersOnly", true); + + assertNoPreconditionException( + "checkJoinRoomPreconditionMemberOnly", + USER_A, Affiliation.admin + ); + } + + @Test + public void testJoinPrecondition_MembersOnlyRoom_Owner_CanJoin() throws Exception { + // XEP-0045 §7.2.6: Owners MUST be allowed to enter a members-only room. + setField(room, "membersOnly", true); + + assertNoPreconditionException( + "checkJoinRoomPreconditionMemberOnly", + USER_A, Affiliation.owner + ); + } + + @Test + public void testJoinPrecondition_OpenRoom_NonMember_CanJoin() throws Exception { + // An open room allows unaffiliated users. + setField(room, "membersOnly", false); + + assertNoPreconditionException( + "checkJoinRoomPreconditionMemberOnly", + USER_A, Affiliation.none + ); + } + + // ------------------------------------------------------------------------- + // XEP-0045 §7.2.9: Max Users + // "If the room has reached its maximum number of occupants, the service + // SHOULD deny access to the room and inform the user of the restriction; + // this is done by returning a presence stanza of type 'error' specifying + // a error condition" + // "if the room has reached its maximum number of occupants and a room admin + // or owner attempts to join, the room MUST allow the admin or owner to join" + // ------------------------------------------------------------------------- + + @Test + public void testJoinPrecondition_MaxUsersReached_ShouldBeDenied() throws Exception { + // XEP-0045 §7.2.9: When max occupants is reached, new users SHOULD be denied. + setField(room, "maxUsers", 1); + room.occupants.add(createDummyOccupant("existing-user", new JID("existing@example.org"))); + + assertThrowsPrecondition( + "checkJoinRoomPreconditionMaxOccupants", + ServiceUnavailableException.class, + USER_A + ); + } + + @Test + public void testJoinPrecondition_MaxUsersReached_AdminCanJoin() throws Exception { + // XEP-0045 §7.2.9: Admins MUST be allowed to join even when the room is full. + setField(room, "maxUsers", 1); + room.occupants.add(createDummyOccupant("existing-user", new JID("existing@example.org"))); + + final ConcurrentGroupList admins = new ConcurrentGroupList<>(); + admins.add(USER_A.asBareJID()); + setField(room, "admins", admins); + + assertNoPreconditionException( + "checkJoinRoomPreconditionMaxOccupants", + USER_A + ); + } + + @Test + public void testJoinPrecondition_MaxUsersReached_OwnerCanJoin() throws Exception { + // XEP-0045 §7.2.9: Owners MUST be allowed to join even when the room is full. + setField(room, "maxUsers", 1); + room.occupants.add(createDummyOccupant("existing-user", new JID("existing@example.org"))); + + final ConcurrentGroupList owners = new ConcurrentGroupList<>(); + owners.add(USER_A.asBareJID()); + setField(room, "owners", owners); + + assertNoPreconditionException( + "checkJoinRoomPreconditionMaxOccupants", + USER_A + ); + } + + @Test + public void testJoinPrecondition_UnlimitedUsers_CanJoin() throws Exception { + // maxUsers == 0 means no limit. + setField(room, "maxUsers", 0); + room.occupants.add(createDummyOccupant("user1", new JID("u1@example.org"))); + room.occupants.add(createDummyOccupant("user2", new JID("u2@example.org"))); + + assertNoPreconditionException( + "checkJoinRoomPreconditionMaxOccupants", + USER_A + ); + } + + // ------------------------------------------------------------------------- + // XEP-0045 §7.2.10: Locked Room + // "If a user attempts to enter a room while it is 'locked' (i.e., before + // the room creator provides an initial configuration), the service MUST + // refuse entry and return an error to the user" + // ------------------------------------------------------------------------- + + @Test + public void testJoinPrecondition_LockedRoom_NonOwner_MustBeRejected() throws Exception { + // XEP-0045 §7.2.10: Non-owners MUST NOT enter a locked room. + setField(room, "lockedTime", System.currentTimeMillis()); + + assertThrowsPrecondition( + "checkJoinRoomPreconditionLocked", + RoomLockedException.class, + USER_A + ); + } + + @Test + public void testJoinPrecondition_LockedRoom_Owner_CanJoin() throws Exception { + // XEP-0045 §7.2.10: The room owner is allowed to enter a locked room. + setField(room, "lockedTime", System.currentTimeMillis()); + final ConcurrentGroupList owners = new ConcurrentGroupList<>(); + owners.add(USER_A.asBareJID()); + setField(room, "owners", owners); + + assertNoPreconditionException( + "checkJoinRoomPreconditionLocked", + USER_A + ); + } + + @Test + public void testJoinPrecondition_UnlockedRoom_CanJoin() throws Exception { + // When the room is not locked, any user passes this check. + setField(room, "lockedTime", 0L); + + assertNoPreconditionException( + "checkJoinRoomPreconditionLocked", + USER_A + ); + } + + // ------------------------------------------------------------------------- + // XEP-0045 §7.2.8: Nickname Conflict + // "If the room already contains another user with the nickname desired by + // the user seeking to enter the room (or if the nickname is reserved by + // another user), the service MUST deny access to the room and inform the + // user of the conflict; this is done by returning a presence stanza of + // type 'error' specifying a error condition" + // "if the bare JID of the present occupant matches the bare JID of the user + // seeking to enter the room, then the service SHOULD allow entry" + // ------------------------------------------------------------------------- + + @Test + public void testJoinPrecondition_NicknameInUse_DifferentUser_MustBeRejected() throws Exception { + // XEP-0045 §7.2.8: A different user already has this nickname → conflict. + room.occupants.add(createDummyOccupant("witch", new JID("other@example.org"))); + + assertThrowsPrecondition( + "checkJoinRoomPreconditionNicknameInUse", + UserAlreadyExistsException.class, + USER_A, "witch" + ); + } + + @Test + public void testJoinPrecondition_NicknameInUse_SameBareJID_CanJoin() throws Exception { + // XEP-0045 §7.2.8: Same bare JID, different resource → multi-session join, allowed. + room.occupants.add(createDummyOccupant("witch", new JID("usera@example.org/res2"))); + + assertNoPreconditionException( + "checkJoinRoomPreconditionNicknameInUse", + USER_A, "witch" + ); + } + + @Test + public void testJoinPrecondition_NicknameNotInUse_CanJoin() throws Exception { + // No occupant has this nickname. + room.occupants.add(createDummyOccupant("otherwitch", new JID("other@example.org"))); + + assertNoPreconditionException( + "checkJoinRoomPreconditionNicknameInUse", + USER_A, "witch" + ); + } + + // ------------------------------------------------------------------------- + // XEP-0045 §7.2.5: Password-Protected Rooms + // "If the room requires a password and the user did not supply one (or the + // password provided is incorrect), the service MUST deny access to the + // room and inform the user that they are unauthorized; this is done by + // returning a presence stanza of type 'error' specifying a + // error" + // ------------------------------------------------------------------------- + + @Test + public void testJoinPrecondition_WrongPassword_MustBeRejected() throws Exception { + // XEP-0045 §7.2.5: Wrong password MUST be rejected. + setField(room, "password", "secret"); + + assertThrowsPrecondition( + "checkJoinRoomPreconditionPasswordProtection", + org.jivesoftware.openfire.auth.UnauthorizedException.class, + USER_A, "wrongpassword" + ); + } + + @Test + public void testJoinPrecondition_NoPasswordProvided_MustBeRejected() throws Exception { + // XEP-0045 §7.2.5: Missing password MUST be rejected. + setField(room, "password", "secret"); + + assertThrowsPrecondition( + "checkJoinRoomPreconditionPasswordProtection", + org.jivesoftware.openfire.auth.UnauthorizedException.class, + USER_A, (Object) null + ); + } + + @Test + public void testJoinPrecondition_CorrectPassword_CanJoin() throws Exception { + // Correct password passes the check. + setField(room, "password", "secret"); + + assertNoPreconditionException( + "checkJoinRoomPreconditionPasswordProtection", + USER_A, "secret" + ); + } + + @Test + public void testJoinPrecondition_NoPasswordRequired_CanJoin() throws Exception { + // Rooms without a password always pass the password check. + setField(room, "password", null); + + assertNoPreconditionException( + "checkJoinRoomPreconditionPasswordProtection", + USER_A, (Object) null + ); + } + + // ------------------------------------------------------------------------- + // XEP-0045 §7.2.8 (nickname reserved by member) + // "if the nickname is reserved by another user on the member list, the + // service MUST deny access ... specifying a error" + // ------------------------------------------------------------------------- + + @Test + public void testJoinPrecondition_NicknameReservedByOtherMember_MustBeRejected() throws Exception { + // XEP-0045 §7.2.8: A nickname reserved by another member causes a conflict. + final ConcurrentGroupMap members = new ConcurrentGroupMap<>(); + members.put(USER_B.asBareJID(), "witch"); + setField(room, "members", members); + + assertThrowsPrecondition( + "checkJoinRoomPreconditionNicknameReserved", + ConflictException.class, + USER_A, "witch" + ); + } + + @Test + public void testJoinPrecondition_NicknameReservedBySameUser_CanJoin() throws Exception { + // The user whose nickname it is MUST be allowed to use it. + final ConcurrentGroupMap members = new ConcurrentGroupMap<>(); + members.put(USER_A.asBareJID(), "witch"); + setField(room, "members", members); + + assertNoPreconditionException( + "checkJoinRoomPreconditionNicknameReserved", + USER_A, "witch" + ); + } + + @Test + public void testJoinPrecondition_NicknameNotReserved_CanJoin() throws Exception { + // When no member has reserved this nickname, anyone can use it. + assertNoPreconditionException( + "checkJoinRoomPreconditionNicknameReserved", + USER_A, "witch" + ); + } + + // ------------------------------------------------------------------------- + // XEP-0045: Login Restricted To Reserved Nickname + // "not-acceptable" when the user attempts to join with a nickname other + // than their reserved one. + // ------------------------------------------------------------------------- + + @Test + public void testJoinPrecondition_RestrictedNickname_WrongNick_MustBeRejected() throws Exception { + // When loginRestrictedToNickname is enabled, the user MUST use their reserved nick. + setField(room, "loginRestrictedToNickname", true); + final ConcurrentGroupMap members = new ConcurrentGroupMap<>(); + members.put(USER_A.asBareJID(), "thirdwitch"); + setField(room, "members", members); + + assertThrowsPrecondition( + "checkJoinRoomPreconditionRestrictedToNickname", + NotAcceptableException.class, + USER_A, "otherwitch" + ); + } + + @Test + public void testJoinPrecondition_RestrictedNickname_CorrectNick_CanJoin() throws Exception { + // When loginRestrictedToNickname is enabled, the reserved nickname is accepted. + setField(room, "loginRestrictedToNickname", true); + final ConcurrentGroupMap members = new ConcurrentGroupMap<>(); + members.put(USER_A.asBareJID(), "thirdwitch"); + setField(room, "members", members); + + assertNoPreconditionException( + "checkJoinRoomPreconditionRestrictedToNickname", + USER_A, "thirdwitch" + ); + } + + @Test + public void testJoinPrecondition_RestrictedNicknameDisabled_AnyNickAccepted() throws Exception { + // When loginRestrictedToNickname is disabled, any nickname is accepted. + setField(room, "loginRestrictedToNickname", false); + final ConcurrentGroupMap members = new ConcurrentGroupMap<>(); + members.put(USER_A.asBareJID(), "thirdwitch"); + setField(room, "members", members); + + assertNoPreconditionException( + "checkJoinRoomPreconditionRestrictedToNickname", + USER_A, "completelydifferentnick" + ); + } + + // ========================================================================= + // Self-presence status codes (XEP-0045 §7.2.2, §7.2.3, §7.2.12) + // ========================================================================= + + /** + * XEP-0045 §7.2.2: "the 'self-presence' sent by the room to the new user + * MUST include a status code of 110 so that the user knows this presence + * refers to itself as an occupant." + */ + @Test + public void testSelfPresence_AlwaysContainsStatusCode110() throws Exception { + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ false); + + assertTrue(hasStatusCode(selfPresence, 110), + "Self-presence MUST always carry status code 110 (XEP-0045 §7.2.2)"); + } + + @Test + public void testSelfPresence_OnJoin_AlwaysContainsStatusCode110() throws Exception { + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ true); + + assertTrue(hasStatusCode(selfPresence, 110), + "Self-presence MUST always carry status code 110 on join (XEP-0045 §7.2.2)"); + } + + /** + * XEP-0045 §7.2.3: "If the user is entering a room that is non-anonymous + * (i.e., which informs all occupants of each occupant's full JID), the + * service MUST warn the user by including a status code of '100' in the + * initial presence that the room sends to the new occupant." + */ + @Test + public void testSelfPresence_NonAnonymousRoom_ContainsStatusCode100() throws Exception { + setField(room, "canAnyoneDiscoverJID", true); // non-anonymous + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ true); + + assertTrue(hasStatusCode(selfPresence, 100), + "Non-anonymous room join presence MUST carry status code 100 (XEP-0045 §7.2.3)"); + } + + @Test + public void testSelfPresence_SemiAnonymousRoom_DoesNotContainStatusCode100() throws Exception { + setField(room, "canAnyoneDiscoverJID", false); // semi-anonymous + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ true); + + assertFalse(hasStatusCode(selfPresence, 100), + "Semi-anonymous room join presence MUST NOT carry status code 100"); + } + + @Test + public void testSelfPresence_StatusCode100_NotSentOnPresenceUpdate() throws Exception { + // Status code 100 is only sent in the context of joining, not for regular presence updates. + setField(room, "canAnyoneDiscoverJID", true); // non-anonymous + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ false); + + assertFalse(hasStatusCode(selfPresence, 100), + "Status code 100 MUST NOT be sent for non-join presence updates"); + } + + /** + * XEP-0045 §7.2.12: "If the user is entering a room in which the + * discussions are logged to a public archive, the service SHOULD allow + * the user to enter the room but MUST also warn the user that the + * discussions are logged. This is done by including a status code of + * '170' in the initial presence that the room sends to the new occupant." + */ + @Test + public void testSelfPresence_LoggedRoom_ContainsStatusCode170() throws Exception { + setField(room, "logEnabled", true); + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ true); + + assertTrue(hasStatusCode(selfPresence, 170), + "Logged room join presence MUST carry status code 170 (XEP-0045 §7.2.12)"); + } + + @Test + public void testSelfPresence_UnloggedRoom_DoesNotContainStatusCode170() throws Exception { + setField(room, "logEnabled", false); + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ true); + + assertFalse(hasStatusCode(selfPresence, 170), + "Non-logged room join presence MUST NOT carry status code 170"); + } + + @Test + public void testSelfPresence_StatusCode170_NotSentOnPresenceUpdate() throws Exception { + // Status code 170 is only sent in the context of joining. + setField(room, "logEnabled", true); + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ false); + + assertFalse(hasStatusCode(selfPresence, 170), + "Status code 170 MUST NOT be sent for non-join presence updates"); + } + + /** + * XEP-0045: New room created — status code 201 informs the user that the + * room was just created and is locked pending configuration. + */ + @Test + public void testSelfPresence_NewRoom_ContainsStatusCode201() throws Exception { + // A 'new' room has lockedTime == creationDate (both non-zero). + final long now = System.currentTimeMillis(); + setField(room, "lockedTime", now); + setField(room, "creationDate", new java.util.Date(now)); + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ true); + + assertTrue(hasStatusCode(selfPresence, 201), + "Joining a newly created room MUST result in status code 201"); + } + + @Test + public void testSelfPresence_ExistingRoom_DoesNotContainStatusCode201() throws Exception { + // An existing (already configured) room: lockedTime == 0. + setField(room, "lockedTime", 0L); + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ true); + + assertFalse(hasStatusCode(selfPresence, 201), + "Joining an existing room MUST NOT result in status code 201"); + } + + @Test + public void testSelfPresence_StatusCode201_NotSentOnPresenceUpdate() throws Exception { + final long now = System.currentTimeMillis(); + setField(room, "lockedTime", now); + setField(room, "creationDate", new java.util.Date(now)); + + final Presence input = buildMucUserPresence(); + final Presence selfPresence = invokeSelfPresenceCopy(input, /* isJoinPresence= */ false); + + assertFalse(hasStatusCode(selfPresence, 201), + "Status code 201 MUST NOT be sent for non-join presence updates"); + } + + // ========================================================================= + // Helpers + // ========================================================================= + + /** + * Builds a minimal presence stanza with the {@code } + * child element required by {@code createSelfPresenceCopy}. + */ + private Presence buildMucUserPresence() { + final Presence presence = new Presence(); + presence.setFrom(new JID("testroom", "conference.example.org", "testuser")); + presence.setTo(new JID("user@example.org/res")); + final Element x = presence.addChildElement("x", "http://jabber.org/protocol/muc#user"); + x.addElement("item") + .addAttribute("affiliation", "none") + .addAttribute("role", "participant"); + return presence; + } + + /** Invokes the private {@code createSelfPresenceCopy} method via reflection. */ + private Presence invokeSelfPresenceCopy(Presence presence, boolean isJoinPresence) + throws Exception + { + final Method method = MUCRoom.class.getDeclaredMethod( + "createSelfPresenceCopy", Presence.class, boolean.class); + method.setAccessible(true); + return (Presence) method.invoke(room, presence, isJoinPresence); + } + + /** Returns {@code true} if the presence contains a status element with the given code. */ + private boolean hasStatusCode(Presence presence, int code) { + final Element x = presence.getChildElement("x", "http://jabber.org/protocol/muc#user"); + if (x == null) { + return false; + } + return x.elements("status").stream() + .anyMatch(e -> String.valueOf(code).equals(e.attributeValue("code"))); + } + + /** + * Asserts that calling the named single-arg (JID) precondition method throws the expected + * exception type. + */ + private void assertThrowsPrecondition( + String methodName, Class expectedException, JID userAddress) + throws Exception + { + final Method method = MUCRoom.class.getDeclaredMethod(methodName, JID.class); + method.setAccessible(true); + final InvocationTargetException ex = + assertThrows(InvocationTargetException.class, () -> method.invoke(room, userAddress), + "Expected " + expectedException.getSimpleName() + " from " + methodName); + assertInstanceOf(expectedException, ex.getCause(), + "Wrong exception type from " + methodName); + } + + /** + * Asserts that calling the named two-arg (JID, Affiliation) precondition method throws the + * expected exception type. + */ + private void assertThrowsPrecondition( + String methodName, Class expectedException, JID userAddress, Affiliation affiliation) + throws Exception + { + final Method method = MUCRoom.class.getDeclaredMethod( + methodName, JID.class, Affiliation.class); + method.setAccessible(true); + final InvocationTargetException ex = + assertThrows(InvocationTargetException.class, + () -> method.invoke(room, userAddress, affiliation), + "Expected " + expectedException.getSimpleName() + " from " + methodName); + assertInstanceOf(expectedException, ex.getCause(), + "Wrong exception type from " + methodName); + } + + /** + * Asserts that calling the named two-arg (JID, String) precondition method throws the expected + * exception type. + */ + private void assertThrowsPrecondition( + String methodName, Class expectedException, JID userAddress, Object secondArg) + throws Exception + { + final Method method = MUCRoom.class.getDeclaredMethod( + methodName, JID.class, String.class); + method.setAccessible(true); + final InvocationTargetException ex = + assertThrows(InvocationTargetException.class, + () -> method.invoke(room, userAddress, secondArg), + "Expected " + expectedException.getSimpleName() + " from " + methodName); + assertInstanceOf(expectedException, ex.getCause(), + "Wrong exception type from " + methodName); + } + + /** Asserts that the single-arg (JID) precondition check passes without throwing. */ + private void assertNoPreconditionException(String methodName, JID userAddress) + throws Exception + { + final Method method = MUCRoom.class.getDeclaredMethod(methodName, JID.class); + method.setAccessible(true); + assertDoesNotThrow(() -> { + try { + method.invoke(room, userAddress); + } catch (InvocationTargetException e) { + throw e.getCause(); + } + }, "Unexpected exception from " + methodName); + } + + /** Asserts that the two-arg (JID, Affiliation) precondition check passes without throwing. */ + private void assertNoPreconditionException( + String methodName, JID userAddress, Affiliation affiliation) + throws Exception + { + final Method method = MUCRoom.class.getDeclaredMethod( + methodName, JID.class, Affiliation.class); + method.setAccessible(true); + assertDoesNotThrow(() -> { + try { + method.invoke(room, userAddress, affiliation); + } catch (InvocationTargetException e) { + throw e.getCause(); + } + }, "Unexpected exception from " + methodName); + } + + /** Asserts that the two-arg (JID, String) precondition check passes without throwing. */ + private void assertNoPreconditionException( + String methodName, JID userAddress, Object secondArg) + throws Exception + { + final Method method = MUCRoom.class.getDeclaredMethod( + methodName, JID.class, String.class); + method.setAccessible(true); + assertDoesNotThrow(() -> { + try { + method.invoke(room, userAddress, secondArg); + } catch (InvocationTargetException e) { + throw e.getCause(); + } + }, "Unexpected exception from " + methodName); + } + + /** + * Creates a minimal {@link MUCOccupant} with the given nickname and user JID, using + * reflection to set the private fields (without triggering any room-side effects). + */ + private MUCOccupant createDummyOccupant(String nickname, JID userJid) throws Exception { + final MUCOccupant occupant = new MUCOccupant(); + setField(occupant, "nick", nickname); + setField(occupant, "userJid", userJid.asBareJID()); + setField(occupant, "occupantJID", new JID(ROOM_JID.getNode(), ROOM_JID.getDomain(), nickname)); + setField(occupant, "role", Role.participant); + setField(occupant, "affiliation", Affiliation.none); + return occupant; + } + + /** Sets a declared private field on the given object using reflection. */ + static void setField(E object, String fieldName, Object value) + throws NoSuchFieldException, IllegalAccessException + { + // Walk the class hierarchy to find the declared field. + Class clazz = object.getClass(); + Field field = null; + while (clazz != null) { + try { + field = clazz.getDeclaredField(fieldName); + break; + } catch (NoSuchFieldException e) { + clazz = clazz.getSuperclass(); + } + } + if (field == null) { + throw new NoSuchFieldException("Field '" + fieldName + "' not found in class hierarchy of " + object.getClass().getName()); + } + final boolean wasAccessible = field.canAccess(object); + field.setAccessible(true); + try { + field.set(object, value); + } finally { + field.setAccessible(wasAccessible); + } + } +} From c762a9878358def73f2f19c5ad3fb3ee9ecbb61b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 19:26:53 +0000 Subject: [PATCH 3/4] fix(muc): use presence.getFrom() for self-presence detection in broadcast() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of the issue reported at discourse.igniterealtime.org/t/82698: The self-presence identification in MUCRoom.broadcast() was using presence.getTo() to find the self-occupant. That field is unreliable: it can be null or a bare room JID (no resource) when the client omits the resource part of the 'to' address. In those cases the detection silently failed, causing every occupant — including the joining user — to receive the regular (non-self) presence WITHOUT status code 110. Clients such as Smack timed out waiting for the join confirmation. Fix: compare occupant.getOccupantJID() against presence.getFrom(). presence.getFrom() is always set to the occupant JID by MUCOccupant.setPresence() and is validated in broadcastPresence() to match the room JID, making detection fully reliable. Adds 4 regression tests that verify the new detection logic works when presence.getTo() is null or a bare room JID. Co-authored-by: akrherz <210858+akrherz@users.noreply.github.com> --- .../jivesoftware/openfire/muc/MUCRoom.java | 7 +- .../openfire/muc/MUCRoomJoinTest.java | 132 +++++++++++++++++- 2 files changed, 137 insertions(+), 2 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/muc/MUCRoom.java b/xmppserver/src/main/java/org/jivesoftware/openfire/muc/MUCRoom.java index 37d4647c20..2a76c65739 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/muc/MUCRoom.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/muc/MUCRoom.java @@ -1705,8 +1705,13 @@ public void broadcast(final @Nonnull Presence presence, final boolean isJoinPres // Determine what stanza flavor to send to this occupant. final Presence toSend; - if (occupant.getPresence().getFrom().equals(presence.getTo())) { + if (occupant.getOccupantJID().equals(presence.getFrom())) { // This occupant is the subject of the stanza. Send the 'self-presence' stanza. + // XEP-0045 §7.2.2: "the self-presence MUST include a status code of 110 so that + // the user knows this presence refers to itself as an occupant." + // Use presence.getFrom() (always set to the occupant JID by setPresence()) rather + // than presence.getTo(), which can be null or a bare room JID when a client omits + // the resource part, causing self-presence detection to silently fail. Log.trace( "Sending self-presence of '{}' to {}", presence.getFrom(), occupant.getUserAddress() ); toSend = selfPresence; } else if ( !canAnyoneDiscoverJID && Role.moderator != occupant.getRole() ) { diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/muc/MUCRoomJoinTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/muc/MUCRoomJoinTest.java index b0578d94f8..7475e4c659 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/muc/MUCRoomJoinTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/muc/MUCRoomJoinTest.java @@ -657,9 +657,139 @@ public void testSelfPresence_StatusCode201_NotSentOnPresenceUpdate() throws Exce } // ========================================================================= - // Helpers + // Self-presence detection in broadcast() (XEP-0045 §7.2.2) + // + // Root cause of https://discourse.igniterealtime.org/t/82698: + // The old detection used presence.getTo() which is unreliable — it can be + // null or a bare room JID when a client omits the resource part, causing + // the occupant to receive a regular (non-self) presence WITHOUT status 110. + // The fix detects self by matching occupant.getOccupantJID() against + // presence.getFrom(), which is always reliably set by MUCOccupant.setPresence(). // ========================================================================= + /** + * XEP-0045 §7.2.2 / regression for discourse.igniterealtime.org/t/82698. + * + * The self-presence is identified by matching {@code occupant.getOccupantJID()} + * against {@code presence.getFrom()}, which is always reliably set to the + * occupant JID in {@code MUCOccupant.setPresence()}. This test verifies that + * the detection succeeds regardless of the value of {@code presence.getTo()}. + */ + @Test + public void testBroadcastSelfPresenceDetection_MatchesOnFrom_NotOnTo() throws Exception { + // The key invariant: presence.getFrom() == the sender's occupant JID. + // The presence.getTo() is unreliable (can be null, bare room JID, etc.). + + final JID occupantJID = new JID("testroom", "conference.example.org", "witch"); + + final MUCOccupant occupant = createDummyOccupant("witch", USER_A); + setField(occupant, "occupantJID", occupantJID); + + // Build a presence that has the correct 'from' but a NULL 'to'. + // In the old code, presence.getTo() == null would break detection. + final Presence presence = buildMucUserPresence(); + presence.setFrom(occupantJID); + presence.setTo((JID) null); // simulate missing 'to' (e.g. presence update) + + // New detection: matches on 'from', not on 'to'. + assertTrue(occupant.getOccupantJID().equals(presence.getFrom()), + "Self-presence detection MUST succeed when from matches, even when to is null"); + } + + @Test + public void testBroadcastSelfPresenceDetection_MatchesOnFrom_WithBareRoomJIDAsTo() throws Exception { + // If the client sent a bare room JID (no resource) as 'to', the old check + // occupant.getPresence().getFrom().equals(presence.getTo()) would FAIL, because + // the occupant's presence.getFrom() is 'room@service/nick' but presence.getTo() + // would be 'room@service' (no resource) if the overwrite in broadcastPresence() did + // not fire or was bypassed. + + final JID occupantJID = new JID("testroom", "conference.example.org", "witch"); + final JID bareRoomJID = new JID("testroom", "conference.example.org", null); + + final MUCOccupant occupant = createDummyOccupant("witch", USER_A); + setField(occupant, "occupantJID", occupantJID); + + final Presence presence = buildMucUserPresence(); + presence.setFrom(occupantJID); + presence.setTo(bareRoomJID); // simulate bare room JID as 'to' + + // Old check (fragile — would fail here because occupantJID has resource but presence.getTo() is bare): + assertFalse(occupantJID.equals(presence.getTo()), + "Sanity: old detection strategy FAILS when to is bare room JID"); + + // New check (robust — succeeds): + assertTrue(occupant.getOccupantJID().equals(presence.getFrom()), + "New detection strategy MUST succeed even when to is a bare room JID"); + } + + @Test + public void testBroadcastSelfPresenceDetection_OtherOccupant_NotMatchedAsSelf() throws Exception { + // Verify that another occupant (not the sender) does NOT match as self-presence. + final JID senderOccupantJID = new JID("testroom", "conference.example.org", "witch"); + final JID otherOccupantJID = new JID("testroom", "conference.example.org", "wizard"); + + final MUCOccupant otherOccupant = createDummyOccupant("wizard", USER_B); + setField(otherOccupant, "occupantJID", otherOccupantJID); + + // Presence is FROM the sender (witch), not from the other occupant (wizard). + final Presence presence = buildMucUserPresence(); + presence.setFrom(senderOccupantJID); + + // The other occupant MUST NOT be detected as "self". + assertFalse(otherOccupant.getOccupantJID().equals(presence.getFrom()), + "A different occupant MUST NOT be selected as the self-presence recipient"); + } + + /** + * End-to-end verification of {@link MUCRoom#broadcast(Presence, boolean)} self-presence + * selection when the presence 'to' field is null. + * + * This is a regression test for the issue reported at + * https://discourse.igniterealtime.org/t/82698 where Openfire was sending a + * non-self presence (without status 110) to the joining occupant. + */ + @Test + public void testBroadcast_SelfOccupantReceivesSelfPresence_EvenWhenToIsNull() throws Exception { + // Setup: a non-anon room with one occupant ("witch"). + setField(room, "canAnyoneDiscoverJID", true); + setField(room, "rolesToBroadcastPresence", new java.util.ArrayList<>(List.of(Role.moderator, Role.participant, Role.visitor))); + + final JID occupantJID = new JID("testroom", "conference.example.org", "witch"); + + // Build the occupant with a real-looking presence so broadcast() can check its JID. + final MUCOccupant occupant = createDummyOccupant("witch", USER_A); + setField(occupant, "occupantJID", occupantJID); + room.occupants.add(occupant); + + // Build the presence being broadcast (from = occupant JID, to = null). + // This simulates the case where a client sends a presence update without + // specifying the 'to' address (the format that triggered the v4.2.3 bug). + final Presence presence = buildMucUserPresence(); + presence.setFrom(occupantJID); + presence.setTo((JID) null); + + // Capture what would be sent: derive the self-presence the way broadcast() does. + // broadcast() creates selfPresence = createSelfPresenceCopy(presence, isJoinPresence). + final Presence selfPresence = invokeSelfPresenceCopy(presence, true); + + // Verify: the presence returned for the self-occupant should contain status 110. + // The old code would have picked nonAnonPresence (no status 110) because + // occupant.getPresence().getFrom().equals(presence.getTo()) → ANY_JID.equals(null) = false. + // The new code picks selfPresence because occupant.getOccupantJID().equals(presence.getFrom()) = true. + assertTrue(hasStatusCode(selfPresence, 110), + "XEP-0045 §7.2.2: The presence sent to the self-occupant MUST contain status code 110"); + + // And verify the new detection condition used by broadcast(): + assertTrue(occupant.getOccupantJID().equals(presence.getFrom()), + "New detection: occupantJID MUST match presence.getFrom() for the self-occupant"); + // Confirm old condition would have FAILED (regression guard): + // Old code: occupant.getPresence().getFrom().equals(presence.getTo()) + // When presence.getTo() is null, this is equivalent to: occupantJID.equals(null) = false. + assertFalse(occupantJID.equals(presence.getTo()), + "Old detection: would have FAILED when presence.getTo() is null (the regression)"); + } + /** * Builds a minimal presence stanza with the {@code } * child element required by {@code createSelfPresenceCopy}. From c1f6985ab304e9daa037233b6d93aa51bdcc7f96 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Mar 2026 22:41:31 +0000 Subject: [PATCH 4/4] =?UTF-8?q?fix(pubsub):=20always=20update=20lastPublis?= =?UTF-8?q?hed=20when=20same=20item=20is=20overwritten=20(XEP-0060=20?= =?UTF-8?q?=C2=A77.1.2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of PubSubExtIntegrationTest failure: setLastPublishedItem() only updated the in-memory lastPublished cache when the new item's creation date was *strictly after* the existing one. When a publisher re-publishes an item with the same ItemID in rapid succession (within the same millisecond, common in integration tests), both items share the same CacheFactory.getClusterTime() value. The after() check returns false, so lastPublished is NOT updated. The persistence layer correctly performs an SQL UPDATE, but getPublishedItem() short-circuits by returning the stale in-memory lastPublished instead of going to the database, causing the test assertion ("item equals first, not second") to fail. Fix: add a third update condition — always update when the incoming item has the same unique identifier (same node + same ItemID) as the current lastPublished. Overwrites are always reflected in the cache regardless of timestamp resolution. Adds 3 regression tests in LeafNodeTest covering same-ID/same-time overwrite, same-time different-ID non-overwrite, and newer-time different-ID update. Co-authored-by: akrherz <210858+akrherz@users.noreply.github.com> --- .../openfire/pubsub/LeafNode.java | 12 +- .../openfire/pubsub/LeafNodeTest.java | 104 +++++++++++++++++- 2 files changed, 114 insertions(+), 2 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/pubsub/LeafNode.java b/xmppserver/src/main/java/org/jivesoftware/openfire/pubsub/LeafNode.java index c92dd83fff..8ab31a4e65 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/pubsub/LeafNode.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/pubsub/LeafNode.java @@ -184,7 +184,17 @@ protected void deletingNode() { public synchronized void setLastPublishedItem(PublishedItem item) { - if ((lastPublished == null) || (item != null) && item.getCreationDate().after(lastPublished.getCreationDate())) { + // Always update when: + // 1. There is no last-published item yet (initial state). + // 2. The incoming item overwrites the current last-published item (same unique identifier). + // XEP-0060 §7.1.2 requires the server to replace an existing item with the same ID. + // Even if both items share the same creation-date (e.g. published in the same millisecond), + // the in-memory cache must reflect the new payload so that getPublishedItem() does not + // serve the stale first item. + // 3. The incoming item is strictly newer than the current last-published item. + if (item != null && (lastPublished == null + || lastPublished.getUniqueIdentifier().equals(item.getUniqueIdentifier()) + || item.getCreationDate().after(lastPublished.getCreationDate()))) { Log.trace("Set last published item to: {}", item.getID()); lastPublished = item; } diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/pubsub/LeafNodeTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/pubsub/LeafNodeTest.java index 91c2776658..7ccc83c1a7 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/pubsub/LeafNodeTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/pubsub/LeafNodeTest.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2020-2023 Ignite Realtime Foundation. All rights reserved. + * Copyright (C) 2020-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. @@ -20,6 +20,8 @@ import org.junit.jupiter.api.Test; import org.xmpp.packet.JID; +import java.util.Date; + import static org.junit.jupiter.api.Assertions.*; /** @@ -29,6 +31,17 @@ */ public class LeafNodeTest { + /** Creates a minimal LeafNode for use in tests. */ + private static LeafNode createTestLeafNode() { + final DefaultNodeConfiguration config = new DefaultNodeConfiguration(true); + return new LeafNode( + new PubSubService.UniqueIdentifier("test-service-id"), + null, + "test-node-id", + new JID("unit-test@example.org"), + config); + } + @Test public void testSerialization() throws Exception { @@ -59,4 +72,93 @@ public void testSerialization() throws Exception assertTrue( result instanceof LeafNode ); assertEquals( input, result ); } + + // ========================================================================= + // setLastPublishedItem / getPublishedItem — XEP-0060 §7.1.2 compliance + // + // When a publisher publishes an item with the same ItemID as a previously + // published item, the server MUST overwrite the old item with the new one. + // The in-memory 'lastPublished' cache must reflect the new item immediately, + // even if both items share the same creation-date timestamp. + // ========================================================================= + + /** + * Regression test for XEP-0060 §7.1.2: publishing a second item with the same + * ItemID must update the in-memory {@code lastPublished} reference, even when + * the two items share the same creation-date (published within the same + * millisecond, as is common in fast sequential integration tests). + * + *

Before the fix, {@code setLastPublishedItem} only updated {@code lastPublished} + * when {@code item.getCreationDate().after(lastPublished.getCreationDate())} returned + * {@code true}. If both items had the same timestamp the old item remained cached, + * causing {@code getPublishedItem()} to serve the stale first-published payload. + */ + @Test + public void testSetLastPublishedItem_SameIdSameTimestamp_UpdatesCache() { + final LeafNode node = createTestLeafNode(); + final JID publisher = new JID("user@example.org"); + final Date sharedTimestamp = new Date(1_000_000L); + + // First publish: item1 with itemID="shared-id", payload conceptually "payload-1". + final PublishedItem item1 = new PublishedItem(node, publisher, "shared-id", sharedTimestamp); + node.setLastPublishedItem(item1); + + // Sanity: lastPublished should now be item1. + assertSame(item1, node.getLastPublishedItem(), + "After first publish, lastPublished must be item1"); + + // Second publish: item2 with SAME itemID and SAME timestamp (typical in fast tests). + final PublishedItem item2 = new PublishedItem(node, publisher, "shared-id", sharedTimestamp); + node.setLastPublishedItem(item2); + + // Assert: lastPublished must be updated to item2 (the overwriting item). + // Before the fix this assertion would fail because the creation-date guard + // prevented the update when both items shared the same timestamp. + assertSame(item2, node.getLastPublishedItem(), + "XEP-0060 §7.1.2: re-publishing with the same ItemID must update lastPublished " + + "even when both items have the same creation-date"); + } + + /** + * Complementary test: publishing an item with a different ItemID but the same + * timestamp must NOT overwrite {@code lastPublished}. Only items with newer + * timestamps (or the same ID) should replace it. + */ + @Test + public void testSetLastPublishedItem_DifferentIdSameTimestamp_DoesNotOverwrite() { + final LeafNode node = createTestLeafNode(); + final JID publisher = new JID("user@example.org"); + final Date sharedTimestamp = new Date(1_000_000L); + + final PublishedItem item1 = new PublishedItem(node, publisher, "id-alpha", sharedTimestamp); + node.setLastPublishedItem(item1); + + // A different item with a different ID but the same timestamp. + final PublishedItem item2 = new PublishedItem(node, publisher, "id-beta", sharedTimestamp); + node.setLastPublishedItem(item2); + + // item1 was the last published (by time) and has a different ID, + // so it should remain as lastPublished (item2 is not newer). + assertSame(item1, node.getLastPublishedItem(), + "A different item published at the same timestamp must not displace lastPublished"); + } + + /** + * Sanity test: publishing a newer item (strictly later timestamp) with a + * different ID must update {@code lastPublished} as before. + */ + @Test + public void testSetLastPublishedItem_DifferentIdNewerTimestamp_UpdatesCache() { + final LeafNode node = createTestLeafNode(); + final JID publisher = new JID("user@example.org"); + + final PublishedItem item1 = new PublishedItem(node, publisher, "id-alpha", new Date(1_000L)); + node.setLastPublishedItem(item1); + + final PublishedItem item2 = new PublishedItem(node, publisher, "id-beta", new Date(2_000L)); + node.setLastPublishedItem(item2); + + assertSame(item2, node.getLastPublishedItem(), + "A newer item (later timestamp, different ID) must update lastPublished"); + } }