Do not fail if user provides a too high/low label value in query

For label queries the max supported range for values is -4 to +4
(hard-coded in LabelPredicate). If the provided label value was too
high/low it could happen that we parsed a range where min was greater
than max. This led to an IllegalArgumentException [1] when we tried to
initialze a list with a size based on min and max. For queries that used
the 'label' predicate (e.g. "label:Code-Review>10") that exception was
caught and the user got a 400 Bad Request. If it was a default query
(e.g. "Code-Review>10") the exception caused a 500 Internal Server
Error.

Fix this by ensuring that for parsed ranges -4 <= min <= +4, -4 <= max
<= +4 and min <= max is always true.

We saw the exception below [1] when a user tried to query "size>1000"
which we interpret as label.

[1]
AutoRetry: restapi.change.QueryChanges failed, retry with tracing enabled [CONTEXT forced=true TRACE_ID="retry-on-failure-1574108318670-91eac85e" ]
java.lang.IllegalArgumentException: initialArraySize cannot be negative but was: -996
        at com.google.common.collect.CollectPreconditions.checkNonnegative(CollectPreconditions.java:39)
        at com.google.common.collect.Lists.newArrayListWithCapacity(Lists.java:174)
        at com.google.gerrit.server.query.change.LabelPredicate.predicates(LabelPredicate.java:117)
        at com.google.gerrit.server.query.change.LabelPredicate.<init>(LabelPredicate.java:79)
        at com.google.gerrit.server.query.change.ChangeQueryBuilder.label(ChangeQueryBuilder.java:869)
        at com.google.gerrit.server.query.change.ChangeQueryBuilder.defaultField(ChangeQueryBuilder.java:1285)
        at com.google.gerrit.index.query.QueryBuilder.toPredicate(QueryBuilder.java:258)
        at com.google.gerrit.index.query.QueryBuilder.children(QueryBuilder.java:331)
        at com.google.gerrit.index.query.QueryBuilder.toPredicate(QueryBuilder.java:251)
        at com.google.gerrit.index.query.QueryBuilder.parse(QueryBuilder.java:224)
        at com.google.gerrit.index.query.QueryBuilder.parse(QueryBuilder.java:243)
        at com.google.gerrit.server.restapi.change.QueryChanges.query(QueryChanges.java:144)
        at com.google.gerrit.server.restapi.change.QueryChanges.apply(QueryChanges.java:121)
        at com.google.gerrit.server.restapi.change.QueryChanges.apply(QueryChanges.java:43)
        at com.google.gerrit.httpd.restapi.RestApiServlet.lambda$invokeRestReadViewWithRetry$3(RestApiServlet.java:723)
        at com.github.rholder.retry.AttemptTimeLimiters$NoAttemptTimeLimit.call(AttemptTimeLimiters.java:78)
        at com.github.rholder.retry.Retryer.call(Retryer.java:160)
        at com.google.gerrit.server.update.RetryHelper.executeWithTimeoutCount(RetryHelper.java:417)
        at com.google.gerrit.server.update.RetryHelper.executeWithAttemptAndTimeoutCount(RetryHelper.java:368)
        at com.google.gerrit.server.update.RetryHelper.execute(RetryHelper.java:271)
        at com.google.gerrit.httpd.restapi.RestApiServlet.invokeRestEndpointWithRetry(RestApiServlet.java:820)
        at com.google.gerrit.httpd.restapi.RestApiServlet.invokeRestReadViewWithRetry(RestApiServlet.java:718)
        at com.google.gerrit.httpd.restapi.RestApiServlet.service(RestApiServlet.java:501)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:717)
        ...

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Icea364742485f9e6d7cc836cfc95c97d2a4569ba

java/com/google/gerrit/index/query/RangeUtil.java

@@ -106,6 +106,10 @@ public final class RangeUtil {
         break;
     }
 
+    // Ensure that minValue <= min/max <= maxValue.
+    min = Ints.constrainToRange(min, minValue, maxValue);
+    max = Ints.constrainToRange(max, minValue, maxValue);
+
     return new Range(prefix, min, max);
   }
 

javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java

@@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static java.util.stream.Collectors.toList;
+import static javax.servlet.http.HttpServletResponse.SC_OK;
 
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -182,6 +183,18 @@ public class QueryChangeIT extends AbstractDaemonTest {
         .containsExactly(changeId3, changeId4);
   }
 
+  @Test
+  public void usingOutOfRangeLabelValuesDoesNotCauseError() throws Exception {
+    for (String operator : ImmutableList.of("=", ">", ">=", "<", "<=")) {
+      QueryChanges queryChanges = queryChangesProvider.get();
+      queryChanges.addQuery("label:Code-Review" + operator + "10");
+      queryChanges.addQuery("label:Code-Review" + operator + "-10");
+      queryChanges.addQuery("Code-Review" + operator + "10");
+      queryChanges.addQuery("Code-Review" + operator + "-10");
+      assertThat(queryChanges.apply(TopLevelResource.INSTANCE).statusCode()).isEqualTo(SC_OK);
+    }
+  }
+
   private static void assertNoChangeHasMoreChangesSet(List<ChangeInfo> results) {
     for (ChangeInfo info : results) {
       assertThat(info._moreChanges).isNull();

javatests/com/google/gerrit/index/query/RangeUtilTest.java

@@ -0,0 +1,34 @@
+// Copyright (C) 2019 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.index.query;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.index.query.RangeUtil.Range;
+import org.junit.Test;
+
+public class RangeUtilTest {
+  @Test
+  public void getRangeForValueOutsideOfMinMaxRange_minNotGreaterThanMax() {
+    for (String operator : ImmutableList.of("=", ">", ">=", "<", "<=")) {
+      Range range = RangeUtil.getRange("foo", operator, 10, -4, 4);
+      assertThat(range.min).isAtMost(range.max);
+
+      range = RangeUtil.getRange("foo", operator, -10, -4, 4);
+      assertThat(range.min).isAtMost(range.max);
+    }
+  }
+}