Bug: NullPointerException in org.jivesoftware. openfire.group.ConcurrentGroupMap .includesKey

Hi Openfire Devs!

I have a bug report which is causing some issues with MUC usage in our installation. We’re seeing NullPointerExceptions like the following:

  1. 2017.08.03 15:02:57 org.jivesoftware.openfire.muc.spi.MultiUserChatServiceImpl - Internal server error
  2. java.lang.NullPointerException
  3. at org.jivesoftware.openfire.group.ConcurrentGroupMap.includesKey(ConcurrentGroupMap.java:49)
  4. at org.jivesoftware.openfire.muc.spi.LocalMUCRoom.joinRoom(LocalMUCRoom.java:642)
  5. at org.jivesoftware.openfire.muc.spi.LocalMUCUser.process(LocalMUCUser.java:471)
  6. at org.jivesoftware.openfire.muc.spi.LocalMUCUser.process(LocalMUCUser.java:177)
  7. at org.jivesoftware.openfire.muc.spi.MultiUserChatServiceImpl.processPacket(MultiUserChatServiceImpl.java:366)
  8. at org.jivesoftware.openfire.component.InternalComponentManager$RoutableComponents.process(InternalComponentManager.java:606)

This is caused because (as best as I can figure) ConcurrentGroupMap.includesKey calls iterator.next().isUser(target); but iterator.next() can return NULL, and that isn’t checked for: iterator is created from getGroupsFromKeys() which calls Group.resolveFrom that can return null:

/**

  • Attempt to resolve the given object into a Group.
  • @param proxy Presumably a Group, a Group name, or a JID that represents a Group
  • @return The corresponding group, or null if the proxy cannot be resolved as a group
    */
    public static Group resolveFrom(Object proxy) {

Group result = null;
try {

GroupManager groupManger = GroupManager.getInstance();
if (proxy instanceof JID) {

result = groupManger.getGroup((JID)proxy);
} else if (proxy instanceof String) {

result = groupManger.getGroup((String)proxy);
} else if (proxy instanceof Group) {

result = (Group) proxy;
}

} catch (GroupNotFoundException gnfe) {

// ignore
}

return result;
}

/**

  • Returns the groups that are implied (resolvable) from the keys in the map.
  • @return A Set containing the groups among the keys
    */
    @Override
    public synchronized Set getGroupsFromKeys() {

Set result = new HashSet<>();
for(String groupName : getKnownGroupNamesFromKeys()) {

result.add(Group.resolveFrom(groupName));
}

return result;
}

The fix is a simple check for NULL before using the result of iterator.next at line 48 of ConcurrentGroupMap.java:

while (!found && iterator.hasNext()) {

found = iterator.next().isUser(target); // check for NULL first here
}

Proposed fix:

while (!found && iterator.hasNext()) {

Group next = iterator.next();
if(next != null)

found = next.isUser(target);
}

I can send a pull request for this if that would help

Alternative fix:

/**

  • Returns the groups that are implied (resolvable) from the keys in the map.
  • @return A Set containing the groups among the keys
    */
    @Override
    public synchronized Set getGroupsFromKeys() {

Set result = new HashSet<>();
for(String groupName : getKnownGroupNamesFromKeys()) {

Group resolved = Group.resolveFrom(groupName);
if(resolved != null)

result.add(resolved);
}

return result;
}

Created pull request Fixing NullPointerException caused by NULLs in map by jgitlin-bt · Pull Request #843 · igniterealtime/Openfire · GitHub

Thanks for your contribution Josh! I’ve logged this issue as [OF-1366] NullPointerException in Group lookup - IgniteRealtime JIRA

Thanks Guus! We appreciate it; this issue causes our MUC system to become unavailable for users when it hits us