OF-523: Apply defensive coding to the MUC implementation (mainly: use JIDs,...

OF-523: Apply defensive coding to the MUC implementation (mainly: use JIDs, not Strings - but includes some other fixes as well).

git-svn-id: http://svn.igniterealtime.org/svn/repos/openfire/trunk@12978 b35dd754-fafc-0310-a699-88a17e54d16e

src/java/org/jivesoftware/openfire/clearspace/ClearspaceMUCEventDelegate.java

@@ -108,8 +108,8 @@ public class ClearspaceMUCEventDelegate extends MUCEventDelegate {
 	public boolean joiningRoom(MUCRoom room, JID userjid) {
         // Always allow an owner to join the room (especially since they need to join to configure the
         // room on initial creation).
-        Collection<String> owners = room.getOwners();
-        if (owners != null && owners.contains(userjid.toBareJID())) {
+        Collection<JID> owners = room.getOwners();
+        if (owners != null && owners.contains(new JID(userjid.toBareJID()))) {
             return true;
         }
 
@@ -186,9 +186,11 @@ public class ClearspaceMUCEventDelegate extends MUCEventDelegate {
             Log.warn(GET_ROOM_CONFIG_WARNING + " Room: " + roomJid.toBareJID());
             return null;
         }
-        Iterator fields = xElement.elementIterator("field");
+        
+        @SuppressWarnings("unchecked")
+		Iterator<Element> fields = xElement.elementIterator("field");
         while (fields.hasNext()) {
-            Element field = (Element) fields.next();
+            Element field = fields.next();
             Attribute varAttribute = field.attribute("var");
             if (varAttribute != null) {
                 Element value = field.element("value");

src/java/org/jivesoftware/openfire/clearspace/ClearspaceMUCTranscriptManager.java

@@ -186,7 +186,7 @@ public class ClearspaceMUCTranscriptManager implements MUCEventListener {
                     MUCRoom room = mucService.getChatRoom(jid.getNode());
                     // Not count room owners as occupants
                     int totalOccupants = room.getOccupantsCount();
-                    for (String owner : room.getOwners()) {
+                    for (JID owner : room.getOwners()) {
                         try {
                             if (!room.getOccupantsByBareJID(owner).isEmpty()) {
                                 totalOccupants--;

src/java/org/jivesoftware/openfire/muc/HistoryRequest.java

@@ -149,9 +149,9 @@ public class HistoryRequest {
      */
     public void sendHistory(LocalMUCRole joinRole, MUCRoomHistory roomHistory) {
         if (!isConfigured()) {
-            Iterator history = roomHistory.getMessageHistory();
+            Iterator<Message> history = roomHistory.getMessageHistory();
             while (history.hasNext()) {
-                joinRole.send((Message) history.next());
+                joinRole.send(history.next());
             }
         }
         else {
@@ -164,14 +164,13 @@ public class HistoryRequest {
                 }
                 return;
             }
-            Message message;
             int accumulatedChars = 0;
             int accumulatedStanzas = 0;
             Element delayInformation;
             LinkedList<Message> historyToSend = new LinkedList<Message>();
-            ListIterator iterator = roomHistory.getReverseMessageHistory();
+            ListIterator<Message> iterator = roomHistory.getReverseMessageHistory();
             while (iterator.hasPrevious()) {
-                message = (Message)iterator.previous();
+                Message message = iterator.previous();
                 // Update number of characters to send
                 String text = message.getBody() == null ? message.getSubject() : message.getBody();
                 if (text == null) {

src/java/org/jivesoftware/openfire/muc/HistoryStrategy.java

@@ -205,7 +205,7 @@ public class HistoryStrategy {
                 // last room subject
                 // message because we want to preserve the room subject if
                 // possible.
-                Iterator historyIter = history.iterator();
+                Iterator<Message> historyIter = history.iterator();
                 while (historyIter.hasNext() && history.size() > strategyMaxNumber) {
                     if (historyIter.next() != roomSubject) {
                         historyIter.remove();

src/java/org/jivesoftware/openfire/muc/MUCEventDelegate.java

@@ -112,7 +112,13 @@ public abstract class MUCEventDelegate {
             room.setRegistrationEnabled("1".equals(roomConfig.get("x-muc#roomconfig_registration")));
             room.setPersistent("1".equals(roomConfig.get("muc#roomconfig_persistentroom")));
             
-            room.addFirstOwner(roomConfig.get("muc#roomconfig_roomowners"));
+            final String property = roomConfig.get("muc#roomconfig_roomowners");
+            if (property != null) {
+                String jids[] = property.split(",");
+                for (String jid : jids) {
+                	room.addFirstOwner(new JID(new JID(jid.trim().toLowerCase()).toBareJID()));
+                }
+            }
             
             try {
                 room.unlock(room.getRole());

src/java/org/jivesoftware/openfire/muc/MUCRoom.java

@@ -170,7 +170,7 @@ public interface MUCRoom extends Externalizable, Result {
      * @return The user's roles in the room
      * @throws UserNotFoundException If there is no user with the given nickname
      */
-    List<MUCRole> getOccupantsByBareJID(String jid) throws UserNotFoundException;
+    List<MUCRole> getOccupantsByBareJID(JID jid) throws UserNotFoundException;
 
     /**
      * Returns the role of a given user in the room by his full JID or <tt>null</tt>
@@ -209,7 +209,7 @@ public interface MUCRoom extends Externalizable, Result {
      * @param bareJID The bare jid of the user of which you'd like to obtain his reserved nickname.
      * @return the reserved room nickname for the bare JID or null if none.
      */
-    String getReservedNickname(String bareJID);
+    String getReservedNickname(JID jid);
 
     /**
      * Returns the affiliation state of the user in the room. Possible affiliations are 
@@ -220,7 +220,7 @@ public interface MUCRoom extends Externalizable, Result {
      * @param bareJID The bare jid of the user of which you'd like to obtain his affiliation.
      * @return the affiliation state of the user in the room.
      */
-    MUCRole.Affiliation getAffiliation(String bareJID);
+    MUCRole.Affiliation getAffiliation(JID bareJID);
 
     /**
      * Joins the room using the given nickname.
@@ -263,7 +263,7 @@ public interface MUCRoom extends Externalizable, Result {
      * @param alternateJID the alternate JID. Commonly used to provide a replacement room.
      * @param reason the reason why the room was destroyed.
      */
-    void destroyRoom(String alternateJID, String reason);
+    void destroyRoom(JID alternateJID, String reason);
 
     /**
      * Create a new presence in this room for the given role.
@@ -296,20 +296,7 @@ public interface MUCRoom extends Externalizable, Result {
      * 
      * @param bareJID The bare JID of the user to add as owner.
      */
-    public void addFirstOwner(String bareJID);
-
-    /**
-     * Adds a new user to the list of owners.
-     * 
-     * @param bareJID The bare JID of the user to add as owner.
-     * @param senderRole the role of the user that is trying to modify the owners list.
-     * @return the list of updated presences of all the client resources that the client used to
-     *         join the room.
-     * @throws ForbiddenException If the user is not allowed to modify the owner list.
-     * @deprecated Replaced by {@link #addOwner(JID, MUCRole)}
-     */
-    @Deprecated
-    public List<Presence> addOwner(String bareJID, MUCRole senderRole) throws ForbiddenException;
+    public void addFirstOwner(JID bareJID);
 
     /**
      * Adds a new user to the list of owners.
@@ -331,7 +318,7 @@ public interface MUCRoom extends Externalizable, Result {
      *         join the room.
      * @throws ForbiddenException If the user is not allowed to modify the owner list.
      */
-    public List<Presence> addOwners(List<String> newOwners, MUCRole senderRole)
+    public List<Presence> addOwners(List<JID> newOwners, MUCRole senderRole)
             throws ForbiddenException;
 
     /**
@@ -344,24 +331,9 @@ public interface MUCRoom extends Externalizable, Result {
      * @throws ForbiddenException If the user is not allowed to modify the admin list.
      * @throws ConflictException If the room was going to lose all its owners.
      */
-    public List<Presence> addAdmins(List<String> newAdmins, MUCRole senderRole)
+    public List<Presence> addAdmins(List<JID> newAdmins, MUCRole senderRole)
             throws ForbiddenException, ConflictException;
 
-    /**
-     * Adds a new user to the list of admins.
-     * 
-     * @param bareJID The bare JID of the user to add as admin.
-     * @param senderRole The role of the user that is trying to modify the admins list.
-     * @return the list of updated presences of all the client resources that the client used to
-     *         join the room.
-     * @throws ForbiddenException If the user is not allowed to modify the admin list.
-     * @throws ConflictException If the room was going to lose all its owners.
-     * @deprecated Replaced by {@link #addAdmin(JID, MUCRole)}
-     */
-    @Deprecated
-    public List<Presence> addAdmin(String bareJID, MUCRole senderRole) throws ForbiddenException,
-            ConflictException;
-    
     /**
      * Adds a new user to the list of admins.
      * 
@@ -375,23 +347,6 @@ public interface MUCRoom extends Externalizable, Result {
     public List<Presence> addAdmin(JID jid, MUCRole senderRole) throws ForbiddenException,
             ConflictException;
 
-    /**
-     * Adds a new user to the list of members.
-     * 
-     * @param bareJID The bare JID of the user to add as a member.
-     * @param nickname The reserved nickname of the member for the room or null if none.
-     * @param senderRole the role of the user that is trying to modify the members list.
-     * @return the list of updated presences of all the client resources that the client used to
-     *         join the room.
-     * @throws ForbiddenException If the user is not allowed to modify the members list.
-     * @throws ConflictException If the desired room nickname is already reserved for the room or if
-     *             the room was going to lose all its owners.
-     * @deprecated Replaced by {@link #addMember(JID, String, MUCRole)}
-     */
-    @Deprecated
-    public List<Presence> addMember(String bareJID, String nickname, MUCRole senderRole)
-            throws ForbiddenException, ConflictException;
-
     /**
      * Adds a new user to the list of members.
      * 
@@ -407,23 +362,6 @@ public interface MUCRoom extends Externalizable, Result {
     public List<Presence> addMember(JID jid, String nickname, MUCRole senderRole)
             throws ForbiddenException, ConflictException;
 
-    /**
-     * Adds a new user to the list of outcast users.
-     * 
-     * @param bareJID The bare JID of the user to add as an outcast.
-     * @param reason The reason why the user was banned.
-     * @param senderRole The role of the user that initiated the ban.
-     * @return the list of updated presences of all the client resources that the client used to
-     *         join the room.
-     * @throws NotAllowedException Thrown if trying to ban an owner or an administrator.
-     * @throws ForbiddenException If the user is not allowed to modify the outcast list.
-     * @throws ConflictException If the room was going to lose all its owners.
-     * @deprecated Replaced by {@link #addOutcast(JID, String, MUCRole)}
-     */
-    @Deprecated
-    public List<Presence> addOutcast(String bareJID, String reason, MUCRole senderRole)
-            throws NotAllowedException, ForbiddenException, ConflictException;
-
     /**
      * Adds a new user to the list of outcast users.
      * 
@@ -439,21 +377,6 @@ public interface MUCRoom extends Externalizable, Result {
     public List<Presence> addOutcast(JID jid, String reason, MUCRole senderRole)
             throws NotAllowedException, ForbiddenException, ConflictException;
 
-    /**
-     * Removes the user from all the other affiliation list thus giving the user a NONE affiliation.
-     * 
-     * @param bareJID The bare JID of the user to keep with a NONE affiliation.
-     * @param senderRole The role of the user that set the affiliation to none.
-     * @return the list of updated presences of all the client resources that the client used to
-     *         join the room or null if none was updated.
-     * @throws ForbiddenException If the user is not allowed to modify the none list.
-     * @throws ConflictException If the room was going to lose all its owners.
-     * @deprecated Replaced by {@link #addNone(JID, MUCRole)}
-     */
-    @Deprecated
-	public List<Presence> addNone(String bareJID, MUCRole senderRole) throws ForbiddenException,
-            ConflictException;
-
     /**
      * Removes the user from all the other affiliation list thus giving the user a NONE affiliation.
      * 
@@ -627,7 +550,7 @@ public interface MUCRoom extends Externalizable, Result {
      *
      * @return a collection with the current list of owners.
      */
-    public Collection<String> getOwners();
+    public Collection<JID> getOwners();
 
     /**
      * Returns a collection with the current list of admins. The collection contains the bareJID of
@@ -635,7 +558,7 @@ public interface MUCRoom extends Externalizable, Result {
      *
      * @return a collection with the current list of admins.
      */
-    public Collection<String> getAdmins();
+    public Collection<JID> getAdmins();
 
     /**
      * Returns a collection with the current list of room members. The collection contains the
@@ -645,7 +568,7 @@ public interface MUCRoom extends Externalizable, Result {
      *
      * @return a collection with the current list of members.
      */
-    public Collection<String> getMembers();
+    public Collection<JID> getMembers();
 
     /**
      * Returns a collection with the current list of outcast users. An outcast user is not allowed
@@ -654,7 +577,7 @@ public interface MUCRoom extends Externalizable, Result {
      *
      * @return a collection with the current list of outcast users.
      */
-    public Collection<String> getOutcasts();
+    public Collection<JID> getOutcasts();
 
     /**
      * Returns a collection with the current list of room moderators. The collection contains the

src/java/org/jivesoftware/openfire/muc/MUCRoomHistory.java

@@ -84,12 +84,10 @@ public final class MUCRoomHistory {
         if (isNonAnonymousRoom != room.canAnyoneDiscoverJID()) {
             isNonAnonymousRoom = room.canAnyoneDiscoverJID();
             // Update the "from" attribute of the delay information in the history
-            Message message;
-            Element delayElement;
             // TODO Make this update in a separate thread
-            for (Iterator it = getMessageHistory(); it.hasNext();) {
-                message = (Message) it.next();
-                delayElement = message.getChildElement("x", "jabber:x:delay");
+            for (Iterator<Message> it = getMessageHistory(); it.hasNext();) {
+                Message message = it.next();
+                Element delayElement = message.getChildElement("x", "jabber:x:delay");
                 if (room.canAnyoneDiscoverJID()) {
                     // Set the Full JID as the "from" attribute
                     try {
@@ -133,7 +131,7 @@ public final class MUCRoomHistory {
         historyStrategy.addMessage(packetToAdd);
     }
 
-    public Iterator getMessageHistory() {
+    public Iterator<Message> getMessageHistory() {
         return historyStrategy.getMessageHistory();
     }
 
@@ -144,7 +142,7 @@ public final class MUCRoomHistory {
      * 
      * @return A list iterator of Message objects positioned at the end of the list.
      */
-    public ListIterator getReverseMessageHistory() {
+    public ListIterator<Message> getReverseMessageHistory() {
         return historyStrategy.getReverseMessageHistory();
     }
 

src/java/org/jivesoftware/openfire/muc/MUCUser.java

@@ -22,6 +22,7 @@ package org.jivesoftware.openfire.muc;
 
 import org.jivesoftware.openfire.ChannelHandler;
 import org.xmpp.packet.JID;
+import org.xmpp.packet.Packet;
 
 /**
  * The chat user is a separate user abstraction for interacting with
@@ -36,7 +37,7 @@ import org.xmpp.packet.JID;
  *
  * @author Gaston Dombiak
  */
-public interface MUCUser extends ChannelHandler {
+public interface MUCUser extends ChannelHandler<Packet> {
 
     /**
       * Obtain the address of the user. The address is used by services like the core

src/java/org/jivesoftware/openfire/muc/MultiUserChatService.java

@@ -62,7 +62,7 @@ public interface MultiUserChatService extends Component {
      * 
      * @return a list of bare JIDs.
      */
-    Collection<String> getSysadmins();
+    Collection<JID> getSysadmins();
 
     /**
      * Adds a new system administrator of the MUC service. A sysadmin has the same permissions as 
@@ -70,14 +70,14 @@ public interface MultiUserChatService extends Component {
      * 
      * @param userJID the bare JID of the new user to add as a system administrator.
      */
-    void addSysadmin(String userJID);
+    void addSysadmin(JID userJID);
 
     /**
      * Removes a system administrator of the MUC service.
      * 
      * @param userJID the bare JID of the user to remove from the list.
      */
-    void removeSysadmin(String userJID);
+    void removeSysadmin(JID userJID);
 
     /**
      * Returns false if anyone can create rooms or true if only the returned JIDs in
@@ -101,21 +101,21 @@ public interface MultiUserChatService extends Component {
      * 
      * @return a list of bare JIDs.
      */
-    Collection<String> getUsersAllowedToCreate();
+    Collection<JID> getUsersAllowedToCreate();
 
     /**
      * Adds a new user to the list of JIDs that are allowed to create MUC rooms.
      * 
      * @param userJID the bare JID of the new user to add to list.
      */
-    void addUserAllowedToCreate(String userJID);
+    void addUserAllowedToCreate(JID userJID);
 
     /**
      * Removes a user from list of JIDs that are allowed to create MUC rooms.
      * 
      * @param userJID the bare JID of the user to remove from the list.
      */
-    void removeUserAllowedToCreate(String userJID);
+    void removeUserAllowedToCreate(JID userJID);
 
     /**
      * Sets the time to elapse between clearing of idle chat users. A <code>TimerTask</code> will be

src/java/org/jivesoftware/openfire/muc/cluster/AddMember.java

@@ -20,33 +20,40 @@
 
 package org.jivesoftware.openfire.muc.cluster;
 
-import org.jivesoftware.openfire.muc.spi.LocalMUCRoom;
-import org.jivesoftware.util.cache.ExternalizableUtil;
-
 import java.io.IOException;
 import java.io.ObjectInput;
 import java.io.ObjectOutput;
 
+import org.jivesoftware.openfire.muc.spi.LocalMUCRoom;
+import org.jivesoftware.util.cache.ExternalizableUtil;
+import org.xmpp.packet.JID;
+
 /**
  * Task that adds a new member to the room in the other cluster nodes.
  *
  * @author Gaston Dombiak
  */
 public class AddMember extends MUCRoomTask {
-    private String bareJID;
+    private JID bareJID;
     private String nickname;
 
     public AddMember() {
         super();
     }
 
+    public AddMember(LocalMUCRoom room, JID bareJID, String nickname) {
+        super(room);
+        this.bareJID = new JID(bareJID.toBareJID());
+        this.nickname = nickname;
+    }
+    
     public AddMember(LocalMUCRoom room, String bareJID, String nickname) {
         super(room);
-        this.bareJID = bareJID;
+        this.bareJID = new JID(new JID(bareJID).toBareJID());
         this.nickname = nickname;
     }
 
-    public String getBareJID() {
+    public JID getBareJID() {
         return bareJID;
     }
 
@@ -70,14 +77,14 @@ public class AddMember extends MUCRoomTask {
     @Override
 	public void writeExternal(ObjectOutput out) throws IOException {
         super.writeExternal(out);
-        ExternalizableUtil.getInstance().writeSafeUTF(out, bareJID);
+        ExternalizableUtil.getInstance().writeSafeUTF(out, bareJID.toFullJID());
         ExternalizableUtil.getInstance().writeSafeUTF(out, nickname);
     }
 
     @Override
 	public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
         super.readExternal(in);
-        bareJID = ExternalizableUtil.getInstance().readSafeUTF(in);
+        bareJID = new JID(ExternalizableUtil.getInstance().readSafeUTF(in));
         nickname = ExternalizableUtil.getInstance().readSafeUTF(in);
     }
 }

src/java/org/jivesoftware/openfire/muc/cluster/DestroyRoomRequest.java

@@ -20,13 +20,14 @@
 
 package org.jivesoftware.openfire.muc.cluster;
 
-import org.jivesoftware.openfire.muc.spi.LocalMUCRoom;
-import org.jivesoftware.util.cache.ExternalizableUtil;
-
 import java.io.IOException;
 import java.io.ObjectInput;
 import java.io.ObjectOutput;
 
+import org.jivesoftware.openfire.muc.spi.LocalMUCRoom;
+import org.jivesoftware.util.cache.ExternalizableUtil;
+import org.xmpp.packet.JID;
+
 /**
  * Task that destroys the local room in the cluster node. Local room occupants
  * hosted in the cluster node will get the notification of the room being
@@ -35,18 +36,24 @@ import java.io.ObjectOutput;
  * @author Gaston Dombiak
  */
 public class DestroyRoomRequest extends MUCRoomTask {
-    private String alternateJID;
+    private JID alternateJID;
     private String reason;
 
     public DestroyRoomRequest() {
     }
 
-    public DestroyRoomRequest(LocalMUCRoom room, String alternateJID, String reason) {
+    public DestroyRoomRequest(LocalMUCRoom room, JID alternateJID, String reason) {
         super(room);
         this.alternateJID = alternateJID;
         this.reason = reason;
     }
 
+    public DestroyRoomRequest(LocalMUCRoom room, String alternateJID, String reason) {
+        super(room);
+        this.alternateJID = new JID(alternateJID);
+        this.reason = reason;
+    }
+
     public Object getResult() {
         return null;
     }
@@ -60,7 +67,7 @@ public class DestroyRoomRequest extends MUCRoomTask {
         });
     }
 
-    public String getAlternateJID() {
+    public JID getAlternateJID() {
         return alternateJID;
     }
 
@@ -73,7 +80,7 @@ public class DestroyRoomRequest extends MUCRoomTask {
         super.writeExternal(out);
         ExternalizableUtil.getInstance().writeBoolean(out, alternateJID != null);
         if (alternateJID != null) {
-            ExternalizableUtil.getInstance().writeSafeUTF(out, alternateJID);
+            ExternalizableUtil.getInstance().writeSafeUTF(out, alternateJID.toFullJID());
         }
         ExternalizableUtil.getInstance().writeBoolean(out, reason != null);
         if (reason != null) {
@@ -85,7 +92,7 @@ public class DestroyRoomRequest extends MUCRoomTask {
 	public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
         super.readExternal(in);
         if (ExternalizableUtil.getInstance().readBoolean(in)) {
-            alternateJID = ExternalizableUtil.getInstance().readSafeUTF(in);
+            alternateJID = new JID(ExternalizableUtil.getInstance().readSafeUTF(in));
         }
         if (ExternalizableUtil.getInstance().readBoolean(in)) {
             reason = ExternalizableUtil.getInstance().readSafeUTF(in);

src/java/org/jivesoftware/openfire/muc/spi/ConversationLogEntry.java

@@ -30,21 +30,23 @@ import org.xmpp.packet.JID;
  * Represents an entry in the conversation log of a room. An entry basically obtains the necessary
  * information to log from the message adding a timestamp of when the message was sent to the room.
  * 
+ * Instances of this class are immutable, and therefor thread safe.
+ * 
  * @author Gaston Dombiak
  */
 class ConversationLogEntry {
 
-    private Date date;
+    private final Date date;
 
-    private String subject;
+    private final String subject;
 
-    private String body;
+    private final String body;
 
-    private JID sender;
+    private final JID sender;
     
-    private String nickname;
+    private final String nickname;
     
-    private long roomID;
+    private final long roomID;
 
     /**
      * Creates a new ConversationLogEntry that registers that a given message was sent to a given

src/java/org/jivesoftware/openfire/muc/spi/IQAdminHandler.java

@@ -47,11 +47,11 @@ import org.xmpp.packet.Presence;
  */
 public class IQAdminHandler {
 	
-	private LocalMUCRoom room;
+	private final LocalMUCRoom room;
 
-    private PacketRouter router;
+    private final PacketRouter router;
 
-    private boolean skipInvite;
+    private final boolean skipInvite;
 
     public IQAdminHandler(LocalMUCRoom chatroom, PacketRouter packetRouter) {
         this.room = chatroom;
@@ -89,7 +89,9 @@ public class IQAdminHandler {
         Element element = packet.getChildElement();
 
         // Analyze the action to perform based on the included element
-        List itemsList = element.elements("item");
+        @SuppressWarnings("unchecked")
+		List<Element> itemsList = element.elements("item");
+        
         if (!itemsList.isEmpty()) {
             handleItemsElement(role, itemsList, reply);
         }
@@ -122,13 +124,13 @@ public class IQAdminHandler {
      * @throws NotAllowedException Thrown if trying to ban an owner or an administrator.
      * @throws CannotBeInvitedException If the user being invited as a result of being added to a members-only room still does not have permission
      */
-    private void handleItemsElement(MUCRole senderRole, List itemsList, IQ reply)
+    private void handleItemsElement(MUCRole senderRole, List<Element> itemsList, IQ reply)
             throws ForbiddenException, ConflictException, NotAllowedException, CannotBeInvitedException {
         Element item;
         String affiliation;
         String roleAttribute;
-        boolean hasJID = ((Element)itemsList.get(0)).attributeValue("jid") != null;
-        boolean hasNick = ((Element)itemsList.get(0)).attributeValue("nick") != null;
+        boolean hasJID = itemsList.get(0).attributeValue("jid") != null;
+        boolean hasNick = itemsList.get(0).attributeValue("nick") != null;
         // Check if the client is requesting or changing the list of moderators/members/etc.
         if (!hasJID && !hasNick) {
             // The client is requesting the list of moderators/members/participants/outcasts
@@ -147,10 +149,10 @@ public class IQAdminHandler {
                             && MUCRole.Affiliation.owner != senderRole.getAffiliation()) {
                         throw new ForbiddenException();
                     }
-                    for (String jid : room.getOutcasts()) {
+                    for (JID jid : room.getOutcasts()) {
                         metaData = result.addElement("item", "http://jabber.org/protocol/muc#admin");
                         metaData.addAttribute("affiliation", "outcast");
-                        metaData.addAttribute("jid", jid);
+                        metaData.addAttribute("jid", jid.toString());
                     }
 
                 } else if ("member".equals(affiliation)) {
@@ -161,10 +163,10 @@ public class IQAdminHandler {
                             && MUCRole.Affiliation.owner != senderRole.getAffiliation()) {
                         throw new ForbiddenException();
                     }
-                    for (String jid : room.getMembers()) {
+                    for (JID jid : room.getMembers()) {
                         metaData = result.addElement("item", "http://jabber.org/protocol/muc#admin");
                         metaData.addAttribute("affiliation", "member");
-                        metaData.addAttribute("jid", jid);
+                        metaData.addAttribute("jid", jid.toString());
                         try {
                             List<MUCRole> roles = room.getOccupantsByBareJID(jid);
                             MUCRole role = roles.get(0);
@@ -210,7 +212,7 @@ public class IQAdminHandler {
             JID jid;
             String nick;
             String target;
-            boolean hasAffiliation = ((Element) itemsList.get(0)).attributeValue("affiliation") !=
+            boolean hasAffiliation = itemsList.get(0).attributeValue("affiliation") !=
                     null;
 
             // Keep a registry of the updated presences
@@ -246,7 +248,7 @@ public class IQAdminHandler {
                         presences.add(room.addVisitor(jid, senderRole));
                     } else if ("member".equals(target)) {
                         // Add the user as a member of the room based on the bare JID
-                        boolean hadAffiliation = room.getAffiliation(jid.toBareJID()) != MUCRole.Affiliation.none;
+                        boolean hadAffiliation = room.getAffiliation(jid) != MUCRole.Affiliation.none;
                         presences.addAll(room.addMember(jid, nick, senderRole));
                         // If the user had an affiliation don't send an invitation. Otherwise
                         // send an invitation if the room is members-only and skipping invites

src/java/org/jivesoftware/openfire/muc/spi/IQMUCRegisterHandler.java

@@ -52,16 +52,9 @@ class IQMUCRegisterHandler {
 
 	private static final Logger Log = LoggerFactory.getLogger(IQMUCRegisterHandler.class);
 
-    private static Element probeResult;
-    private MultiUserChatService mucService;
+    private static final Element probeResult;
     
-    public IQMUCRegisterHandler(MultiUserChatService mucService) {
-        this.mucService = mucService;
-        initialize();
-    }
-
-    public void initialize() {
-        if (probeResult == null) {
+    static {
         // Create the registration form of the room which contains information
         // such as: first name, last name and  nickname.
         final DataForm registrationForm = new DataForm(DataForm.Type.form);
@@ -111,6 +104,11 @@ class IQMUCRegisterHandler {
         probeResult = DocumentHelper.createElement(QName.get("query", "jabber:iq:register"));
         probeResult.add(registrationForm.getElement());
     }
+    
+    private final MultiUserChatService mucService;
+
+    public IQMUCRegisterHandler(MultiUserChatService mucService) {
+        this.mucService = mucService;
     }
 
     public IQ handleIQ(IQ packet) {
@@ -138,16 +136,19 @@ class IQMUCRegisterHandler {
 
         if (IQ.Type.get == packet.getType()) {
             reply = IQ.createResultIQ(packet);
-            String nickname = room.getReservedNickname(packet.getFrom().toBareJID());
+            String nickname = room.getReservedNickname(packet.getFrom());
             Element currentRegistration = probeResult.createCopy();
             if (nickname != null) {
                 // The user is already registered with the room so answer a completed form
                 ElementUtil.setProperty(currentRegistration, "query.registered", null);
                 Element form = currentRegistration.element(QName.get("x", "jabber:x:data"));
-                Iterator fields = form.elementIterator("field");
+                
+                @SuppressWarnings("unchecked")
+				Iterator<Element> fields = form.elementIterator("field");
+                
                 Element field;
                 while (fields.hasNext()) {
-                    field = (Element) fields.next();
+                    field = fields.next();
                     if ("muc#register_roomnick".equals(field.attributeValue("var"))) {
                         field.addElement("value").addText(nickname);
                     }

src/java/org/jivesoftware/openfire/muc/spi/IQOwnerHandler.java

@@ -60,15 +60,15 @@ public class IQOwnerHandler {
 	
 	private static final Logger Log = LoggerFactory.getLogger(IQOwnerHandler.class);
 
-    private LocalMUCRoom room;
+    private final LocalMUCRoom room;
 
-    private PacketRouter router;
+    private final PacketRouter router;
 
     private DataForm configurationForm;
 
     private Element probeResult;
 
-    private boolean skipInvite;
+    private final boolean skipInvite;
 
     public IQOwnerHandler(LocalMUCRoom chatroom, PacketRouter packetRouter) {
         this.room = chatroom;
@@ -122,8 +122,8 @@ public class IQOwnerHandler {
                     }
                 }
 
-                room.destroyRoom(destroyElement.attributeValue("jid"), destroyElement
-                        .elementTextTrim("reason"));
+				room.destroyRoom(new JID(destroyElement.attributeValue("jid")),
+						destroyElement.elementTextTrim("reason"));
             }
             else {
                 List<Element> itemsList = element.elements("item");
@@ -183,10 +183,10 @@ public class IQOwnerHandler {
                     // The client is requesting the list of owners
                     Element ownerMetaData;
                     MUCRole role;
-                    for (String jid : room.getOwners()) {
+                    for (JID jid : room.getOwners()) {
                         ownerMetaData = result.addElement("item", "http://jabber.org/protocol/muc#owner");
                         ownerMetaData.addAttribute("affiliation", "owner");
-                        ownerMetaData.addAttribute("jid", jid);
+                        ownerMetaData.addAttribute("jid", jid.toString());
                         // Add role and nick to the metadata if the user is in the room
                         try {
                             List<MUCRole> roles = room.getOccupantsByBareJID(jid);
@@ -202,10 +202,10 @@ public class IQOwnerHandler {
                     // The client is requesting the list of admins
                     Element adminMetaData;
                     MUCRole role;
-                    for (String jid : room.getAdmins()) {
+                    for (JID jid : room.getAdmins()) {
                         adminMetaData = result.addElement("item", "http://jabber.org/protocol/muc#owner");
                         adminMetaData.addAttribute("affiliation", "admin");
-                        adminMetaData.addAttribute("jid", jid);
+                        adminMetaData.addAttribute("jid", jid.toString());
                         // Add role and nick to the metadata if the user is in the room
                         try {
                             List<MUCRole> roles = room.getOccupantsByBareJID(jid);
@@ -270,7 +270,7 @@ public class IQOwnerHandler {
                             presences.addAll(room.addAdmin(jid, senderRole));
                         } else if ("member".equals(targetAffiliation)) {
                             // Add the new user as a member of the room
-                            boolean hadAffiliation = room.getAffiliation(jid.toBareJID()) != MUCRole.Affiliation.none;
+                            boolean hadAffiliation = room.getAffiliation(jid) != MUCRole.Affiliation.none;
                             presences.addAll(room.addMember(jid, null, senderRole));
                             // If the user had an affiliation don't send an invitation. Otherwise
                             // send an invitation if the room is members-only and skipping invites
@@ -365,17 +365,25 @@ public class IQOwnerHandler {
         // Get the new list of admins
         field = completedForm.getField("muc#roomconfig_roomadmins");
         boolean adminsSent = field != null;
-        List<String> admins = new ArrayList<String>();
+        List<JID> admins = new ArrayList<JID>();
         if (field != null) {
-        	admins.addAll(field.getValues());
+        	for (String value : field.getValues()) {
+        		// XEP-0045: "Affiliations are granted, revoked, and 
+        		// maintained based on the user's bare JID, (...)"
+        		admins.add(new JID(new JID(value).toBareJID()));
+        	}
         }
 
         // Get the new list of owners
         field = completedForm.getField("muc#roomconfig_roomowners");
         boolean ownersSent = field != null;
-        List<String> owners = new ArrayList<String>(); 
+        List<JID> owners = new ArrayList<JID>(); 
         if (field != null) {
-        	owners.addAll(field.getValues());
+        	for(String value : field.getValues()) {
+        		// XEP-0045: "Affiliations are granted, revoked, and 
+        		// maintained based on the user's bare JID, (...)"
+        		owners.add(new JID(new JID(value).toBareJID()));
+        	}
         }
 
         // Answer a conflic error if all the current owners will be removed
@@ -526,22 +534,22 @@ public class IQOwnerHandler {
         if (ownersSent) {
             // Change the affiliation to "member" for the current owners that won't be neither
             // owner nor admin (if the form included the owners field)
-            List<String> ownersToRemove = new ArrayList<String>(room.owners);
+            List<JID> ownersToRemove = new ArrayList<JID>(room.owners);
             ownersToRemove.removeAll(admins);
             ownersToRemove.removeAll(owners);
-            for (String jid : ownersToRemove) {
-                presences.addAll(room.addMember(new JID(jid), null, senderRole));
+            for (JID jid : ownersToRemove) {
+                presences.addAll(room.addMember(jid, null, senderRole));
             }
         }
 
         if (adminsSent) {
             // Change the affiliation to "member" for the current admins that won't be neither
             // owner nor admin (if the form included the admins field)
-            List<String> adminsToRemove = new ArrayList<String>(room.admins);
+            List<JID> adminsToRemove = new ArrayList<JID>(room.admins);
             adminsToRemove.removeAll(admins);
             adminsToRemove.removeAll(owners);
-            for (String jid : adminsToRemove) {
-                presences.addAll(room.addMember(new JID(jid), null, senderRole));
+            for (JID jid : adminsToRemove) {
+                presences.addAll(room.addMember(jid, null, senderRole));
             }
         }
 
@@ -632,14 +640,14 @@ public class IQOwnerHandler {
 
             field = configurationForm.getField("muc#roomconfig_roomadmins");
             field.clearValues();
-            for (String jid : room.getAdmins()) {
-                field.addValue(jid);
+            for (JID jid : room.getAdmins()) {
+                field.addValue(jid.toString());
             }
 
             field = configurationForm.getField("muc#roomconfig_roomowners");
             field.clearValues();
-            for (String jid : room.getOwners()) {
-                field.addValue(jid);
+            for (JID jid : room.getOwners()) {
+                field.addValue(jid.toString());
             }
 
             // Remove the old element

src/java/org/jivesoftware/openfire/muc/spi/LocalMUCRoom.java

@@ -31,6 +31,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -111,12 +112,12 @@ public class LocalMUCRoom implements MUCRoom {
     /**
      * The occupants of the room accessible by the occupants bare JID.
      */
-    private Map<String, List<MUCRole>> occupantsByBareJID = new ConcurrentHashMap<String, List<MUCRole>>();
+    private ConcurrentMap<JID, List<MUCRole>> occupantsByBareJID = new ConcurrentHashMap<JID, List<MUCRole>>();
 
     /**
      * The occupants of the room accessible by the occupants full JID.
      */
-    private Map<JID, MUCRole> occupantsByFullJID = new ConcurrentHashMap<JID, MUCRole>();
+    private ConcurrentMap<JID, MUCRole> occupantsByFullJID = new ConcurrentHashMap<JID, MUCRole>();
 
     /**
      * The name of the room.
@@ -169,22 +170,22 @@ public class LocalMUCRoom implements MUCRoom {
     /**
      * List of chatroom's owner. The list contains only bare jid.
      */
-    List<String> owners = new CopyOnWriteArrayList<String>();
+    List<JID> owners = new CopyOnWriteArrayList<JID>();
 
     /**
      * List of chatroom's admin. The list contains only bare jid.
      */
-    List<String> admins = new CopyOnWriteArrayList<String>();
+    List<JID> admins = new CopyOnWriteArrayList<JID>();
 
     /**
-     * List of chatroom's members. The list contains only bare jid.
+     * List of chatroom's members. The list contains only bare jid, mapped to a nickname.
      */
-    private Map<String, String> members = new ConcurrentHashMap<String,String>();
+    private ConcurrentMap<JID, String> members = new ConcurrentHashMap<JID,String>();
 
     /**
      * List of chatroom's outcast. The list contains only bare jid of not allowed users.
      */
-    private List<String> outcasts = new CopyOnWriteArrayList<String>();
+    private List<JID> outcasts = new CopyOnWriteArrayList<JID>();
 
     /**
      * The natural language name of the room.
@@ -445,7 +446,7 @@ public class LocalMUCRoom implements MUCRoom {
         throw new UserNotFoundException();
     }
 
-    public List<MUCRole> getOccupantsByBareJID(String jid) throws UserNotFoundException {
+    public List<MUCRole> getOccupantsByBareJID(JID jid) throws UserNotFoundException {
         List<MUCRole> roles = occupantsByBareJID.get(jid);
         if (roles != null && !roles.isEmpty()) {
             return Collections.unmodifiableList(roles);
@@ -473,7 +474,8 @@ public class LocalMUCRoom implements MUCRoom {
         return occupants.containsKey(nickname.toLowerCase());
     }
 
-    public String getReservedNickname(String bareJID) {
+    public String getReservedNickname(JID jid) {
+    	final JID bareJID = new JID(jid.toBareJID());
         String answer = members.get(bareJID);
         if (answer == null || answer.trim().length() == 0) {
             return null;
@@ -481,7 +483,9 @@ public class LocalMUCRoom implements MUCRoom {
         return answer;
     }
 
-    public MUCRole.Affiliation getAffiliation(String bareJID) {
+    public MUCRole.Affiliation getAffiliation(JID jid) {
+    	final JID bareJID = new JID(jid.toBareJID());
+
         if (owners.contains(bareJID)) {
             return MUCRole.Affiliation.owner;
         }
@@ -515,7 +519,8 @@ public class LocalMUCRoom implements MUCRoom {
             if (isDestroyed || (getMaxUsers() > 0 && getOccupantsCount() >= getMaxUsers())) {
                 throw new ServiceUnavailableException();
             }
-            boolean isOwner = owners.contains(user.getAddress().toBareJID());
+            final JID bareJID = new JID(user.getAddress().toBareJID());
+			boolean isOwner = owners.contains(bareJID);
             // If the room is locked and this user is not an owner raise a RoomLocked exception
             if (isLocked()) {
                 if (!isOwner) {
@@ -524,7 +529,7 @@ public class LocalMUCRoom implements MUCRoom {
             }
             // Check if the nickname is already used in the room
             if (occupants.containsKey(nickname.toLowerCase())) {
-                if (occupants.get(nickname.toLowerCase()).getUserAddress().toBareJID().equals(user.getAddress().toBareJID())) {
+                if (occupants.get(nickname.toLowerCase()).getUserAddress().toBareJID().equals(bareJID.toBareJID())) {
                     // Nickname exists in room, and belongs to this user, pretend to kick the previous instance.
                     // The previous instance will see that they are disconnected, and the new instance will
                     // "take over" the previous role.  Participants in the room shouldn't notice anything
@@ -560,12 +565,12 @@ public class LocalMUCRoom implements MUCRoom {
             // If another user attempts to join the room with a nickname reserved by the first user
             // raise a ConflictException
             if (members.containsValue(nickname)) {
-                if (!nickname.equals(members.get(user.getAddress().toBareJID()))) {
+                if (!nickname.equals(members.get(bareJID))) {
                     throw new ConflictException();
                 }
             }
             if (isLoginRestrictedToNickname()) {
-                String reservedNickname = members.get(user.getAddress().toBareJID());
+                String reservedNickname = members.get(bareJID);
                 if (reservedNickname != null && !nickname.equals(reservedNickname)) {
                     throw new NotAcceptableException();
                 }
@@ -579,23 +584,23 @@ public class LocalMUCRoom implements MUCRoom {
                 role = MUCRole.Role.moderator;
                 affiliation = MUCRole.Affiliation.owner;
             }
-            else if (mucService.getSysadmins().contains(user.getAddress().toBareJID())) {
+            else if (mucService.getSysadmins().contains(bareJID)) {
                 // The user is a system administrator of the MUC service. Treat him as an owner
                 // although he won't appear in the list of owners
                 role = MUCRole.Role.moderator;
                 affiliation = MUCRole.Affiliation.owner;
             }
-            else if (admins.contains(user.getAddress().toBareJID())) {
+            else if (admins.contains(bareJID)) {
                 // The user is an admin. Set the role and affiliation accordingly.
                 role = MUCRole.Role.moderator;
                 affiliation = MUCRole.Affiliation.admin;
             }
-            else if (members.containsKey(user.getAddress().toBareJID())) {
+            else if (members.containsKey(bareJID)) {
                 // The user is a member. Set the role and affiliation accordingly.
                 role = MUCRole.Role.participant;
                 affiliation = MUCRole.Affiliation.member;
             }
-            else if (outcasts.contains(user.getAddress().toBareJID())) {
+            else if (outcasts.contains(bareJID)) {
                 // The user is an outcast. Raise a "Forbidden" exception.
                 throw new ForbiddenException();
             }
@@ -614,10 +619,10 @@ public class LocalMUCRoom implements MUCRoom {
             // Add the new user as an occupant of this room
             occupants.put(nickname.toLowerCase(), joinRole);
             // Update the tables of occupants based on the bare and full JID
-            List<MUCRole> list = occupantsByBareJID.get(user.getAddress().toBareJID());
+            List<MUCRole> list = occupantsByBareJID.get(bareJID);
             if (list == null) {
                 list = new ArrayList<MUCRole>();
-                occupantsByBareJID.put(user.getAddress().toBareJID(), list);
+                occupantsByBareJID.put(bareJID, list);
             }
             list.add(joinRole);
             occupantsByFullJID.put(user.getAddress(), joinRole);
@@ -674,9 +679,9 @@ public class LocalMUCRoom implements MUCRoom {
             joinRole.send(message);
         }
         if (historyRequest == null) {
-            Iterator history = roomHistory.getMessageHistory();
+            Iterator<Message> history = roomHistory.getMessageHistory();
             while (history.hasNext()) {
-                joinRole.send((Message) history.next());
+                joinRole.send(history.next());
             }
         }
         else {
@@ -732,10 +737,11 @@ public class LocalMUCRoom implements MUCRoom {
         // Add the new user as an occupant of this room
         occupants.put(event.getNickname().toLowerCase(), joinRole);
         // Update the tables of occupants based on the bare and full JID
-        List<MUCRole> list = occupantsByBareJID.get(event.getUserAddress().toBareJID());
+        JID bareJID = new JID(event.getUserAddress().toBareJID());
+		List<MUCRole> list = occupantsByBareJID.get(bareJID);
         if (list == null) {
             list = new ArrayList<MUCRole>();
-            occupantsByBareJID.put(event.getUserAddress().toBareJID(), list);
+            occupantsByBareJID.put(bareJID, list);
         }
         list.add(joinRole);
         occupantsByFullJID.put(event.getUserAddress(), joinRole);
@@ -852,11 +858,12 @@ public class LocalMUCRoom implements MUCRoom {
         // Notify the user that he/she is no longer in the room
         leaveRole.destroy();
         // Update the tables of occupants based on the bare and full JID
-        List list = occupantsByBareJID.get(userAddress.toBareJID());
+        JID bareJID = new JID(userAddress.toBareJID());
+		List<MUCRole> list = occupantsByBareJID.get(bareJID);
         if (list != null) {
             list.remove(leaveRole);
             if (list.isEmpty()) {
-                occupantsByBareJID.remove(userAddress.toBareJID());
+                occupantsByBareJID.remove(bareJID);
             }
         }
         occupantsByFullJID.remove(userAddress);
@@ -867,7 +874,7 @@ public class LocalMUCRoom implements MUCRoom {
     }
 
     public void destroyRoom(DestroyRoomRequest destroyRequest) {
-        String alternateJID = destroyRequest.getAlternateJID();
+        JID alternateJID = destroyRequest.getAlternateJID();
         String reason = destroyRequest.getReason();
         MUCRole leaveRole;
         Collection<MUCRole> removedRoles = new ArrayList<MUCRole>();
@@ -918,8 +925,8 @@ public class LocalMUCRoom implements MUCRoom {
                 Element item = fragment.addElement("item");
                 item.addAttribute("affiliation", "none");
                 item.addAttribute("role", "none");
-                if (alternateJID != null && alternateJID.length() > 0) {
-                    fragment.addElement("destroy").addAttribute("jid", alternateJID);
+                if (alternateJID != null) {
+                    fragment.addElement("destroy").addAttribute("jid", alternateJID.toFullJID());
                 }
                 if (reason != null && reason.length() > 0) {
                     Element destroy = fragment.element("destroy");
@@ -942,7 +949,7 @@ public class LocalMUCRoom implements MUCRoom {
         }
     }
 
-    public void destroyRoom(String alternateJID, String reason) {
+    public void destroyRoom(JID alternateJID, String reason) {
         DestroyRoomRequest destroyRequest = new DestroyRoomRequest(this, alternateJID, reason);
         destroyRequest.setOriginator(true);
         destroyRequest.run();
@@ -1230,7 +1237,8 @@ public class LocalMUCRoom implements MUCRoom {
             throws NotAllowedException {
         List<Presence> presences = new ArrayList<Presence>();
         // Get all the roles (i.e. occupants) of this user based on his/her bare JID
-        List<MUCRole> roles = occupantsByBareJID.get(jid.toBareJID());
+        JID bareJID = new JID(jid.toBareJID());
+		List<MUCRole> roles = occupantsByBareJID.get(bareJID);
         if (roles == null) {
             return presences;
         }
@@ -1303,16 +1311,12 @@ public class LocalMUCRoom implements MUCRoom {
         return null;
     }
 
-    public void addFirstOwner(String bareJID) {
-        owners.add(bareJID);
-    }
-
-    @Deprecated
-    public List<Presence> addOwner(String bareJID, MUCRole sendRole) throws ForbiddenException {
-        return addOwner(new JID(bareJID), sendRole);
+    public void addFirstOwner(JID bareJID) {
+        owners.add(new JID(bareJID.toBareJID()));
     }
 
     public List<Presence> addOwner(JID jid, MUCRole sendRole) throws ForbiddenException {
+        final JID bareJID = new JID(jid.toBareJID());
         lock.writeLock().lock();
         try {
             MUCRole.Affiliation oldAffiliation = MUCRole.Affiliation.none;
@@ -1320,25 +1324,25 @@ public class LocalMUCRoom implements MUCRoom {
                 throw new ForbiddenException();
             }
             // Check if user is already an owner
-            if (owners.contains(jid.toBareJID())) {
+			if (owners.contains(bareJID)) {
                 // Do nothing
                 return Collections.emptyList();
             }
-            owners.add(jid.toBareJID());
+            owners.add(bareJID);
             // Remove the user from other affiliation lists
-            if (removeAdmin(jid.toBareJID())) {
+            if (removeAdmin(bareJID)) {
                 oldAffiliation = MUCRole.Affiliation.admin;
             }
-            else if (removeMember(jid.toBareJID())) {
+            else if (removeMember(bareJID)) {
                 oldAffiliation = MUCRole.Affiliation.member;
             }
-            else if (removeOutcast(jid.toBareJID())) {
+            else if (removeOutcast(bareJID)) {
                 oldAffiliation = MUCRole.Affiliation.outcast;
             }
             // Update the DB if the room is persistent
             MUCPersistenceManager.saveAffiliationToDB(
                 this,
-                jid.toBareJID(),
+                bareJID,
                 null,
                 MUCRole.Affiliation.owner,
                 oldAffiliation);
@@ -1357,18 +1361,13 @@ public class LocalMUCRoom implements MUCRoom {
         }
     }
 
-    private boolean removeOwner(String bareJID) {
-        return owners.remove(bareJID);
-    }
-
-    @Deprecated
-    public List<Presence> addAdmin(String bareJID, MUCRole sendRole) throws ForbiddenException,
-    ConflictException {
-    	return addAdmin(new JID(bareJID), sendRole);
+    private boolean removeOwner(JID jid) {
+        return owners.remove(new JID(jid.toBareJID()));
     }
 
     public List<Presence> addAdmin(JID jid, MUCRole sendRole) throws ForbiddenException,
             ConflictException {
+    	final JID bareJID = new JID(jid.toBareJID());
         lock.writeLock().lock();
         try {
             MUCRole.Affiliation oldAffiliation = MUCRole.Affiliation.none;
@@ -1376,29 +1375,29 @@ public class LocalMUCRoom implements MUCRoom {
                 throw new ForbiddenException();
             }
             // Check that the room always has an owner
-            if (owners.contains(jid.toBareJID()) && owners.size() == 1) {
+            if (owners.contains(bareJID) && owners.size() == 1) {
                 throw new ConflictException();
             }
             // Check if user is already an admin
-            if (admins.contains(jid.toBareJID())) {
+            if (admins.contains(bareJID)) {
                 // Do nothing
                 return Collections.emptyList();
             }
-            admins.add(jid.toBareJID());
+            admins.add(bareJID);
             // Remove the user from other affiliation lists
-            if (removeOwner(jid.toBareJID())) {
+            if (removeOwner(bareJID)) {
                 oldAffiliation = MUCRole.Affiliation.owner;
             }
-            else if (removeMember(jid.toBareJID())) {
+            else if (removeMember(bareJID)) {
                 oldAffiliation = MUCRole.Affiliation.member;
             }
-            else if (removeOutcast(jid.toBareJID())) {
+            else if (removeOutcast(bareJID)) {
                 oldAffiliation = MUCRole.Affiliation.outcast;
             }
             // Update the DB if the room is persistent
             MUCPersistenceManager.saveAffiliationToDB(
                 this,
-                jid.toBareJID(),
+                bareJID,
                 null,
                 MUCRole.Affiliation.admin,
                 oldAffiliation);
@@ -1417,21 +1416,16 @@ public class LocalMUCRoom implements MUCRoom {
         }
     }
 
-    private boolean removeAdmin(String bareJID) {
-        return admins.remove(bareJID);
-    }
-
-    @Deprecated
-    public List<Presence> addMember(String bareJID, String nickname, MUCRole sendRole)
-            throws ForbiddenException, ConflictException {
-        return addMember(new JID(bareJID), nickname, sendRole);
+    private boolean removeAdmin(JID bareJID) {
+        return admins.remove(new JID(bareJID.toBareJID()));
     }
 
     public List<Presence> addMember(JID jid, String nickname, MUCRole sendRole)
             throws ForbiddenException, ConflictException {
+    	final JID bareJID = new JID(jid.toBareJID());
         lock.writeLock().lock();
         try {
-            MUCRole.Affiliation oldAffiliation = (members.containsKey(jid.toBareJID()) ?
+            MUCRole.Affiliation oldAffiliation = (members.containsKey(bareJID) ?
                     MUCRole.Affiliation.member : MUCRole.Affiliation.none);
             if (isMembersOnly()) {
                 if (!canOccupantsInvite()) {
@@ -1449,31 +1443,31 @@ public class LocalMUCRoom implements MUCRoom {
             }
             // Check if the desired nickname is already reserved for another member
             if (nickname != null && nickname.trim().length() > 0 && members.containsValue(nickname)) {
-                if (!nickname.equals(members.get(jid.toBareJID()))) {
+                if (!nickname.equals(members.get(bareJID))) {
                     throw new ConflictException();
                 }
             }
             // Check that the room always has an owner
-            if (owners.contains(jid.toBareJID()) && owners.size() == 1) {
+            if (owners.contains(bareJID) && owners.size() == 1) {
                 throw new ConflictException();
             }
             // Associate the reserved nickname with the bareJID. If nickname is null then associate an
             // empty string
-            members.put(jid.toBareJID(), (nickname == null ? "" : nickname));
+            members.put(bareJID, (nickname == null ? "" : nickname));
             // Remove the user from other affiliation lists
-            if (removeOwner(jid.toBareJID())) {
+            if (removeOwner(bareJID)) {
                 oldAffiliation = MUCRole.Affiliation.owner;
             }
-            else if (removeAdmin(jid.toBareJID())) {
+            else if (removeAdmin(bareJID)) {
                 oldAffiliation = MUCRole.Affiliation.admin;
             }
-            else if (removeOutcast(jid.toBareJID())) {
+            else if (removeOutcast(bareJID)) {
                 oldAffiliation = MUCRole.Affiliation.outcast;
             }
             // Update the DB if the room is persistent
             MUCPersistenceManager.saveAffiliationToDB(
                 this,
-                jid.toBareJID(),
+                bareJID,
                 nickname,
                 MUCRole.Affiliation.member,
                 oldAffiliation);
@@ -1494,20 +1488,16 @@ public class LocalMUCRoom implements MUCRoom {
         }
     }
 
-    private boolean removeMember(String bareJID) {
+    private boolean removeMember(JID jid) {
+        final JID bareJID = new JID(jid.toBareJID());
 		boolean answer = members.containsKey(bareJID);
         members.remove(bareJID);
         return answer;
     }
 
-    @Deprecated
-    public List<Presence> addOutcast(String bareJID, String reason, MUCRole senderRole)
-            throws NotAllowedException, ForbiddenException, ConflictException {
-        return addOutcast(new JID(bareJID), reason, senderRole);
-    }
-
     public List<Presence> addOutcast(JID jid, String reason, MUCRole senderRole)
             throws NotAllowedException, ForbiddenException, ConflictException {
+        final JID bareJID = new JID(jid.toBareJID());
         lock.writeLock().lock();
         try {
             MUCRole.Affiliation oldAffiliation = MUCRole.Affiliation.none;
@@ -1516,31 +1506,31 @@ public class LocalMUCRoom implements MUCRoom {
                 throw new ForbiddenException();
             }
             // Check that the room always has an owner
-            if (owners.contains(jid.toBareJID()) && owners.size() == 1) {
+            if (owners.contains(bareJID) && owners.size() == 1) {
                 throw new ConflictException();
             }
             // Check if user is already an outcast
-            if (outcasts.contains(jid.toBareJID())) {
+            if (outcasts.contains(bareJID)) {
                 // Do nothing
                 return Collections.emptyList();
             }
 
             // Update the affiliation lists
-            outcasts.add(jid.toBareJID());
+            outcasts.add(bareJID);
             // Remove the user from other affiliation lists
-            if (removeOwner(jid.toBareJID())) {
+            if (removeOwner(bareJID)) {
                 oldAffiliation = MUCRole.Affiliation.owner;
             }
-            else if (removeAdmin(jid.toBareJID())) {
+            else if (removeAdmin(bareJID)) {
                 oldAffiliation = MUCRole.Affiliation.admin;
             }
-            else if (removeMember(jid.toBareJID())) {
+            else if (removeMember(bareJID)) {
                 oldAffiliation = MUCRole.Affiliation.member;
             }
             // Update the DB if the room is persistent
             MUCPersistenceManager.saveAffiliationToDB(
                 this,
-                jid.toBareJID(),
+                bareJID,
                 null,
                 MUCRole.Affiliation.outcast,
                 oldAffiliation);
@@ -1575,18 +1565,13 @@ public class LocalMUCRoom implements MUCRoom {
         return updatedPresences;
     }
 
-    private boolean removeOutcast(String bareJID) {
-        return outcasts.remove(bareJID);
-    }
-
-    @Deprecated
-    public List<Presence> addNone(String bareJID, MUCRole senderRole) throws ForbiddenException,
-            ConflictException {
-        return addNone(new JID(bareJID), senderRole);
+    private boolean removeOutcast(JID bareJID) {
+        return outcasts.remove(new JID(bareJID.toBareJID()));
     }
 
     public List<Presence> addNone(JID jid, MUCRole senderRole) throws ForbiddenException,
             ConflictException {
+    	final JID bareJID = new JID(jid.toBareJID());
         List<Presence> updatedPresences = null;
         boolean wasMember = false;
         lock.writeLock().lock();
@@ -1597,25 +1582,25 @@ public class LocalMUCRoom implements MUCRoom {
                 throw new ForbiddenException();
             }
             // Check that the room always has an owner
-            if (owners.contains(jid.toBareJID()) && owners.size() == 1) {
+            if (owners.contains(bareJID) && owners.size() == 1) {
                 throw new ConflictException();
             }
-            wasMember = members.containsKey(jid.toBareJID()) || admins.contains(jid.toBareJID()) || owners.contains(jid.toBareJID());
+            wasMember = members.containsKey(bareJID) || admins.contains(bareJID) || owners.contains(bareJID);
             // Remove the user from ALL the affiliation lists
-            if (removeOwner(jid.toBareJID())) {
+            if (removeOwner(bareJID)) {
                 oldAffiliation = MUCRole.Affiliation.owner;
             }
-            else if (removeAdmin(jid.toBareJID())) {
+            else if (removeAdmin(bareJID)) {
                 oldAffiliation = MUCRole.Affiliation.admin;
             }
-            else if (removeMember(jid.toBareJID())) {
+            else if (removeMember(bareJID)) {
                 oldAffiliation = MUCRole.Affiliation.member;
             }
-            else if (removeOutcast(jid.toBareJID())) {
+            else if (removeOutcast(bareJID)) {
                 oldAffiliation = MUCRole.Affiliation.outcast;
             }
             // Remove the affiliation of this user from the DB if the room is persistent
-            MUCPersistenceManager.removeAffiliationFromDB(this, jid.toBareJID(), oldAffiliation);
+            MUCPersistenceManager.removeAffiliationFromDB(this, bareJID, oldAffiliation);
         }
         finally {
             lock.writeLock().unlock();
@@ -1629,7 +1614,7 @@ public class LocalMUCRoom implements MUCRoom {
             else {
                 newRole = isModerated() ? MUCRole.Role.visitor : MUCRole.Role.participant;
             }
-            updatedPresences = changeOccupantAffiliation(jid, MUCRole.Affiliation.none, newRole);
+            updatedPresences = changeOccupantAffiliation(bareJID, MUCRole.Affiliation.none, newRole);
             if (isMembersOnly() && wasMember) {
                 // If the room is members-only, remove the user from the room including a status
                 // code of 321 to indicate that the user was removed because of an affiliation
@@ -1656,6 +1641,7 @@ public class LocalMUCRoom implements MUCRoom {
         }
         catch (NotAllowedException e) {
             // We should never receive this exception....in theory
+        	Log.error(e.getMessage(), e);
         }
         return updatedPresences;
     }
@@ -1891,19 +1877,19 @@ public class LocalMUCRoom implements MUCRoom {
         return roomHistory;
     }
 
-    public Collection<String> getOwners() {
+    public Collection<JID> getOwners() {
         return Collections.unmodifiableList(owners);
     }
 
-    public Collection<String> getAdmins() {
+    public Collection<JID> getAdmins() {
         return Collections.unmodifiableList(admins);
     }
 
-    public Collection<String> getMembers() {
+    public Collection<JID> getMembers() {
         return Collections.unmodifiableMap(members).keySet();
     }
 
-    public Collection<String> getOutcasts() {
+    public Collection<JID> getOutcasts() {
         return Collections.unmodifiableList(outcasts);
     }
 
@@ -2268,23 +2254,25 @@ public class LocalMUCRoom implements MUCRoom {
         return new Date(lockedTime);
     }
 
-    public List<Presence> addAdmins(List<String> newAdmins, MUCRole senderRole)
+    public List<Presence> addAdmins(List<JID> newAdmins, MUCRole senderRole)
             throws ForbiddenException, ConflictException {
         List<Presence> answer = new ArrayList<Presence>(newAdmins.size());
-        for (String newAdmin : newAdmins) {
-            if (newAdmin.trim().length() > 0 && !admins.contains(newAdmin)) {
-                answer.addAll(addAdmin(newAdmin, senderRole));
+        for (JID newAdmin : newAdmins) {
+        	final JID bareJID = new JID(newAdmin.toBareJID());
+            if (!admins.contains(bareJID)) {
+                answer.addAll(addAdmin(bareJID, senderRole));
             }
         }
         return answer;
     }
 
-    public List<Presence> addOwners(List<String> newOwners, MUCRole senderRole)
+    public List<Presence> addOwners(List<JID> newOwners, MUCRole senderRole)
             throws ForbiddenException {
         List<Presence> answer = new ArrayList<Presence>(newOwners.size());
-        for (String newOwner : newOwners) {
-            if (newOwner.trim().length() > 0 && !owners.contains(newOwner)) {
-                answer.addAll(addOwner(new JID(newOwner), senderRole));
+        for (JID newOwner : newOwners) {
+        	final JID bareJID = new JID(newOwner.toBareJID());
+            if (!owners.contains(newOwner)) {
+                answer.addAll(addOwner(bareJID, senderRole));
             }
         }
         return answer;
@@ -2297,7 +2285,7 @@ public class LocalMUCRoom implements MUCRoom {
             // Set that the room is now in the DB
             savedToDB = true;
             // Save the existing room owners to the DB
-            for (String owner : owners) {
+            for (JID owner : owners) {
                 MUCPersistenceManager.saveAffiliationToDB(
                     this,
                     owner,
@@ -2306,7 +2294,7 @@ public class LocalMUCRoom implements MUCRoom {
                     MUCRole.Affiliation.none);
             }
             // Save the existing room admins to the DB
-            for (String admin : admins) {
+            for (JID admin : admins) {
                 MUCPersistenceManager.saveAffiliationToDB(
                     this,
                     admin,
@@ -2315,12 +2303,12 @@ public class LocalMUCRoom implements MUCRoom {
                     MUCRole.Affiliation.none);
             }
             // Save the existing room members to the DB
-            for (String bareJID : members.keySet()) {
+            for (JID bareJID : members.keySet()) {
                 MUCPersistenceManager.saveAffiliationToDB(this, bareJID, members.get(bareJID),
                         MUCRole.Affiliation.member, MUCRole.Affiliation.none);
             }
             // Save the existing room outcasts to the DB
-            for (String outcast : outcasts) {
+            for (JID outcast : outcasts) {
                 MUCPersistenceManager.saveAffiliationToDB(
                     this,
                     outcast,
@@ -2335,10 +2323,10 @@ public class LocalMUCRoom implements MUCRoom {
         ExternalizableUtil.getInstance().writeSafeUTF(out, name);
         ExternalizableUtil.getInstance().writeLong(out, startTime);
         ExternalizableUtil.getInstance().writeLong(out, lockedTime);
-        ExternalizableUtil.getInstance().writeStringList(out, owners);
-        ExternalizableUtil.getInstance().writeStringList(out, admins);
-        ExternalizableUtil.getInstance().writeStringMap(out, members);
-        ExternalizableUtil.getInstance().writeStringList(out, outcasts);
+        ExternalizableUtil.getInstance().writeSerializableCollection(out, owners);
+        ExternalizableUtil.getInstance().writeSerializableCollection(out, admins);
+        ExternalizableUtil.getInstance().writeSerializableMap(out, members);
+        ExternalizableUtil.getInstance().writeSerializableCollection(out, outcasts);
         ExternalizableUtil.getInstance().writeSafeUTF(out, naturalLanguageName);
         ExternalizableUtil.getInstance().writeSafeUTF(out, description);
         ExternalizableUtil.getInstance().writeBoolean(out, canOccupantsChangeSubject);
@@ -2371,10 +2359,10 @@ public class LocalMUCRoom implements MUCRoom {
         name = ExternalizableUtil.getInstance().readSafeUTF(in);
         startTime = ExternalizableUtil.getInstance().readLong(in);
         lockedTime = ExternalizableUtil.getInstance().readLong(in);
-        owners.addAll(ExternalizableUtil.getInstance().readStringList(in));
-        admins.addAll(ExternalizableUtil.getInstance().readStringList(in));
-        members.putAll(ExternalizableUtil.getInstance().readStringMap(in));
-        outcasts.addAll(ExternalizableUtil.getInstance().readStringList(in));
+        ExternalizableUtil.getInstance().readSerializableCollection(in, owners, getClass().getClassLoader());
+        ExternalizableUtil.getInstance().readSerializableCollection(in, admins, getClass().getClassLoader());
+        ExternalizableUtil.getInstance().readSerializableMap(in, members, getClass().getClassLoader());
+        ExternalizableUtil.getInstance().readSerializableCollection(in, outcasts, getClass().getClassLoader());
         naturalLanguageName = ExternalizableUtil.getInstance().readSafeUTF(in);
         description = ExternalizableUtil.getInstance().readSafeUTF(in);
         canOccupantsChangeSubject = ExternalizableUtil.getInstance().readBoolean(in);

src/java/org/jivesoftware/openfire/muc/spi/LocalMUCUser.java

@@ -312,9 +312,12 @@ public class LocalMUCUser implements MUCUser {
                                     List<Element> extensions = new ArrayList<Element>(packet
                                             .getElement().elements());
                                     extensions.remove(userInfo);
+                                    
                                     // Send invitations to invitees
-                                    for (Iterator it=userInfo.elementIterator("invite");it.hasNext();) {
-                                        Element info = (Element) it.next();
+                                    @SuppressWarnings("unchecked")
+									Iterator<Element> it = userInfo.elementIterator("invite");
+                                    while(it.hasNext()) {
+                                        Element info = it.next();
                                         JID jid = new JID(info.attributeValue("to"));
 
                                         // Add the user as a member of the room if the room is

src/java/org/jivesoftware/openfire/muc/spi/MUCPersistenceManager.java

@@ -557,16 +557,8 @@ public class MUCPersistenceManager {
 				} catch (IllegalArgumentException ex) {
 					Log.warn("An illegal JID ({}) was found in the database, "
 							+ "while trying to load all affiliations for room "
-							+ "{} on the MUC service {}. An attempt is made to"
-							+ " delete the associated affiliation. The JID is"
-							+ "  otherwise ignored.", new Object[] { jidValue,
-							roomID, chatserver.getName() });
-					try {
-						removeAffiliationFromDB(room, jidValue, affiliation);
-						Log.warn("Affiliation removed.");
-					} catch (RuntimeException e) {
-						Log.warn("Unable to remove affiliation.", e);
-					}
+							+ "{} on the MUC service {}. The JID is ignored."
+							, new Object[] { jidValue, roomID, chatserver.getName() });
 					continue;
 				}
                 try {
@@ -727,9 +719,10 @@ public class MUCPersistenceManager {
      * @param newAffiliation the new affiliation of the user in the room.
      * @param oldAffiliation the previous affiliation of the user in the room.
      */
-    public static void saveAffiliationToDB(MUCRoom room, String bareJID, String nickname,
+    public static void saveAffiliationToDB(MUCRoom room, JID jid, String nickname,
             MUCRole.Affiliation newAffiliation, MUCRole.Affiliation oldAffiliation)
     {
+    	final String bareJID = jid.toBareJID();
         if (!room.isPersistent() || !room.wasSavedToDB()) {
             return;
         }
@@ -880,9 +873,10 @@ public class MUCPersistenceManager {
      * @param bareJID The bareJID of the user to remove his affiliation.
      * @param oldAffiliation the previous affiliation of the user in the room.
      */
-    public static void removeAffiliationFromDB(MUCRoom room, String bareJID,
+    public static void removeAffiliationFromDB(MUCRoom room, JID jid,
             MUCRole.Affiliation oldAffiliation)
     {
+    	final String bareJID = jid.toBareJID();
         if (room.isPersistent() && room.wasSavedToDB()) {
             if (MUCRole.Affiliation.member == oldAffiliation) {
                 // Remove the user from the members table
@@ -1005,12 +999,13 @@ public class MUCPersistenceManager {
      * @return the property value specified by name.
      */
     public static String getProperty(String subdomain, String name) {    	
-        MUCServiceProperties properties = propertyMaps.get(subdomain);
-        if (properties == null) {
-            properties = new MUCServiceProperties(subdomain);
-            propertyMaps.put(subdomain, properties);
+    	final MUCServiceProperties newProps = new MUCServiceProperties(subdomain);
+    	final MUCServiceProperties oldProps = propertyMaps.putIfAbsent(subdomain, newProps);
+    	if (oldProps != null) {
+    		return oldProps.get(name);
+    	} else {
+    		return newProps.get(name);
     	}
-        return properties.get(name);
     }
 
     /**
@@ -1023,16 +1018,10 @@ public class MUCPersistenceManager {
      * @return the property value specified by name.
      */
     public static String getProperty(String subdomain, String name, String defaultValue) {
-        MUCServiceProperties properties = propertyMaps.get(subdomain);
-        if (properties == null) {
-            properties = new MUCServiceProperties(subdomain);
-            propertyMaps.put(subdomain, properties);
-        }
-        String value = properties.get(name);
+    	final String value = getProperty(subdomain, name);
     	if (value != null) {
     		return value;
-        }
-        else {
+    	} else {
     		return defaultValue;
     	}
     }
@@ -1130,10 +1119,10 @@ public class MUCPersistenceManager {
      * @return a List of all immediate children property names (Strings).
      */
     public static List<String> getPropertyNames(String subdomain, String parent) {
-        MUCServiceProperties properties = propertyMaps.get(subdomain);
-        if (properties == null) {
-            properties = new MUCServiceProperties(subdomain);
-            propertyMaps.put(subdomain, properties);
+    	MUCServiceProperties properties = new MUCServiceProperties(subdomain);
+    	final MUCServiceProperties oldProps = propertyMaps.putIfAbsent(subdomain, properties);
+    	if (oldProps != null) {
+    		properties = oldProps;
     	} 
         return new ArrayList<String>(properties.getChildrenNames(parent));
     }
@@ -1150,10 +1139,10 @@ public class MUCPersistenceManager {
      * @return all child property values for the given parent.
      */
     public static List<String> getProperties(String subdomain, String parent) {
-        MUCServiceProperties properties = propertyMaps.get(subdomain);
-        if (properties == null) {
-            properties = new MUCServiceProperties(subdomain);
-            propertyMaps.put(subdomain, properties);
+    	MUCServiceProperties properties = new MUCServiceProperties(subdomain);
+    	final MUCServiceProperties oldProps = propertyMaps.putIfAbsent(subdomain, properties);
+    	if (oldProps != null) {
+    		properties = oldProps;
     	} 
 
         Collection<String> propertyNames = properties.getChildrenNames(parent);
@@ -1175,10 +1164,10 @@ public class MUCPersistenceManager {
      * @return a List of all property names (Strings).
      */
     public static List<String> getPropertyNames(String subdomain) {
-        MUCServiceProperties properties = propertyMaps.get(subdomain);
-        if (properties == null) {
-            properties = new MUCServiceProperties(subdomain);
-            propertyMaps.put(subdomain, properties);
+    	MUCServiceProperties properties = new MUCServiceProperties(subdomain);
+    	final MUCServiceProperties oldProps = propertyMaps.putIfAbsent(subdomain, properties);
+    	if (oldProps != null) {
+    		properties = oldProps;
     	} 
         return new ArrayList<String>(properties.getPropertyNames());
     }

src/java/org/jivesoftware/openfire/muc/spi/MultiUserChatServiceImpl.java

@@ -198,13 +198,13 @@ public class MultiUserChatServiceImpl implements Component, MultiUserChatService
      * Bare jids of users that are allowed to create MUC rooms. An empty list means that anyone can 
      * create a room. 
      */
-    private List<String> allowedToCreate = new CopyOnWriteArrayList<String>();
+    private List<JID> allowedToCreate = new CopyOnWriteArrayList<JID>();
 
     /**
      * Bare jids of users that are system administrators of the MUC service. A sysadmin has the same
      * permissions as a room owner.
      */
-    private List<String> sysadmins = new CopyOnWriteArrayList<String>();
+    private List<JID> sysadmins = new CopyOnWriteArrayList<JID>();
 
     /**
      * Queue that holds the messages to log for the rooms that need to log their conversations.
@@ -547,15 +547,16 @@ public class MultiUserChatServiceImpl implements Component, MultiUserChatService
                     else {
                         // The room does not exist so check for creation permissions
                         // Room creation is always allowed for sysadmin
-                        if (isRoomCreationRestricted() && !sysadmins.contains(userjid.toBareJID())) {
+                        final JID bareJID = new JID(userjid.toBareJID());
+						if (isRoomCreationRestricted() && !sysadmins.contains(bareJID)) {
                             // The room creation is only allowed for certain JIDs
-                            if (!allowedToCreate.contains(userjid.toBareJID())) {
+                            if (!allowedToCreate.contains(bareJID)) {
                                 // The user is not in the list of allowed JIDs to create a room so raise
                                 // an exception
                                 throw new NotAllowedException();
                             }
                         }
-                        room.addFirstOwner(userjid.toBareJID());
+                        room.addFirstOwner(userjid);
                         created = true;
                     }
                 }
@@ -806,31 +807,42 @@ public class MultiUserChatServiceImpl implements Component, MultiUserChatService
         return log_batch_size;
     }
 
-    public Collection<String> getUsersAllowedToCreate() {
-        return allowedToCreate;
+    public Collection<JID> getUsersAllowedToCreate() {
+        return Collections.unmodifiableCollection(allowedToCreate);
     }
 
-    public Collection<String> getSysadmins() {
-        return sysadmins;
+    public Collection<JID> getSysadmins() {
+        return Collections.unmodifiableCollection(sysadmins);
     }
 
-    public void addSysadmin(String userJID) {
-        sysadmins.add(userJID.trim().toLowerCase());
+    public void addSysadmin(JID userJID) {
+    	final JID bareJID = new JID(userJID.toBareJID());
+    	
+        sysadmins.add(bareJID);
+        
         // CopyOnWriteArray does not allow sorting, so do sorting in temp list.
-        ArrayList<String> tempList = new ArrayList<String>(sysadmins);
+        ArrayList<JID> tempList = new ArrayList<JID>(sysadmins);
         Collections.sort(tempList);
-        sysadmins = new CopyOnWriteArrayList<String>(tempList);
+        sysadmins = new CopyOnWriteArrayList<JID>(tempList);
+        
         // Update the config.
         String[] jids = new String[sysadmins.size()];
-        jids = sysadmins.toArray(jids);
+        for (int i = 0; i < jids.length; i++) {
+			jids[i] = sysadmins.get(i).toBareJID();
+		}
         MUCPersistenceManager.setProperty(chatServiceName, "sysadmin.jid", fromArray(jids));
     }
 
-    public void removeSysadmin(String userJID) {
-        sysadmins.remove(userJID.trim().toLowerCase());
+    public void removeSysadmin(JID userJID) {
+    	final JID bareJID = new JID(userJID.toBareJID());
+    	
+        sysadmins.remove(bareJID);
+        
         // Update the config.
         String[] jids = new String[sysadmins.size()];
-        jids = sysadmins.toArray(jids);
+        for (int i = 0; i < jids.length; i++) {
+			jids[i] = sysadmins.get(i).toBareJID();
+		}
         MUCPersistenceManager.setProperty(chatServiceName, "sysadmin.jid", fromArray(jids));
     }
 
@@ -868,27 +880,39 @@ public class MultiUserChatServiceImpl implements Component, MultiUserChatService
         MUCPersistenceManager.setProperty(chatServiceName, "create.anyone", Boolean.toString(roomCreationRestricted));
     }
 
-    public void addUserAllowedToCreate(String userJID) {
+    public void addUserAllowedToCreate(JID userJID) {
+    	final JID bareJID = new JID(userJID.toBareJID());
+    	
         // Update the list of allowed JIDs to create MUC rooms. Since we are updating the instance
         // variable there is no need to restart the service
-        allowedToCreate.add(userJID.trim().toLowerCase());
+    	
+        allowedToCreate.add(bareJID);
+        
         // CopyOnWriteArray does not allow sorting, so do sorting in temp list.
-        ArrayList<String> tempList = new ArrayList<String>(allowedToCreate);
+        ArrayList<JID> tempList = new ArrayList<JID>(allowedToCreate);
         Collections.sort(tempList);
-        allowedToCreate = new CopyOnWriteArrayList<String>(tempList);
+        allowedToCreate = new CopyOnWriteArrayList<JID>(tempList);
+        
         // Update the config.
         String[] jids = new String[allowedToCreate.size()];
-        jids = allowedToCreate.toArray(jids);
+        for (int i = 0; i < jids.length; i++) {
+			jids[i] = allowedToCreate.get(i).toBareJID();
+		}
         MUCPersistenceManager.setProperty(chatServiceName, "create.jid", fromArray(jids));
     }
 
-    public void removeUserAllowedToCreate(String userJID) {
+    public void removeUserAllowedToCreate(JID userJID) {
+    	final JID bareJID = new JID(userJID.toBareJID());
+    	
         // Update the list of allowed JIDs to create MUC rooms. Since we are updating the instance
         // variable there is no need to restart the service
-        allowedToCreate.remove(userJID.trim().toLowerCase());
+        allowedToCreate.remove(bareJID);
+        
         // Update the config.
         String[] jids = new String[allowedToCreate.size()];
-        jids = allowedToCreate.toArray(jids);
+        for (int i = 0; i < jids.length; i++) {
+			jids[i] = allowedToCreate.get(i).toBareJID();
+		}
         MUCPersistenceManager.setProperty(chatServiceName, "create.jid", fromArray(jids));
     }
 
@@ -915,7 +939,7 @@ public class MultiUserChatServiceImpl implements Component, MultiUserChatService
         if (property != null) {
             jids = property.split(",");
             for (String jid : jids) {
-                sysadmins.add(jid.trim().toLowerCase());
+                sysadmins.add(new JID(new JID(jid.trim().toLowerCase()).toBareJID()));
             }
         }
         allowToDiscoverLockedRooms =
@@ -928,7 +952,7 @@ public class MultiUserChatServiceImpl implements Component, MultiUserChatService
         if (property != null) {
             jids = property.split(",");
             for (String jid : jids) {
-                allowedToCreate.add(jid.trim().toLowerCase());
+            	allowedToCreate.add(new JID(new JID(jid.trim().toLowerCase()).toBareJID()));
             }
         }
         String value = MUCPersistenceManager.getProperty(chatServiceName, "tasks.user.timeout");
@@ -1201,7 +1225,7 @@ public class MultiUserChatServiceImpl implements Component, MultiUserChatService
             // Answer reserved nickname for the sender of the disco request in the requested room
             MUCRoom room = getChatRoom(name);
             if (room != null) {
-                String reservedNick = room.getReservedNickname(senderJID.toBareJID());
+                String reservedNick = room.getReservedNickname(senderJID);
                 if (reservedNick != null) {
                     Element identity = DocumentHelper.createElement("identity");
                     identity.addAttribute("category", "conference");
@@ -1346,9 +1370,11 @@ public class MultiUserChatServiceImpl implements Component, MultiUserChatService
      * @param name Name of identity to remove.
      */
     public void removeExtraIdentity(String name) {
-        for (Element elem : extraDiscoIdentities) {
+    	final Iterator<Element> iter = extraDiscoIdentities.iterator();
+    	while (iter.hasNext()) {
+			Element elem = iter.next();
             if (name.equals(elem.attribute("name").getStringValue())) {
-                extraDiscoFeatures.remove(elem);
+                iter.remove();
                 break;
             }
 		}

src/java/org/jivesoftware/openfire/webdav/WebDAVLiteServlet.java

@@ -145,7 +145,6 @@ public class WebDAVLiteServlet extends HttpServlet {
     private Boolean isAuthorized(HttpServletRequest request, HttpServletResponse response,
                                  String service, String room) throws ServletException, IOException {
         String auth = request.getHeader("Authorization");
-        JID jid;
         try {
             if (auth == null || !request.getAuthType().equals(HttpServletRequest.BASIC_AUTH)) {
                 throw new Exception("No authorization or improper authorization provided.");
@@ -157,8 +156,8 @@ public class WebDAVLiteServlet extends HttpServlet {
             if (!username.contains("@")) {
                 throw new Exception("Not a valid JID.");
             }
-            jid = new JID(username);
-            XMPPServer.getInstance().getMultiUserChatManager().getMultiUserChatService(service).getChatRoom(room).getOccupantsByBareJID(jid.toBareJID());
+            final JID bareJID = new JID(new JID(username).toBareJID());
+            XMPPServer.getInstance().getMultiUserChatManager().getMultiUserChatService(service).getChatRoom(room).getOccupantsByBareJID(bareJID);
             return true;
         }
         catch (Exception e) {

src/java/org/jivesoftware/util/cache/DummyExternalizableUtil.java

@@ -74,7 +74,7 @@ public class DummyExternalizableUtil implements ExternalizableUtilStrategy {
      * @return a Map of Long key/Integer value pairs.
      * @throws IOException if an error occurs.
      */
-    public Map readLongIntMap(DataInput in) throws IOException {
+    public Map<Long, Integer> readLongIntMap(DataInput in) throws IOException {
         // Do nothing
         return Collections.emptyMap();
     }
@@ -87,7 +87,7 @@ public class DummyExternalizableUtil implements ExternalizableUtilStrategy {
      * @param stringList the List of Strings.
      * @throws IOException if an error occurs.
      */
-    public void writeStringList(DataOutput out, List stringList) throws IOException {
+    public void writeStringList(DataOutput out, List<String> stringList) throws IOException {
         // Do nothing
     }
 
@@ -206,11 +206,11 @@ public class DummyExternalizableUtil implements ExternalizableUtilStrategy {
         return 0;
     }
 
-    public void writeSerializableMap(DataOutput out, Map<String, ? extends Serializable> map) throws IOException {
+    public void writeSerializableMap(DataOutput out, Map<? extends Serializable, ? extends Serializable> map) throws IOException {
         // Do nothing
     }
 
-    public int readSerializableMap(DataInput in, Map<String, ? extends Serializable> map, ClassLoader loader)
+    public int readSerializableMap(DataInput in, Map<? extends Serializable, ? extends Serializable> map, ClassLoader loader)
             throws IOException {
         // Do nothing
         return 0;

src/java/org/jivesoftware/util/cache/ExternalizableUtil.java

@@ -124,7 +124,7 @@ public class ExternalizableUtil {
      * @return a Map of Long key/Integer value pairs.
      * @throws IOException if an error occurs.
      */
-    public Map readLongIntMap(DataInput in) throws IOException {
+    public Map<Long, Integer> readLongIntMap(DataInput in) throws IOException {
         return strategy.readLongIntMap(in);
     }
 
@@ -136,7 +136,7 @@ public class ExternalizableUtil {
      * @param stringList the List of Strings.
      * @throws IOException if an error occurs.
      */
-    public void writeStringList(DataOutput out, List stringList) throws IOException {
+    public void writeStringList(DataOutput out, List<String> stringList) throws IOException {
         strategy.writeStringList(out, stringList);
     }
 
@@ -291,14 +291,14 @@ public class ExternalizableUtil {
     }
 
     /**
-     * Writes a Map of String key and value pairs. This method handles the
+     * Writes a Map of Serializable key and value pairs. This method handles the
      * case when the Map is <tt>null</tt>.
      *
      * @param out       the output stream.
-     * @param map       the Map of String key and Externalizable value pairs.
+     * @param map       the Map of Serializable key and value pairs.
      * @throws java.io.IOException if an error occurs.
      */
-    public void writeSerializableMap(DataOutput out, Map<String, ? extends Serializable> map) throws IOException {
+    public void writeSerializableMap(DataOutput out, Map<? extends Serializable, ? extends Serializable> map) throws IOException {
         strategy.writeSerializableMap(out, map);
     }
     
@@ -317,16 +317,16 @@ public class ExternalizableUtil {
     }
 
     /**
-     * Reads a Map of String key and value pairs. This method will return
+     * Reads a Map of Serializable key and value pairs. This method will return
      * <tt>null</tt> if the Map written to the stream was <tt>null</tt>.
      *
      * @param in the input stream.
-     * @param map a Map of String key and Serializable value pairs.
+     * @param map a Map of Serializable key and value pairs.
      * @param loader class loader to use to build elements inside of the serialized collection.
      * @throws IOException if an error occurs.
      * @return the number of elements added to the collection.
      */
-    public int readSerializableMap(DataInput in, Map<String, ? extends Serializable> map, ClassLoader loader) throws IOException {
+    public int readSerializableMap(DataInput in, Map<? extends Serializable, ? extends Serializable> map, ClassLoader loader) throws IOException {
         return strategy.readSerializableMap(in, map, loader);
     }
 

src/java/org/jivesoftware/util/cache/ExternalizableUtilStrategy.java

@@ -72,7 +72,7 @@ public interface ExternalizableUtilStrategy {
      * @return a Map of Long key/Integer value pairs.
      * @throws IOException if an error occurs.
      */
-    Map readLongIntMap(DataInput in) throws IOException;
+    Map<Long, Integer> readLongIntMap(DataInput in) throws IOException;
 
     /**
      * Writes a List of Strings. This method handles the case when the List is
@@ -82,7 +82,7 @@ public interface ExternalizableUtilStrategy {
      * @param stringList the List of Strings.
      * @throws IOException if an error occurs.
      */
-    void writeStringList(DataOutput out, List stringList) throws IOException;
+    void writeStringList(DataOutput out, List<String> stringList) throws IOException;
 
     /**
      * Reads a List of Strings. This method will return <tt>null</tt> if the List
@@ -148,9 +148,9 @@ public interface ExternalizableUtilStrategy {
 
     int readExternalizableMap(DataInput in, Map<String, ? extends Externalizable> map, ClassLoader loader) throws IOException;
 
-    void writeSerializableMap(DataOutput out, Map<String, ? extends Serializable> map) throws IOException;
+    void writeSerializableMap(DataOutput out, Map<? extends Serializable, ? extends Serializable> map) throws IOException;
 
-    int readSerializableMap(DataInput in, Map<String, ? extends Serializable> map, ClassLoader loader) throws IOException;
+    int readSerializableMap(DataInput in, Map<? extends Serializable, ? extends Serializable> map, ClassLoader loader) throws IOException;
     
     void writeStringsMap(DataOutput out, Map<String, Set<String>> map)  throws IOException;
 

src/plugins/clustering/src/java/com/jivesoftware/util/cache/CoherenceExternalizableUtil.java

@@ -320,7 +320,7 @@ public class CoherenceExternalizableUtil implements ExternalizableUtilStrategy {
         ExternalizableHelper.writeMap(out, map);
     }
 
-    public void writeSerializableMap(DataOutput out, Map<String, ? extends Serializable> map) throws IOException {
+    public void writeSerializableMap(DataOutput out, Map<? extends Serializable, ? extends Serializable> map) throws IOException {
         ExternalizableHelper.writeMap(out, map);
     }
     
@@ -328,7 +328,7 @@ public class CoherenceExternalizableUtil implements ExternalizableUtilStrategy {
         return ExternalizableHelper.readMap(in, map, loader);
     }
 
-    public int readSerializableMap(DataInput in, Map<String, ? extends Serializable> map, ClassLoader loader) throws IOException {
+    public int readSerializableMap(DataInput in, Map<? extends Serializable, ? extends Serializable> map, ClassLoader loader) throws IOException {
         return ExternalizableHelper.readMap(in, map, loader);
     }