Optimise rejection of out-of-range long values (#40325)

Today if you try and insert a very large number like `1e9999999` into a long
field we first construct this number as a `BigDecimal`, convert this to a
`BigInteger` and then reject it because it is out of range. Unfortunately
making such a large `BigInteger` is rather expensive.

We can avoid this expense by performing a (weaker) range check on the
`BigDecimal` representation of incoming `long`s too.

Relates #26137
Closes #40323

Author: DaveCTurner

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java

@@ -151,6 +151,12 @@ public int intValue(boolean coerce) throws IOException {
 
     protected abstract int doIntValue() throws IOException;
 
+    private static BigInteger LONG_MAX_VALUE_AS_BIGINTEGER = BigInteger.valueOf(Long.MAX_VALUE);
+    private static BigInteger LONG_MIN_VALUE_AS_BIGINTEGER = BigInteger.valueOf(Long.MIN_VALUE);
+    // weak bounds on the BigDecimal representation to allow for coercion
+    private static BigDecimal BIGDECIMAL_GREATER_THAN_LONG_MAX_VALUE = BigDecimal.valueOf(Long.MAX_VALUE).add(BigDecimal.ONE);
+    private static BigDecimal BIGDECIMAL_LESS_THAN_LONG_MIN_VALUE = BigDecimal.valueOf(Long.MIN_VALUE).subtract(BigDecimal.ONE);
+
     /** Return the long that {@code stringValue} stores or throws an exception if the
      *  stored value cannot be converted to a long that stores the exact same
      *  value and {@code coerce} is false. */
@@ -163,19 +169,23 @@ private static long toLong(String stringValue, boolean coerce) {
 
         final BigInteger bigIntegerValue;
         try {
-            BigDecimal bigDecimalValue = new BigDecimal(stringValue);
+            final BigDecimal bigDecimalValue = new BigDecimal(stringValue);
+            if (bigDecimalValue.compareTo(BIGDECIMAL_GREATER_THAN_LONG_MAX_VALUE) >= 0 ||
+                bigDecimalValue.compareTo(BIGDECIMAL_LESS_THAN_LONG_MIN_VALUE) <= 0) {
+                throw new IllegalArgumentException("Value [" + stringValue + "] is out of range for a long");
+            }
             bigIntegerValue = coerce ? bigDecimalValue.toBigInteger() : bigDecimalValue.toBigIntegerExact();
         } catch (ArithmeticException e) {
             throw new IllegalArgumentException("Value [" + stringValue + "] has a decimal part");
         } catch (NumberFormatException e) {
             throw new IllegalArgumentException("For input string: \"" + stringValue + "\"");
         }
 
-        if (bigIntegerValue.compareTo(BigInteger.valueOf(Long.MAX_VALUE)) > 0 ||
-                bigIntegerValue.compareTo(BigInteger.valueOf(Long.MIN_VALUE)) < 0) {
+        if (bigIntegerValue.compareTo(LONG_MAX_VALUE_AS_BIGINTEGER) > 0 || bigIntegerValue.compareTo(LONG_MIN_VALUE_AS_BIGINTEGER) < 0) {
             throw new IllegalArgumentException("Value [" + stringValue + "] is out of range for a long");
         }
 
+        assert bigIntegerValue.longValueExact() <= Long.MAX_VALUE; // asserting that no ArithmeticException is thrown
         return bigIntegerValue.longValue();
     }
 

server/src/main/java/org/elasticsearch/common/Numbers.java

@@ -125,6 +125,10 @@ public static long toLongExact(Number n) {
         }
     }
 
+    // weak bounds on the BigDecimal representation to allow for coercion
+    private static BigDecimal BIGDECIMAL_GREATER_THAN_LONG_MAX_VALUE = BigDecimal.valueOf(Long.MAX_VALUE).add(BigDecimal.ONE);
+    private static BigDecimal BIGDECIMAL_LESS_THAN_LONG_MIN_VALUE = BigDecimal.valueOf(Long.MIN_VALUE).subtract(BigDecimal.ONE);
+
     /** Return the long that {@code stringValue} stores or throws an exception if the
      *  stored value cannot be converted to a long that stores the exact same
      *  value and {@code coerce} is false. */
@@ -138,6 +142,10 @@ public static long toLong(String stringValue, boolean coerce) {
         final BigInteger bigIntegerValue;
         try {
             BigDecimal bigDecimalValue = new BigDecimal(stringValue);
+            if (bigDecimalValue.compareTo(BIGDECIMAL_GREATER_THAN_LONG_MAX_VALUE) >= 0 ||
+                bigDecimalValue.compareTo(BIGDECIMAL_LESS_THAN_LONG_MIN_VALUE) <= 0) {
+                throw new IllegalArgumentException("Value [" + stringValue + "] is out of range for a long");
+            }
             bigIntegerValue = coerce ? bigDecimalValue.toBigInteger() : bigDecimalValue.toBigIntegerExact();
         } catch (ArithmeticException e) {
             throw new IllegalArgumentException("Value [" + stringValue + "] has a decimal part");

server/src/test/java/org/elasticsearch/common/NumbersTests.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.common;
 
+import com.carrotsearch.randomizedtesting.annotations.Timeout;
 import org.elasticsearch.test.ESTestCase;
 
 import java.math.BigDecimal;
@@ -27,19 +28,26 @@
 
 public class NumbersTests extends ESTestCase {
 
+    @Timeout(millis = 10000)
     public void testToLong() {
         assertEquals(3L, Numbers.toLong("3", false));
         assertEquals(3L, Numbers.toLong("3.1", true));
         assertEquals(9223372036854775807L, Numbers.toLong("9223372036854775807.00", false));
         assertEquals(-9223372036854775808L, Numbers.toLong("-9223372036854775808.00", false));
+        assertEquals(9223372036854775807L, Numbers.toLong("9223372036854775807.00", true));
+        assertEquals(-9223372036854775808L, Numbers.toLong("-9223372036854775808.00", true));
+        assertEquals(9223372036854775807L, Numbers.toLong("9223372036854775807.99", true));
+        assertEquals(-9223372036854775808L, Numbers.toLong("-9223372036854775808.99", true));
 
-        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
-            () -> Numbers.toLong("9223372036854775808", false));
-        assertEquals("Value [9223372036854775808] is out of range for a long", e.getMessage());
+        assertEquals("Value [9223372036854775808] is out of range for a long", expectThrows(IllegalArgumentException.class,
+            () -> Numbers.toLong("9223372036854775808", false)).getMessage());
+        assertEquals("Value [-9223372036854775809] is out of range for a long", expectThrows(IllegalArgumentException.class,
+            () -> Numbers.toLong("-9223372036854775809", false)).getMessage());
 
-        e = expectThrows(IllegalArgumentException.class,
-            () -> Numbers.toLong("-9223372036854775809", false));
-        assertEquals("Value [-9223372036854775809] is out of range for a long", e.getMessage());
+        assertEquals("Value [1e99999999] is out of range for a long", expectThrows(IllegalArgumentException.class,
+            () -> Numbers.toLong("1e99999999", false)).getMessage());
+        assertEquals("Value [-1e99999999] is out of range for a long", expectThrows(IllegalArgumentException.class,
+            () -> Numbers.toLong("-1e99999999", false)).getMessage());
     }
 
     public void testToLongExact() {

server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.index.mapper;
 
+import com.carrotsearch.randomizedtesting.annotations.Timeout;
 import org.apache.lucene.index.DocValuesType;
 import org.apache.lucene.index.IndexableField;
 import org.elasticsearch.common.Strings;
@@ -367,17 +368,20 @@ public void testEmptyName() throws IOException {
         }
     }
 
+    @Timeout(millis = 30000)
     public void testOutOfRangeValues() throws IOException {
         final List<OutOfRangeSpec<Object>> inputs = Arrays.asList(
             OutOfRangeSpec.of(NumberType.BYTE, "128", "is out of range for a byte"),
             OutOfRangeSpec.of(NumberType.SHORT, "32768", "is out of range for a short"),
             OutOfRangeSpec.of(NumberType.INTEGER, "2147483648", "is out of range for an integer"),
             OutOfRangeSpec.of(NumberType.LONG, "9223372036854775808", "out of range for a long"),
+            OutOfRangeSpec.of(NumberType.LONG, "1e999999999", "out of range for a long"),
 
             OutOfRangeSpec.of(NumberType.BYTE, "-129", "is out of range for a byte"),
             OutOfRangeSpec.of(NumberType.SHORT, "-32769", "is out of range for a short"),
             OutOfRangeSpec.of(NumberType.INTEGER, "-2147483649", "is out of range for an integer"),
             OutOfRangeSpec.of(NumberType.LONG, "-9223372036854775809", "out of range for a long"),
+            OutOfRangeSpec.of(NumberType.LONG, "-1e999999999", "out of range for a long"),
 
             OutOfRangeSpec.of(NumberType.BYTE, 128, "is out of range for a byte"),
             OutOfRangeSpec.of(NumberType.SHORT, 32768, "out of range of Java short"),
@@ -419,6 +423,10 @@ public void testOutOfRangeValues() throws IOException {
                     e.getCause().getMessage(), containsString(item.message));
             }
         }
+
+        // the following two strings are in-range for a long after coercion
+        parseRequest(NumberType.LONG, createIndexRequest("9223372036854775807.9"));
+        parseRequest(NumberType.LONG, createIndexRequest("-9223372036854775808.9"));
     }
 
     private void parseRequest(NumberType type, BytesReference content) throws IOException {