Merge branch 'stable-3.7' into stable-3.8

* stable-3.7:
  Reset pageResultSize when PaginationType NONE back-fill results
  Add test paginationType NONE to query only if there are more results
  Fix paginationType NONE to run further queries only if needed
  Run tests in FakeQueryChangesTest for paginationType NONE
  Fix detection of invalid SSH key algorithm
  Demonstrate SshKeyUtil fails to validate invalid SSH keys
  Move comment of PaginatingSource.read()
  Fix visibility filter for changes when paginationType NONE
  Add tests for pagination to back fill the results
  Allow plugins to use PredicateArgs

Release-Notes: skip
Change-Id: I80dcc0e671fe46e9ca98c99f09bd6503ff5d6fab

14 files changed, 262 insertions, 73 deletions

diff --git a/java/com/google/gerrit/index/QueryOptions.java b/java/com/google/gerrit/index/QueryOptions.java
index 29ab6d0119..40677a18da 100644
--- a/java/com/google/gerrit/index/QueryOptions.java
+++ b/java/com/google/gerrit/index/QueryOptions.java
@@ -128,4 +128,8 @@ public abstract class QueryOptions {
         limit(),
         filter.apply(this));
   }
+
+  public int getLimitBasedOnPaginationType() {
+    return PaginationType.NONE == config().paginationType() ? limit() : pageSize();
+  }
 }
diff --git a/java/com/google/gerrit/index/query/IndexedQuery.java b/java/com/google/gerrit/index/query/IndexedQuery.java
index ee25ef96eb..cb98c06a67 100644
--- a/java/com/google/gerrit/index/query/IndexedQuery.java
+++ b/java/com/google/gerrit/index/query/IndexedQuery.java
@@ -87,6 +87,12 @@ public class IndexedQuery<I, T> extends Predicate<T> implements DataSource<T>, P
   }
 
   @Override
+  public ResultSet<T> restart(int start) {
+    opts = opts.withStart(start);
+    return search();
+  }
+
+  @Override
   public ResultSet<T> restart(int start, int pageSize) {
     opts = opts.withStart(start).withPageSize(pageSize);
     return search();
diff --git a/java/com/google/gerrit/index/query/Paginated.java b/java/com/google/gerrit/index/query/Paginated.java
index 552199093b..1065019c12 100644
--- a/java/com/google/gerrit/index/query/Paginated.java
+++ b/java/com/google/gerrit/index/query/Paginated.java
@@ -19,6 +19,8 @@ import com.google.gerrit.index.QueryOptions;
 public interface Paginated<T> {
   QueryOptions getOptions();
 
+  ResultSet<T> restart(int start);
+
   ResultSet<T> restart(int start, int pageSize);
 
   ResultSet<T> restart(Object searchAfter, int pageSize);
diff --git a/java/com/google/gerrit/index/query/PaginatingSource.java b/java/com/google/gerrit/index/query/PaginatingSource.java
index 98a0ed33f8..8a2d94e719 100644
--- a/java/com/google/gerrit/index/query/PaginatingSource.java
+++ b/java/com/google/gerrit/index/query/PaginatingSource.java
@@ -63,42 +63,55 @@ public class PaginatingSource<T> implements DataSource<T> {
             pageResultSize++;
           }
 
-          if (last != null
-              && source instanceof Paginated
-              // TODO: this fix is only for the stable branches and the real refactoring would be to
-              // restore the logic
-              // for the filtering in AndSource (L58 - 64) as per
-              // https://gerrit-review.googlesource.com/c/gerrit/+/345634/9
-              && !indexConfig.paginationType().equals(PaginationType.NONE)) {
+          if (last != null && source instanceof Paginated) {
             // Restart source and continue if we have not filled the
             // full limit the caller wants.
-            //
             @SuppressWarnings("unchecked")
             Paginated<T> p = (Paginated<T>) source;
             QueryOptions opts = p.getOptions();
             final int limit = opts.limit();
-            int pageSize = opts.pageSize();
-            int pageSizeMultiplier = opts.pageSizeMultiplier();
-            Object searchAfter = resultSet.searchAfter();
-            int nextStart = pageResultSize;
-            while (pageResultSize == pageSize && r.size() <= limit) { // get 1 more than the limit
-              pageSize = getNextPageSize(pageSize, pageSizeMultiplier);
-              ResultSet<T> next =
-                  indexConfig.paginationType().equals(PaginationType.SEARCH_AFTER)
-                      ? p.restart(searchAfter, pageSize)
-                      : p.restart(nextStart, pageSize);
-              pageResultSize = 0;
-              for (T data : buffer(next)) {
-                if (match(data)) {
-                  r.add(data);
+
+            // TODO: this fix is only for the stable branches and the real refactoring would be to
+            // restore the logic
+            // for the filtering in AndSource (L58 - 64) as per
+            // https://gerrit-review.googlesource.com/c/gerrit/+/345634/9
+            if (!indexConfig.paginationType().equals(PaginationType.NONE)) {
+              int pageSize = opts.pageSize();
+              int pageSizeMultiplier = opts.pageSizeMultiplier();
+              Object searchAfter = resultSet.searchAfter();
+              int nextStart = pageResultSize;
+              while (pageResultSize == pageSize && r.size() <= limit) { // get 1 more than the limit
+                pageSize = getNextPageSize(pageSize, pageSizeMultiplier);
+                ResultSet<T> next =
+                    indexConfig.paginationType().equals(PaginationType.SEARCH_AFTER)
+                        ? p.restart(searchAfter, pageSize)
+                        : p.restart(nextStart, pageSize);
+                pageResultSize = 0;
+                for (T data : buffer(next)) {
+                  if (match(data)) {
+                    r.add(data);
+                  }
+                  pageResultSize++;
+                  if (r.size() > limit) {
+                    break;
+                  }
                 }
-                pageResultSize++;
-                if (r.size() > limit) {
-                  break;
+                nextStart += pageResultSize;
+                searchAfter = next.searchAfter();
+              }
+            } else {
+              int nextStart = pageResultSize;
+              while (pageResultSize == limit && r.size() < limit) {
+                ResultSet<T> next = p.restart(nextStart);
+                pageResultSize = 0;
+                for (T data : buffer(next)) {
+                  if (match(data)) {
+                    r.add(data);
+                  }
+                  pageResultSize++;
                 }
+                nextStart += pageResultSize;
               }
-              nextStart += pageResultSize;
-              searchAfter = next.searchAfter();
             }
           }
 
diff --git a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
index e4c6745ded..071c7fbb33 100644
--- a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
+++ b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
@@ -150,10 +150,14 @@ public abstract class AbstractFakeIndex<K, V, D> implements Index<K, V> {
                     .findFirst()
                     .orElse(-1)
                 + 1;
-        int toIndex = Math.min(fromIndex + opts.pageSize(), valueList.size());
+        int toIndex = Math.min(fromIndex + opts.getLimitBasedOnPaginationType(), valueList.size());
         results = valueList.subList(fromIndex, toIndex);
       } else {
-        results = valueStream.skip(opts.start()).limit(opts.pageSize()).collect(toImmutableList());
+        results =
+            valueStream
+                .skip(opts.start())
+                .limit(opts.getLimitBasedOnPaginationType())
+                .collect(toImmutableList());
       }
       queryCount++;
       resultsSizes.add(results.size());
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index 0d90f5d88c..57ef441e57 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -400,11 +400,11 @@ public class LuceneChangeIndex implements ChangeIndex {
       IndexSearcher[] searchers = new IndexSearcher[indexes.size()];
       Map<ChangeSubIndex, ScoreDoc> searchAfterBySubIndex = new HashMap<>();
       try {
-        int realPageSize = opts.start() + opts.pageSize();
-        if (Integer.MAX_VALUE - opts.pageSize() < opts.start()) {
-          realPageSize = Integer.MAX_VALUE;
+        int pageLimit = AbstractLuceneIndex.getLimitBasedOnPaginationType(opts, opts.pageSize());
+        int queryLimit = opts.start() + pageLimit;
+        if (Integer.MAX_VALUE - pageLimit < opts.start()) {
+          queryLimit = Integer.MAX_VALUE;
         }
-        int queryLimit = AbstractLuceneIndex.getLimitBasedOnPaginationType(opts, realPageSize);
         List<TopFieldDocs> hits = new ArrayList<>();
         int searchAfterHitsCount = 0;
         for (int i = 0; i < indexes.size(); i++) {
@@ -412,7 +412,7 @@ public class LuceneChangeIndex implements ChangeIndex {
           searchers[i] = subIndex.acquire();
           if (isSearchAfterPagination) {
             ScoreDoc searchAfter = getSearchAfter(subIndex);
-            int maxRemainingHits = realPageSize - searchAfterHitsCount;
+            int maxRemainingHits = queryLimit - searchAfterHitsCount;
             if (maxRemainingHits > 0) {
               TopFieldDocs subIndexHits =
                   searchers[i].searchAfter(
diff --git a/java/com/google/gerrit/server/query/change/PredicateArgs.java b/java/com/google/gerrit/server/query/change/PredicateArgs.java
index ebe4390f08..02d2ca6951 100644
--- a/java/com/google/gerrit/server/query/change/PredicateArgs.java
+++ b/java/com/google/gerrit/server/query/change/PredicateArgs.java
@@ -71,7 +71,7 @@ public class PredicateArgs {
    *
    * @param args arguments to be parsed
    */
-  PredicateArgs(String args) throws QueryParseException {
+  public PredicateArgs(String args) throws QueryParseException {
     positional = new ArrayList<>();
     keyValue = new HashMap<>();
 
diff --git a/java/com/google/gerrit/sshd/InvalidKeyAlgorithmException.java b/java/com/google/gerrit/sshd/InvalidKeyAlgorithmException.java
new file mode 100644
index 0000000000..5f09658ba2
--- /dev/null
+++ b/java/com/google/gerrit/sshd/InvalidKeyAlgorithmException.java
@@ -0,0 +1,44 @@
+// Copyright (C) 2024 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.sshd;
+
+import java.security.PublicKey;
+import java.security.spec.InvalidKeySpecException;
+
+public class InvalidKeyAlgorithmException extends InvalidKeySpecException {
+  private final String invalidKeyAlgo;
+  private final String expectedKeyAlgo;
+  private final PublicKey publicKey;
+
+  public InvalidKeyAlgorithmException(
+      String invalidKeyAlgo, String expectedKeyAlgo, PublicKey publicKey) {
+    super("Key algorithm mismatch: expected " + expectedKeyAlgo + " but got " + invalidKeyAlgo);
+    this.invalidKeyAlgo = invalidKeyAlgo;
+    this.expectedKeyAlgo = expectedKeyAlgo;
+    this.publicKey = publicKey;
+  }
+
+  public String getInvalidKeyAlgo() {
+    return invalidKeyAlgo;
+  }
+
+  public String getExpectedKeyAlgo() {
+    return expectedKeyAlgo;
+  }
+
+  public PublicKey getPublicKey() {
+    return publicKey;
+  }
+}
diff --git a/java/com/google/gerrit/sshd/SshKeyCacheImpl.java b/java/com/google/gerrit/sshd/SshKeyCacheImpl.java
index 628a0503c0..58e331bb65 100644
--- a/java/com/google/gerrit/sshd/SshKeyCacheImpl.java
+++ b/java/com/google/gerrit/sshd/SshKeyCacheImpl.java
@@ -19,6 +19,7 @@ import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USE
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
 import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.exceptions.InvalidSshKeyException;
 import com.google.gerrit.server.account.AccountSshKey;
 import com.google.gerrit.server.account.VersionedAuthorizedKeys;
 import com.google.gerrit.server.account.externalids.ExternalId;
@@ -144,7 +145,16 @@ public class SshKeyCacheImpl implements SshKeyCache {
         // to do with the key object, and instead we must abort this load.
         //
         throw e;
-      } catch (Exception e) {
+      } catch (InvalidKeyAlgorithmException e) {
+        logger.atWarning().withCause(e).log(
+            "SSH key %d of account %s has an invalid algorithm %s: fixing the algorithm to %s",
+            k.seq(), k.accountId(), e.getInvalidKeyAlgo(), e.getExpectedKeyAlgo());
+        if (fixKeyAlgorithm(k, e.getExpectedKeyAlgo())) {
+          kl.add(new SshKeyCacheEntry(k.accountId(), e.getPublicKey()));
+        } else {
+          markInvalid(k);
+        }
+      } catch (Throwable e) {
         markInvalid(k);
       }
     }
@@ -158,5 +168,20 @@ public class SshKeyCacheImpl implements SshKeyCache {
             "Failed to mark SSH key %d of account %s invalid", k.seq(), k.accountId());
       }
     }
+
+    private boolean fixKeyAlgorithm(AccountSshKey k, String keyAlgo) {
+      try {
+        logger.atInfo().log(
+            "Fixing SSH key %d of account %s algorithm to %s", k.seq(), k.accountId(), keyAlgo);
+        authorizedKeys.deleteKey(k.accountId(), k.seq());
+        String sshKey = k.sshPublicKey();
+        authorizedKeys.addKey(k.accountId(), keyAlgo + sshKey.substring(sshKey.indexOf(' ')));
+        return true;
+      } catch (IOException | ConfigInvalidException | InvalidSshKeyException e) {
+        logger.atSevere().withCause(e).log(
+            "Failed to fix SSH key %d of account %s with algo %s", k.seq(), k.accountId(), keyAlgo);
+        return false;
+      }
+    }
   }
 }
diff --git a/java/com/google/gerrit/sshd/SshUtil.java b/java/com/google/gerrit/sshd/SshUtil.java
index abbd81d83a..29d0e90f9f 100644
--- a/java/com/google/gerrit/sshd/SshUtil.java
+++ b/java/com/google/gerrit/sshd/SshUtil.java
@@ -57,7 +57,12 @@ public class SshUtil {
         throw new InvalidKeySpecException("No key string");
       }
       final byte[] bin = BaseEncoding.base64().decode(s);
-      return new ByteArrayBuffer(bin).getRawPublicKey();
+      String publicKeyAlgo = new ByteArrayBuffer(bin).getString();
+      PublicKey publicKey = new ByteArrayBuffer(bin).getRawPublicKey();
+      if (!key.algorithm().equals(publicKeyAlgo)) {
+        throw new InvalidKeyAlgorithmException(key.algorithm(), publicKeyAlgo, publicKey);
+      }
+      return publicKey;
     } catch (RuntimeException | SshException e) {
       throw new InvalidKeySpecException("Cannot parse key", e);
     }
diff --git a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
index 72fc6d270d..7641544d56 100644
--- a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
@@ -25,6 +25,8 @@ import static org.junit.Assume.assumeFalse;
 import static org.junit.Assume.assumeTrue;
 
 import com.google.gerrit.acceptance.UseClockStep;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Change;
 import com.google.gerrit.entities.Permission;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.extensions.common.ChangeInfo;
@@ -84,11 +86,48 @@ public abstract class FakeQueryChangesTest extends AbstractQueryChangesTest {
     AbstractFakeIndex<?, ?, ?> idx =
         (AbstractFakeIndex<?, ?, ?>) changeIndexCollection.getSearchIndex();
     newQuery("status:new").withLimit(5).get();
+    // Since the limit of the query (i.e. 5) is more than the total number of changes (i.e. 4),
+    // only 1 index search is expected.
     assertThatSearchQueryWasNotPaginated(idx.getQueryCount());
   }
 
   @Test
   @UseClockStep
+  public void queryRightNumberOfTimes() throws Exception {
+    TestRepository<Repository> repo = createAndOpenProject("repo");
+    Account.Id user2 =
+        accountManager.authenticate(authRequestFactory.createForUser("anotheruser")).getAccountId();
+
+    // create 1 visible change
+    Change visibleChange1 = insert("repo", newChangeWithStatus(repo, Change.Status.NEW));
+
+    // create 4 private changes
+    Change invisibleChange2 = insert("repo", newChangeWithStatus(repo, Change.Status.NEW), user2);
+    Change invisibleChange3 = insert("repo", newChangeWithStatus(repo, Change.Status.NEW), user2);
+    Change invisibleChange4 = insert("repo", newChangeWithStatus(repo, Change.Status.NEW), user2);
+    Change invisibleChange5 = insert("repo", newChangeWithStatus(repo, Change.Status.NEW), user2);
+    gApi.changes().id(invisibleChange2.getChangeId()).setPrivate(true, null);
+    gApi.changes().id(invisibleChange3.getChangeId()).setPrivate(true, null);
+    gApi.changes().id(invisibleChange4.getChangeId()).setPrivate(true, null);
+    gApi.changes().id(invisibleChange5.getChangeId()).setPrivate(true, null);
+
+    AbstractFakeIndex<?, ?, ?> idx =
+        (AbstractFakeIndex<?, ?, ?>) changeIndexCollection.getSearchIndex();
+    idx.resetQueryCount();
+    List<ChangeInfo> queryResult = newQuery("status:new").withLimit(2).get();
+    assertThat(queryResult).hasSize(1);
+    assertThat(queryResult.get(0).changeId).isEqualTo(visibleChange1.getKey().get());
+
+    // Since the limit of the query (i.e. 2), 2 index searches are expected in fact:
+    // 1: The first query will return invisibleChange5, invisibleChange4 and invisibleChange3,
+    // 2: Another query is needed to back-fill the limit requested by the user.
+    // even if one result in the second query is skipped because it is not visible,
+    // there are no more results to query.
+    assertThatSearchQueryWasPaginated(idx.getQueryCount(), 2);
+  }
+
+  @Test
+  @UseClockStep
   public void noLimitQueryPaginates() throws Exception {
     assumeFalse(PaginationType.NONE == getCurrentPaginationType());
 
@@ -119,45 +158,15 @@ public abstract class FakeQueryChangesTest extends AbstractQueryChangesTest {
   @UseClockStep
   public void noLimitQueryDoesNotPaginatesWithNonePaginationType() throws Exception {
     assumeTrue(PaginationType.NONE == getCurrentPaginationType());
-    AbstractFakeIndex idx = setupRepoWithFourChanges();
+    AbstractFakeIndex<?, ?, ?> idx = setupRepoWithFourChanges();
     newQuery("status:new").withNoLimit().get();
     assertThatSearchQueryWasNotPaginated(idx.getQueryCount());
   }
 
   @Test
   @UseClockStep
-  public void invisibleChangesNotPaginatedWithNonePaginationType() throws Exception {
-    assumeTrue(PaginationType.NONE == getCurrentPaginationType());
-    AbstractFakeIndex idx = setupRepoWithFourChanges();
-    final int LIMIT = 3;
-
-    projectOperations
-        .project(allProjectsName)
-        .forUpdate()
-        .removeAllAccessSections()
-        .add(allow(Permission.READ).ref("refs/*").group(SystemGroupBackend.REGISTERED_USERS))
-        .update();
-
-    // Set queryLimit to 3
-    projectOperations
-        .project(allProjects)
-        .forUpdate()
-        .add(allowCapability(QUERY_LIMIT).group(ANONYMOUS_USERS).range(0, LIMIT))
-        .update();
-
-    requestContext.setContext(anonymousUserProvider::get);
-    List<ChangeInfo> result = newQuery("status:new").withLimit(LIMIT).get();
-    assertThat(result.size()).isEqualTo(0);
-    assertThatSearchQueryWasNotPaginated(idx.getQueryCount());
-    assertThat(idx.getResultsSizes().get(0)).isEqualTo(LIMIT + 1);
-  }
-
-  @Test
-  @UseClockStep
   public void invisibleChangesPaginatedWithPagination() throws Exception {
-    assumeFalse(PaginationType.NONE == getCurrentPaginationType());
-
-    AbstractFakeIndex idx = setupRepoWithFourChanges();
+    AbstractFakeIndex<?, ?, ?> idx = setupRepoWithFourChanges();
     final int LIMIT = 3;
 
     projectOperations
@@ -199,7 +208,8 @@ public abstract class FakeQueryChangesTest extends AbstractQueryChangesTest {
         .forUpdate()
         .add(allowCapability(QUERY_LIMIT).group(REGISTERED_USERS).range(0, LIMIT))
         .update();
-    AbstractFakeIndex idx = (AbstractFakeIndex) changeIndexCollection.getSearchIndex();
+    AbstractFakeIndex<?, ?, ?> idx =
+        (AbstractFakeIndex<?, ?, ?>) changeIndexCollection.getSearchIndex();
     // 2 index searches are expected. The first index search will run with size 3 (i.e.
     // the configured query-limit+1), and then we will paginate to get the remaining
     // changes with the second index search.
@@ -213,7 +223,7 @@ public abstract class FakeQueryChangesTest extends AbstractQueryChangesTest {
   public void internalQueriesDoNotPaginateWithNonePaginationType() throws Exception {
     assumeTrue(PaginationType.NONE == getCurrentPaginationType());
 
-    AbstractFakeIndex idx = setupRepoWithFourChanges();
+    AbstractFakeIndex<?, ?, ?> idx = setupRepoWithFourChanges();
     // 1 index search is expected since we are not paginating.
     executeQuery("status:new");
     assertThatSearchQueryWasNotPaginated(idx.getQueryCount());
@@ -231,7 +241,7 @@ public abstract class FakeQueryChangesTest extends AbstractQueryChangesTest {
     assertThat(queryCount).isEqualTo(expectedPages);
   }
 
-  private AbstractFakeIndex setupRepoWithFourChanges() throws Exception {
+  private AbstractFakeIndex<?, ?, ?> setupRepoWithFourChanges() throws Exception {
     try (TestRepository<Repository> testRepo = createAndOpenProject("repo")) {
       insert("repo", newChange(testRepo));
       insert("repo", newChange(testRepo));
@@ -246,6 +256,6 @@ public abstract class FakeQueryChangesTest extends AbstractQueryChangesTest {
         .add(allowCapability(QUERY_LIMIT).group(REGISTERED_USERS).range(0, 2))
         .update();
 
-    return (AbstractFakeIndex) changeIndexCollection.getSearchIndex();
+    return (AbstractFakeIndex<?, ?, ?>) changeIndexCollection.getSearchIndex();
   }
 }
diff --git a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
index 7f383f948b..d9a0767df7 100644
--- a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
@@ -20,6 +20,7 @@ import static com.google.gerrit.common.data.GlobalCapability.QUERY_LIMIT;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static com.google.gerrit.testing.GerritJUnit.assertThrows;
 
+import com.google.gerrit.entities.Account;
 import com.google.gerrit.entities.Change;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.server.config.AllProjectsName;
@@ -89,4 +90,27 @@ public abstract class LuceneQueryChangesTest extends AbstractQueryChangesTest {
     Change[] expected = new Change[] {change6, change5, change4, change3, change2, change1};
     assertQuery(newQuery("project:repo").withNoLimit(), expected);
   }
+
+  @Test
+  public void skipChangesNotVisible() throws Exception {
+    // create 1 new change on a repo
+    repo = createAndOpenProject("repo");
+    Change visibleChange = insert("repo", newChangeWithStatus(repo, Change.Status.NEW));
+    Change[] expected = new Change[] {visibleChange};
+
+    // pagination does not need to restart the datasource, the request is fulfilled
+    assertQuery(newQuery("status:new").withLimit(1), expected);
+
+    // create 2 new private changes
+    Account.Id user2 =
+        accountManager.authenticate(authRequestFactory.createForUser("anotheruser")).getAccountId();
+
+    Change invisibleChange1 = insert("repo", newChangeWithStatus(repo, Change.Status.NEW), user2);
+    Change invisibleChange2 = insert("repo", newChangeWithStatus(repo, Change.Status.NEW), user2);
+    gApi.changes().id(invisibleChange1.getChangeId()).setPrivate(true, null);
+    gApi.changes().id(invisibleChange2.getChangeId()).setPrivate(true, null);
+
+    // pagination should back-fill when the results skipped because of the visibility
+    assertQuery(newQuery("status:new").withLimit(1), expected);
+  }
 }
diff --git a/javatests/com/google/gerrit/sshd/BUILD b/javatests/com/google/gerrit/sshd/BUILD
index 3e11ff22e2..44b9c62a79 100644
--- a/javatests/com/google/gerrit/sshd/BUILD
+++ b/javatests/com/google/gerrit/sshd/BUILD
@@ -4,8 +4,11 @@ junit_tests(
     name = "sshd_tests",
     srcs = glob(["**/*.java"]),
     deps = [
+        "//java/com/google/gerrit/entities",
         "//java/com/google/gerrit/extensions:api",
+        "//java/com/google/gerrit/server",
         "//java/com/google/gerrit/sshd",
+        "//java/com/google/gerrit/testing:gerrit-test-util",
         "//lib/mina:sshd",
         "//lib/truth",
     ],
diff --git a/javatests/com/google/gerrit/sshd/SshUtilTest.java b/javatests/com/google/gerrit/sshd/SshUtilTest.java
new file mode 100644
index 0000000000..1585bc3a97
--- /dev/null
+++ b/javatests/com/google/gerrit/sshd/SshUtilTest.java
@@ -0,0 +1,49 @@
+// Copyright (C) 2024 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.sshd;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.server.account.AccountSshKey;
+import java.security.spec.InvalidKeySpecException;
+import org.junit.Test;
+
+public class SshUtilTest {
+  private static final Account.Id TEST_ACCOUNT_ID = Account.id(1);
+  private static final int TEST_SSHKEY_SEQUENCE = 1;
+  private static final String INVALID_ALGO = "invalid-algo";
+  private static final String VALID_OPENSSH_RSA_KEY =
+      "AAAAB3NzaC1yc2EAAAABIwAAAIEA0R66EoZ7hFp81w9sAJqu34UFyE+w36H/mobUqnT5Lns7PcTOJh3sgMJAlswX2lFAWqvF2gd2PRMpMhbfEU4iq2SfY8x+RDCJ4ZQWESln/587T41BlQjOXzu3W1bqgmtHnRCte3DjyWDvM/fucnUMSwOgP+FVEZCLTrk3thLMWsU=";
+  private static final Object VALID_SSH_RSA_ALGO = "ssh-rsa";
+
+  @Test
+  public void shouldFailParsingOpenSshKeyWithInvalidAlgo() {
+    String sshKeyWithInvalidAlgo = String.format("%s %s", INVALID_ALGO, VALID_OPENSSH_RSA_KEY);
+    AccountSshKey sshKey =
+        AccountSshKey.create(TEST_ACCOUNT_ID, TEST_SSHKEY_SEQUENCE, sshKeyWithInvalidAlgo);
+    assertThrows(InvalidKeySpecException.class, () -> SshUtil.parse(sshKey));
+  }
+
+  @Test
+  public void shouldParseSshKeyWithAlgoMatchingKey() {
+    String sshKeyWithValidKeyAlgo =
+        String.format("%s %s", VALID_SSH_RSA_ALGO, VALID_OPENSSH_RSA_KEY);
+    AccountSshKey sshKey =
+        AccountSshKey.create(TEST_ACCOUNT_ID, TEST_SSHKEY_SEQUENCE, sshKeyWithValidKeyAlgo);
+    assertThat(sshKey).isNotNull();
+  }
+}