I would like to report a bug in MUC OccupantLeftEvent.java that a remote node may remove a wrong entry when one client session of an occupant (who had multiple client sessions) left a room. I had a 2-node clustered Openfire and 2 clients with same username but different resources (focus@mydomain/f1 and focus@mydomain/f2.) The two clients were connecting to two XMPP nodes and they both joined the MUC room “jibribrewery” with same nickname “focus”. When the focus@mydomain/f1 disconnected, this occupant was removed correctly from the room in both XMPP nodes. When this user (focus@mydomain/f1) reconnected, rejoined the room and disconnected again, the remote XMPP removed “focus@mydomain/f2” from the room. In one XMPP node the remaining occupant was focus@mydomain/f2 (which is correct), but in the other XMPP node the remaining occupant was focus@mydomain/f1 (which is incorrect.) Although my use case may be unusual, it may happen to normal clients with Stream-Management enabled (a mobile client lost connection temporarily, but reconnected with different resource.)
The bug is in OccupantLeftEvent.getRole() that it used a deprecated getRoom().getOccupant(nickname) which always returns the first MUCRole from a list. Since this occupant had two client sessions and used the same nickname, the list had multiple MUCRole entries. Subsequently, the occupants in this room among the nodes would be out of sync and caused a lot of unexpected behavior. The fix is to pass along the full JID and use it to get the correct MUCRole from the list. Also, the field member “role” probably can be declared as transient.
Here is a suggested fix in xmppserver/src/main/java/org/jivesoftware/openfire/muc/cluster/OccupantLeftEvent.java
git diff OccupantLeftEvent.java
diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/muc/cluster/OccupantLeftEvent.java b/xmppserver/src/main/java/org/jivesoftware/openfire/muc/cluster/OccupantLeftEvent.java
index 219805c..8ac348f 100644
--- a/xmppserver/src/main/java/org/jivesoftware/openfire/muc/cluster/OccupantLeftEvent.java
+++ b/xmppserver/src/main/java/org/jivesoftware/openfire/muc/cluster/OccupantLeftEvent.java
@@ -20,20 +20,23 @@ import org.jivesoftware.openfire.muc.MUCRole;
import org.jivesoftware.openfire.muc.spi.LocalMUCRoom;
import org.jivesoftware.openfire.user.UserNotFoundException;
import org.jivesoftware.util.cache.ExternalizableUtil;
+import org.xmpp.packet.JID;
import java.io.IOException;
import java.io.ObjectInput;
import java.io.ObjectOutput;
+import java.util.List;
/**
* Task that removes a room occupant from the list of occupants in the room. The
- * occupant to remove is actualy a {@link org.jivesoftware.openfire.muc.spi.RemoteMUCRole}.
+ * occupant to remove is actually a {@link org.jivesoftware.openfire.muc.spi.RemoteMUCRole}.
*
* @author Gaston Dombiak
*/
public class OccupantLeftEvent extends MUCRoomTask<Void> {
private MUCRole role;
private String nickname;
+ private JID userAddress;
public OccupantLeftEvent() {
}
@@ -42,12 +45,21 @@ public class OccupantLeftEvent extends MUCRoomTask<Void> {
super(room);
this.role = role;
this.nickname = role.getNickname();
+ this.userAddress = role.getUserAddress();
}
public MUCRole getRole() {
if (role == null) {
try {
- role = getRoom().getOccupant(nickname);
+ // If there are multiple entries, get one with same full JID
+ List<MUCRole> roles = getRoom().getOccupantsByNickname(nickname);
+ for (MUCRole r : roles) {
+ if (userAddress.equals(r.getUserAddress())) {
+ role = r;
+ break;
+ }
+ }
+ // TODO: if no matching full JID, what to do?
} catch (UserNotFoundException e) {
// Ignore
}
@@ -75,11 +87,13 @@ public class OccupantLeftEvent extends MUCRoomTask<Void> {
public void writeExternal(ObjectOutput out) throws IOException {
super.writeExternal(out);
ExternalizableUtil.getInstance().writeSafeUTF(out, nickname);
+ ExternalizableUtil.getInstance().writeSerializable(out, userAddress);
}
@Override
public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
super.readExternal(in);
nickname = ExternalizableUtil.getInstance().readSafeUTF(in);
+ userAddress = (JID) ExternalizableUtil.getInstance().readSerializable(in);
}
}