Validate the 'start' parameter in all query endpoints
Release-Notes: skip
Google-Bug-Id: b/242968251
Change-Id: I63a7ed9949bb59f3d41aeccf7eb4fdbef535ce8a
diff --git a/java/com/google/gerrit/server/restapi/account/QueryAccounts.java b/java/com/google/gerrit/server/restapi/account/QueryAccounts.java
index c671562..30534b5 100644
--- a/java/com/google/gerrit/server/restapi/account/QueryAccounts.java
+++ b/java/com/google/gerrit/server/restapi/account/QueryAccounts.java
@@ -202,6 +202,9 @@
}
if (start != null) {
+ if (start < 0) {
+ throw new BadRequestException("'start' parameter cannot be less than zero");
+ }
queryProcessor.setStart(start);
}
diff --git a/java/com/google/gerrit/server/restapi/change/QueryChanges.java b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
index 2c15bc9..6ce4b39 100644
--- a/java/com/google/gerrit/server/restapi/change/QueryChanges.java
+++ b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
@@ -159,7 +159,8 @@
return Response.ok(out.size() == 1 ? out.get(0) : out);
}
- private List<List<ChangeInfo>> query() throws QueryParseException, PermissionBackendException {
+ private List<List<ChangeInfo>> query()
+ throws BadRequestException, QueryParseException, PermissionBackendException {
ChangeQueryProcessor queryProcessor = queryProcessorProvider.get();
if (queryProcessor.isDisabled()) {
throw new QueryParseException("query disabled");
@@ -169,6 +170,9 @@
queryProcessor.setUserProvidedLimit(limit);
}
if (start != null) {
+ if (start < 0) {
+ throw new BadRequestException("'start' parameter cannot be less than zero");
+ }
queryProcessor.setStart(start);
}
if (noLimit != null && !AnonymousUser.class.isAssignableFrom(userProvider.get().getClass())) {
diff --git a/java/com/google/gerrit/server/restapi/group/QueryGroups.java b/java/com/google/gerrit/server/restapi/group/QueryGroups.java
index befccfe..fed2302 100644
--- a/java/com/google/gerrit/server/restapi/group/QueryGroups.java
+++ b/java/com/google/gerrit/server/restapi/group/QueryGroups.java
@@ -107,6 +107,10 @@
throw new MethodNotAllowedException("query disabled");
}
+ if (start < 0) {
+ throw new BadRequestException("'start' parameter cannot be less than zero");
+ }
+
if (start != 0) {
queryProcessor.setStart(start);
}
diff --git a/java/com/google/gerrit/server/restapi/project/QueryProjects.java b/java/com/google/gerrit/server/restapi/project/QueryProjects.java
index a9d818d..b219085 100644
--- a/java/com/google/gerrit/server/restapi/project/QueryProjects.java
+++ b/java/com/google/gerrit/server/restapi/project/QueryProjects.java
@@ -106,6 +106,10 @@
ProjectQueryProcessor queryProcessor = queryProcessorProvider.get();
+ if (start < 0) {
+ throw new BadRequestException("'start' parameter cannot be less than zero");
+ }
+
if (start != 0) {
queryProcessor.setStart(start);
}
diff --git a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
index bf1b302..322a934 100644
--- a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
+++ b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
@@ -447,6 +447,12 @@
}
@Test
+ public void withStartCannotBeLessThanZero() throws Exception {
+ assertFailingQuery(
+ newQuery("self").withStart(-1), "'start' parameter cannot be less than zero");
+ }
+
+ @Test
public void sortedByFullname() throws Exception {
String appendix = name("name");
@@ -849,7 +855,7 @@
return accounts.stream().map(a -> a._accountId).collect(toList());
}
- protected void assertFailingQuery(String query, String expectedMessage) throws Exception {
+ protected void assertFailingQuery(QueryRequest query, String expectedMessage) throws Exception {
try {
assertQuery(query);
fail("expected BadRequestException for query '" + query + "'");
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 5d587f1..ab2b9b6 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -1477,6 +1477,12 @@
}
@Test
+ public void startCannotBeLessThanZero() throws Exception {
+ assertFailingQuery(
+ newQuery("owner:self").withStart(-1), "'start' parameter cannot be less than zero");
+ }
+
+ @Test
public void startWithLimit() throws Exception {
TestRepository<Repo> repo = createProject("repo");
List<Change> changes = new ArrayList<>();
@@ -4279,6 +4285,15 @@
assertFailingQuery(query, null);
}
+ protected void assertFailingQuery(QueryRequest query, String expectedMessage) throws Exception {
+ try {
+ assertQuery(query);
+ fail("expected BadRequestException for query '" + query + "'");
+ } catch (BadRequestException e) {
+ assertThat(e.getMessage()).isEqualTo(expectedMessage);
+ }
+ }
+
protected void assertFailingQuery(String query, @Nullable String expectedMessage)
throws Exception {
try {
diff --git a/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java b/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
index 6c8e4d8..a7b73d6 100644
--- a/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
+++ b/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
@@ -20,6 +20,7 @@
import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.util.stream.Collectors.toList;
+import static org.junit.Assert.fail;
import com.google.common.base.CharMatcher;
import com.google.gerrit.entities.Account;
@@ -326,6 +327,13 @@
}
@Test
+ public void withStartCannotBeLessThanZero() throws Exception {
+ GroupInfo group1 = createGroup(name("group1"));
+ assertFailingQuery(
+ newQuery("uuid:" + group1.id).withStart(-1), "'start' parameter cannot be less than zero");
+ }
+
+ @Test
public void sortedByUuid() throws Exception {
GroupInfo group1 = createGroup(name("group1"));
GroupInfo group2 = createGroup(name("group2"));
@@ -477,6 +485,15 @@
return result;
}
+ protected void assertFailingQuery(QueryRequest query, String expectedMessage) throws Exception {
+ try {
+ assertQuery(query);
+ fail("expected BadRequestException for query '" + query + "'");
+ } catch (BadRequestException e) {
+ assertThat(e.getMessage()).isEqualTo(expectedMessage);
+ }
+ }
+
protected QueryRequest newQuery(Object query) {
return gApi.groups().query(query.toString());
}
diff --git a/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java b/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java
index 60d1655..c06fcde 100644
--- a/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java
+++ b/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java
@@ -19,6 +19,7 @@
import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.util.stream.Collectors.toList;
+import static org.junit.Assert.fail;
import com.google.common.base.CharMatcher;
import com.google.common.collect.ImmutableMap;
@@ -290,6 +291,13 @@
}
@Test
+ public void withStartCannotBeLessThanZero() throws Exception {
+ assertFailingQuery(
+ newQuery("name:" + allProjects.get()).withStart(-1),
+ "'start' parameter cannot be less than zero");
+ }
+
+ @Test
public void sortedByName() throws Exception {
ProjectInfo projectFoo = createProject("foo-" + name("project1"));
ProjectInfo projectBar = createProject("bar-" + name("project2"));
@@ -396,6 +404,15 @@
return result;
}
+ protected void assertFailingQuery(QueryRequest query, String expectedMessage) throws Exception {
+ try {
+ assertQuery(query);
+ fail("expected BadRequestException for query '" + query + "'");
+ } catch (BadRequestException e) {
+ assertThat(e.getMessage()).isEqualTo(expectedMessage);
+ }
+ }
+
protected QueryRequest newQuery(Object query) {
return gApi.projects().query(query.toString());
}
