From 7dadcbf091db64f302999cccfa74ab1bf512cebe Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Tue, 29 Sep 2015 15:25:42 +0100 Subject: [PATCH] Fixes NPE when username not found and matches by external ID In Ia6561200 we changed to look up users in the cache by the username from both the "gerrit:" and "user:" schemes. However, the username cache only contains keys for the "user:" scheme, so we need to double-check that there is actually a corresponding external ID in the cached AccountState for the actual ID we're looking up. This was not the case in e.g. LuceneQueryChangesTest, causing an NPE. If the requested key does not match exactly, we fall back to fetching the external ID directly from the database. Change-Id: Id3b1b9b76d6ba415abee2051409ff0843caf1c85 --- .../gerrit/server/account/AccountManager.java | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java index d7b93421cb..7de3d64e1e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java @@ -38,7 +38,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; @@ -141,18 +140,15 @@ public class AccountManager { // without having to query the DB every time if (keyScheme.equals(AccountExternalId.SCHEME_GERRIT) || keyScheme.equals(AccountExternalId.SCHEME_USERNAME)) { - String username = keyValue.substring(keyScheme.length()); - Collection externalIds = - byIdCache.getByUsername(username).getExternalIds(); - for (AccountExternalId accountExternalId : externalIds) { - if (accountExternalId.isScheme(keyScheme)) { - return accountExternalId; + AccountState state = byIdCache.getByUsername( + keyValue.substring(keyScheme.length())); + if (state != null) { + for (AccountExternalId accountExternalId : state.getExternalIds()) { + if (accountExternalId.getKey().equals(key)) { + return accountExternalId; + } } } - - log.warn( - "Cannot find account external id for user {} in cache, possibly a stale entry", - username); } return db.accountExternalIds().get(key); }