opendev/gerrit
Code Issues Proposed changes

AccountResolver: Fix StringIndexOutOfBoundsException

Browse Source 
If a change query is done for an account with the 'Full Name <email>'
format, Gerrit first finds accounts that match the email. If there are
multiple accounts that match the email, Gerrit only returns the matches
where also the full name matches. This logic to match on the full name
run into a StringIndexOutOfBoundsException when the input string was an
email surrounded by '<' and '>', e.g. "<foo.bar@example.com>". Such an
input string is deteced as 'Full Name <email>' format but then parsing
the full name in front of '<' failed because it is not present in the
input string.

Stacktrace:
java.lang.StringIndexOutOfBoundsException: String index out of range: -1
  at java.lang.String.substring(String.java:1969)
  at com.google.gerrit.server.account.AccountResolver$ByNameAndEmail.search(AccountResolver.java:349)
  at com.google.gerrit.server.account.AccountResolver$ByNameAndEmail.search(AccountResolver.java:328)
  at com.google.gerrit.server.account.AccountResolver$Searcher.trySearch(AccountResolver.java:227)
  at com.google.gerrit.server.account.AccountResolver.searchImpl(AccountResolver.java:595)
  at com.google.gerrit.server.account.AccountResolver.resolve(AccountResolver.java:519)
  at com.google.gerrit.server.query.change.ChangeQueryBuilder.parseAccount(ChangeQueryBuilder.java:1394)
  at com.google.gerrit.server.query.change.ChangeQueryBuilder.ownerDefaultField(ChangeQueryBuilder.java:1034)
  at com.google.gerrit.server.query.change.ChangeQueryBuilder.defaultField(ChangeQueryBuilder.java:1320)
  at com.google.gerrit.index.query.QueryBuilder.toPredicate(QueryBuilder.java:258)
  at com.google.gerrit.index.query.QueryBuilder.children(QueryBuilder.java:331)
  at com.google.gerrit.index.query.QueryBuilder.toPredicate(QueryBuilder.java:251)
  at com.google.gerrit.index.query.QueryBuilder.parse(QueryBuilder.java:224)
  at com.google.gerrit.index.query.QueryBuilder.parse(QueryBuilder.java:243)
  at com.google.gerrit.server.restapi.change.QueryChanges.query(QueryChanges.java:166)
  at com.google.gerrit.server.restapi.change.QueryChanges.apply(QueryChanges.java:130)
  at com.google.gerrit.server.restapi.change.QueryChanges.apply(QueryChanges.java:45)
  ...

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Id03b16fedd6b31ac2666d165a0627a46c163f3c7
This commit is contained in:
Edwin Kempin
2020-02-18 10:06:51 +01:00
parent d31ad03da9
commit e9f5e658ae
5 changed files with 130 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java
View File

@@ -25,8 +25,10 @@ import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.UseClockStep;
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.TopLevelResource;
@@ -40,6 +42,7 @@ import org.junit.Test;
@NoHttpd
public class QueryChangeIT extends AbstractDaemonTest {
@Inject private AccountOperations accountOperations;
@Inject private ProjectOperations projectOperations;
@Inject private Provider<QueryChanges> queryChangesProvider;
@@ -195,6 +198,33 @@ public class QueryChangeIT extends AbstractDaemonTest {
}
}
@Test
public void queryByFullNameEmailFormatWithEmptyFullNameWhenEmailMatchesSeveralAccounts()
throws Exception {
// Create 2 accounts with the same preferred email (both account must have no external ID for
// the email because otherwise the account with the external ID takes precedence).
String email = "foo.bar@example.com";
Account.Id account1 = accountOperations.newAccount().create();
accountOperations
.account(account1)
.forInvalidation()
.preferredEmailWithoutExternalId(email)
.invalidate();
Account.Id account2 = accountOperations.newAccount().create();
accountOperations
.account(account2)
.forInvalidation()
.preferredEmailWithoutExternalId(email)
.invalidate();
// Search with "Full Name <email>" format, but without full name. Both created accounts match
// the email. In this case Gerrit falls back to match on the full name. Check that this logic
// doesn't fail if the full name in the input string is not present.
QueryChanges queryChanges = queryChangesProvider.get();
queryChanges.addQuery("<" + email + ">");
assertThat(queryChanges.apply(TopLevelResource.INSTANCE).statusCode()).isEqualTo(SC_OK);
}
private static void assertNoChangeHasMoreChangesSet(List<ChangeInfo> results) {
for (ChangeInfo info : results) {
assertThat(info._moreChanges).isNull();