opendev/gerrit
Code Issues Proposed changes

Try to avoid null group UUIDs in transitive include processing

Browse Source 
One of our production Gerrit instances is seeing null UUIDs in the
group backend code, leading to NPEs from the IncludingGroupMembership
code paths. We presently have no idea how the null UUID is being
added into the group collection.

Guard the internal group listing for an AccountState to avoid
groups that are missing a UUID. Also ensure the queue does not get
initialized with a null entry, and never add a null UUID during
iteration. This should stop the NPE we are seeing in our logs,
but is probably only a bandaid over the real source of the null.

Change-Id: I61543cbf71f62aa42d1188818e1296020497e4a3
This commit is contained in:
Shawn Pearce 2014-02-04 15:18:30 -08:00
parent d1f95aebe3
commit 5ac379ca64
2 changed files with 5 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
View File

@ -167,7 +167,7 @@ public class AccountCacheImpl implements AccountCache {
for (AccountGroupMember g : db.accountGroupMembers().byAccount(who)) {
final AccountGroup.Id groupId = g.getAccountGroupId();
final AccountGroup group = groupCache.get(groupId);
if (group != null) {
if (group != null && group.getGroupUUID() != null) {
internalGroups.add(group.getGroupUUID());
}
}

7
gerrit-server/src/main/java/com/google/gerrit/server/account/IncludingGroupMembership.java
View File

@ -119,11 +119,12 @@ public class IncludingGroupMembership implements GroupMembership {
GroupMembership membership = user.getEffectiveGroups();
Set<AccountGroup.UUID> direct = user.state().getInternalGroups();
Set<AccountGroup.UUID> r = Sets.newHashSet(direct);
List<AccountGroup.UUID> q = Lists.newArrayList(r);
r.remove(null);
List<AccountGroup.UUID> q = Lists.newArrayList(r);
for (AccountGroup.UUID g : membership.intersection(
includeCache.allExternalMembers())) {
if (r.add(g)) {
if (g != null && r.add(g)) {
q.add(g);
}
}
@ -131,7 +132,7 @@ public class IncludingGroupMembership implements GroupMembership {
while (!q.isEmpty()) {
AccountGroup.UUID id = q.remove(q.size() - 1);
for (AccountGroup.UUID g : includeCache.memberIn(id)) {
if (r.add(g)) {
if (g != null && r.add(g)) {
q.add(g);
memberOf.put(g, true);
}