From 7072e5c2cb89b9126f9078d708c7f60845ba04b8 Mon Sep 17 00:00:00 2001
From: Jacek Centkowski <geminica.programs@gmail.com>
Date: Fri, 4 Mar 2022 11:07:15 +0100
Subject: Ignore '--no-limit' query changes option for anonymous users

Setting 'maxPages' is not enough to limit resources consumed by the
anonymous users as one can still request low page number with unlimited
result set (adding 'no-limit' option to REST API query changes call).

In order to solve it make 'no-limit' not applicable for anonymous users.
Notes:
* one can still configure them to request unlimited results by setting
  'Query Limit' Global Capability to Integer.MAX_VALUE for 'Anonymous
  Users' group
* 'no-limit' option is only a part of query changes API hence accounts,
  groups and projects are not affected by this change

Release-Notes: Ignore '--no-limit' for anonymous users change queries
Bug: Issue 15563
Change-Id: Ic789690ffd2f94f02989c2906fcd75e442df86f8
---
 Documentation/rest-api-changes.txt                 |  4 ++--
 .../gerrit/server/restapi/change/QueryChanges.java |  7 +++++--
 .../gerrit/acceptance/api/change/ChangeIT.java     | 22 +++++++++++++++++++++-
 3 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 2bfb5d5b80..32bfc6b496 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -74,8 +74,8 @@ Queries changes visible to the caller. The
 link:user-search.html#_search_operators[query string] must be provided
 by the `q` parameter. The `n` parameter can be used to limit the
 returned results. The `no-limit` parameter can be used remove the default
-limit on queries and return all results. This might not be supported by
-all index backends.
+limit on queries and return all results (does not apply to anonymous requests).
+This might not be supported by all index backends.
 
 As result a list of link:#change-info[ChangeInfo] entries is returned.
 The change output is sorted by the last update time, most recently
diff --git a/java/com/google/gerrit/server/restapi/change/QueryChanges.java b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
index 3c8157b51e..7df74f817f 100644
--- a/java/com/google/gerrit/server/restapi/change/QueryChanges.java
+++ b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
@@ -27,6 +27,7 @@ import com.google.gerrit.extensions.restapi.TopLevelResource;
 import com.google.gerrit.index.query.QueryParseException;
 import com.google.gerrit.index.query.QueryRequiresAuthException;
 import com.google.gerrit.index.query.QueryResult;
+import com.google.gerrit.server.AnonymousUser;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.DynamicOptions;
 import com.google.gerrit.server.change.ChangeJson;
@@ -95,7 +96,9 @@ public class QueryChanges implements RestReadView<TopLevelResource>, DynamicOpti
     this.start = start;
   }
 
-  @Option(name = "--no-limit", usage = "Return all results, overriding the default limit")
+  @Option(
+      name = "--no-limit",
+      usage = "Return all results, overriding the default limit. Ignored for anonymous users.")
   public void setNoLimit(boolean on) {
     this.noLimit = on;
   }
@@ -168,7 +171,7 @@ public class QueryChanges implements RestReadView<TopLevelResource>, DynamicOpti
     if (start != null) {
       queryProcessor.setStart(start);
     }
-    if (noLimit != null) {
+    if (noLimit != null && !AnonymousUser.class.isAssignableFrom(userProvider.get().getClass())) {
       queryProcessor.setNoLimit(noLimit);
     }
     if (skipVisibility != null) {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 0c30ef5dce..7c504b8c25 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -2618,7 +2618,7 @@ public class ChangeIT extends AbstractDaemonTest {
   }
 
   @Test
-  public void queryChangesNoLimit() throws Exception {
+  public void queryChangesNoLimitRegisteredUser() throws Exception {
     projectOperations
         .allProjectsForUpdate()
         .add(
@@ -2635,6 +2635,26 @@ public class ChangeIT extends AbstractDaemonTest {
     assertThat(resultsWithNoLimit.size()).isAtLeast(3);
   }
 
+  @Test
+  public void queryChangesNoLimitIgnoredForAnonymousUser() throws Exception {
+    int limit = 2;
+    projectOperations
+        .allProjectsForUpdate()
+        .add(
+            allowCapability(GlobalCapability.QUERY_LIMIT)
+                .group(SystemGroupBackend.ANONYMOUS_USERS)
+                .range(0, limit))
+        .update();
+    for (int i = 0; i < 3; i++) {
+      createChange();
+    }
+    requestScopeOperations.setApiUserAnonymous();
+    List<ChangeInfo> resultsWithDefaultLimit = gApi.changes().query().get();
+    List<ChangeInfo> resultsWithNoLimit = gApi.changes().query().withNoLimit().get();
+    assertThat(resultsWithDefaultLimit).hasSize(limit);
+    assertThat(resultsWithNoLimit).hasSize(limit);
+  }
+
   @Test
   public void queryChangesStart() throws Exception {
     PushOneCommit.Result r1 = createChange();
-- 
cgit v1.2.3