Check the scale, rather than the absolute value

elastic · Aug 2, 2024 · 01b3c94 · 01b3c94
1 parent f79c621
commit 01b3c94
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 7 deletions.
diff --git a/libs/x-content/src/main/java/org/elasticsearch/xcontent/support/AbstractXContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/xcontent/support/AbstractXContentParser.java
@@ -151,11 +151,8 @@ 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);
+    private static final BigInteger LONG_MAX_VALUE_AS_BIGINTEGER = BigInteger.valueOf(Long.MAX_VALUE);
+    private static final BigInteger LONG_MIN_VALUE_AS_BIGINTEGER = BigInteger.valueOf(Long.MIN_VALUE);
 
     /** 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
@@ -170,8 +167,8 @@ private static long toLong(String stringValue, boolean coerce) {
         final BigInteger bigIntegerValue;
         try {
             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) {
+            // long can have a maximum of 19 digits - any more than that cannot be a long
+            if (Math.abs(bigDecimalValue.scale()) > 19) {
                 throw new IllegalArgumentException("Value [" + stringValue + "] is out of range for a long");
             }
             bigIntegerValue = coerce ? bigDecimalValue.toBigInteger() : bigDecimalValue.toBigIntegerExact();

diff --git a/libs/x-content/src/test/java/org/elasticsearch/xcontent/XContentParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/xcontent/XContentParserTests.java
@@ -31,6 +31,7 @@
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.in;
 import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.nullValue;
 import static org.junit.internal.matchers.ThrowableMessageMatcher.hasMessage;
 
@@ -74,6 +75,48 @@ public void testFloat() throws IOException {
         }
     }
 
+    public void testLongCoercion() throws IOException {
+        XContentType xContentType = randomFrom(XContentType.values());
+
+        String longValue1 = "5.5";
+        String longValue2 = "5e18";
+        String longValue3 = "2e100";
+        String longValue4 = "2e-100";
+
+        try (XContentBuilder builder = XContentBuilder.builder(xContentType.xContent())) {
+            builder.startObject();
+            builder.field("decimal", longValue1);
+            builder.field("expInRange", longValue2);
+            builder.field("expTooBig", longValue3);
+            builder.field("expTooSmall", longValue4);
+            builder.endObject();
+
+            try (XContentParser parser = createParser(xContentType.xContent(), BytesReference.bytes(builder))) {
+                assertThat(parser.nextToken(), is(XContentParser.Token.START_OBJECT));
+
+                assertThat(parser.nextToken(), is(XContentParser.Token.FIELD_NAME));
+                assertThat(parser.currentName(), is("decimal"));
+                assertThat(parser.nextToken(), is(XContentParser.Token.VALUE_STRING));
+                assertThat(parser.longValue(), equalTo(5L));
+
+                assertThat(parser.nextToken(), is(XContentParser.Token.FIELD_NAME));
+                assertThat(parser.currentName(), is("expInRange"));
+                assertThat(parser.nextToken(), is(XContentParser.Token.VALUE_STRING));
+                assertThat(parser.longValue(), equalTo((long) 5e18));
+
+                assertThat(parser.nextToken(), is(XContentParser.Token.FIELD_NAME));
+                assertThat(parser.currentName(), is("expTooBig"));
+                assertThat(parser.nextToken(), is(XContentParser.Token.VALUE_STRING));
+                expectThrows(IllegalArgumentException.class, parser::longValue);
+
+                assertThat(parser.nextToken(), is(XContentParser.Token.FIELD_NAME));
+                assertThat(parser.currentName(), is("expTooSmall"));
+                assertThat(parser.nextToken(), is(XContentParser.Token.VALUE_STRING));
+                expectThrows(IllegalArgumentException.class, parser::longValue);
+            }
+        }
+    }
+
     public void testReadList() throws IOException {
         assertThat(readList("{\"foo\": [\"bar\"]}"), contains("bar"));
         assertThat(readList("{\"foo\": [\"bar\",\"baz\"]}"), contains("bar", "baz"));