commit 1523f3357b80cb4c3f150ac413e8d6a659fe6600
author Marija Savtchouk <mariasavtchouk@google.com> Fri Oct 01 15:23:39 2021 +0100
committer Marija Savtchouk <mariasavtchouk@google.com> Mon Oct 04 11:28:51 2021 +0000
tree 133618fa6b777f3174baae4a6d849801779c6ed1
parent 88ab2404c2ae42ad8c30546b4565bb66b4cb1bf8

Do not check if user can read all refs with -b option

Currently, with -b option, ListRefsfirst first checks if all refs are
visible to the user [1].
This is done by checking permission for each ref pattern that has 'read' permission block
block in project.config[2][3]
Then 'read' permissions for each of the requested refs are checked[4]

This is problematic if project.config has a large number of 'read' blocks.
Since it is expected that the number of branches in the request is small, we can
skip the check that all refs are visible and only check requested branches.

[1]https://gerrit.googlesource.com/gerrit/+/refs/heads/master/java/com/google/gerrit/server/restapi/project/ListProjects.java#586
[2]https://gerrit.googlesource.com/gerrit/+/refs/heads/master/java/com/google/gerrit/server/permissions/ProjectControl.java#442
[3]https://gerrit.googlesource.com/gerrit/+/refs/heads/master/java/com/google/gerrit/server/permissions/ProjectControl.java#293
[4]https://gerrit.googlesource.com/gerrit/+/refs/heads/master/java/com/google/gerrit/server/restapi/project/ListProjects.java#723

Google-bug-Id: b/201425838
Change-Id: Ideb4cb41f106a2b1a58da777371d687dc1a5c65e

java/com/google/gerrit/server/restapi/project/ListProjects.java

diff --git a/java/com/google/gerrit/server/restapi/project/ListProjects.java b/java/com/google/gerrit/server/restapi/project/ListProjects.java
index 3ef9c8f..4d8005b 100644
--- a/java/com/google/gerrit/server/restapi/project/ListProjects.java
+++ b/java/com/google/gerrit/server/restapi/project/ListProjects.java
@@ -578,18 +578,12 @@
     }
   }
 
-  private List<Ref> retrieveBranchRefs(ProjectState e, Repository git)
-      throws PermissionBackendException {
-    boolean canReadAllRefs = e.statePermitsRead();
-    if (canReadAllRefs) {
-      try {
-        permissionBackend.user(currentUser).project(e.getNameKey()).check(ProjectPermission.READ);
-      } catch (AuthException exp) {
-        canReadAllRefs = false;
-      }
+  private List<Ref> retrieveBranchRefs(ProjectState e, Repository git) {
+    if (!e.statePermitsRead()) {
+      return ImmutableList.of();
     }
 
-    return getBranchRefs(e.getNameKey(), canReadAllRefs, git);
+    return getBranchRefs(e.getNameKey(), git);
   }
 
   private void addParentProjectInfo(
@@ -709,16 +703,13 @@
     stdout.flush();
   }
 
-  private List<Ref> getBranchRefs(
-      Project.NameKey projectName, boolean canReadAllRefs, Repository git) {
+  private List<Ref> getBranchRefs(Project.NameKey projectName, Repository git) {
     Ref[] result = new Ref[showBranch.size()];
     try {
       PermissionBackend.ForProject perm = permissionBackend.user(currentUser).project(projectName);
       for (int i = 0; i < showBranch.size(); i++) {
         Ref ref = git.findRef(showBranch.get(i));
-        if (all && canReadAllRefs) {
-          result[i] = ref;
-        } else if (ref != null && ref.getObjectId() != null) {
+        if (ref != null && ref.getObjectId() != null) {
           try {
             perm.ref(ref.getLeaf().getName()).check(RefPermission.READ);
             result[i] = ref;