Limit returned changes for a triplet without Change-Id

When looking for a single change, do not perform a full scan
of all changes on a repository/branch as they would not identify
a single entry but rather limit the result to just 2 elements,
which would be enough to identify the response.

Fix a bug where invoking the REST-API /changes/repo~branch~ without
mentioning the change-id resulted in the full scan of all changes
on a repository/branch causing a high CPU overload and occupying
the request threads for a long time.

Requesting a single change by NOT specifying its change-id is not
a valid use-case, and it should always return a 404, regardless of
how many changes exist on the project/branch.

Bug: Issue 14785
Change-Id: Ief77d199c917fea8ebaa6ecc44a7b4b2c59d6f4e

3 files changed, 26 insertions, 2 deletions

diff --git a/java/com/google/gerrit/server/change/ChangeFinder.java b/java/com/google/gerrit/server/change/ChangeFinder.java
index 71d7ba01e8..ba104d8cca 100644
--- a/java/com/google/gerrit/server/change/ChangeFinder.java
+++ b/java/com/google/gerrit/server/change/ChangeFinder.java
@@ -100,7 +100,9 @@ public class ChangeFinder {
   }
 
   public Optional<ChangeNotes> findOne(String id) {
-    List<ChangeNotes> ctls = find(id);
+    // Limit the maximum number of results to just 2 items for saving CPU cycles
+    // in reading change-notes.
+    List<ChangeNotes> ctls = find(id, 2);
     if (ctls.size() != 1) {
       return Optional.empty();
     }
@@ -114,6 +116,17 @@ public class ChangeFinder {
    * @return possibly-empty list of notes for all matching changes; may or may not be visible.
    */
   public List<ChangeNotes> find(String id) {
+    return find(id, 0);
+  }
+
+  /**
+   * Find at most N changes matching the given identifier.
+   *
+   * @param id change identifier.
+   * @param queryLimit maximum number of changes to be returned
+   * @return possibly-empty list of notes for all matching changes; may or may not be visible.
+   */
+  public List<ChangeNotes> find(String id, int queryLimit) {
     if (id.isEmpty()) {
       return Collections.emptyList();
     }
@@ -141,6 +154,9 @@ public class ChangeFinder {
     // Use the index to search for changes, but don't return any stored fields,
     // to force rereading in case the index is stale.
     InternalChangeQuery query = queryProvider.get().noFields();
+    if (queryLimit > 0) {
+      query.setLimit(queryLimit);
+    }
 
     // Try commit hash
     if (id.matches("^([0-9a-fA-F]{" + ObjectIds.ABBREV_STR_LEN + "," + ObjectIds.STR_LEN + "})$")) {
diff --git a/java/com/google/gerrit/server/restapi/change/ChangesCollection.java b/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
index 95b74f8595..572f704834 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
@@ -82,7 +82,7 @@ public class ChangesCollection implements RestCollection<TopLevelResource, Chang
   @Override
   public ChangeResource parse(TopLevelResource root, IdString id)
       throws RestApiException, PermissionBackendException, IOException {
-    List<ChangeNotes> notes = changeFinder.find(id.encoded());
+    List<ChangeNotes> notes = changeFinder.find(id.encoded(), 2);
     if (notes.isEmpty()) {
       throw new ResourceNotFoundException(id);
     } else if (notes.size() != 1) {
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeIdIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeIdIT.java
index bc52681fdb..06e24ab81b 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeIdIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeIdIT.java
@@ -81,6 +81,14 @@ public class ChangeIdIT extends AbstractDaemonTest {
   }
 
   @Test
+  public void tripletWithoutChangeIdReturnsNotFound() throws Exception {
+    createChange().assertOkStatus();
+    createChange().assertOkStatus();
+    RestResponse res = adminRestSession.get(changeDetail(project.get() + "~master~"));
+    res.assertNotFound();
+  }
+
+  @Test
   public void changeIdReturnsChange() throws Exception {
     PushOneCommit.Result c = createChange();
     RestResponse res = adminRestSession.get(changeDetail(c.getChangeId()));