honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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
This commit is contained in:
David Turner 2019-03-28 12:13:25 +00:00
parent 6ef657c5ad
commit 1a3916a8de
4 changed files with 43 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -151,6 +151,12 @@ public abstract class AbstractXContentParser implements XContentParser {
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,7 +169,11 @@ public abstract class AbstractXContentParser implements XContentParser {
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");
@ -171,11 +181,11 @@ public abstract class AbstractXContentParser implements XContentParser {
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();
}

8
server/src/main/java/org/elasticsearch/common/Numbers.java
View File

@ -125,6 +125,10 @@ public final class Numbers {
}
}
// 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 final class Numbers {
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");

20
server/src/test/java/org/elasticsearch/common/NumbersTests.java
View File

@ -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 @@ import java.util.concurrent.atomic.AtomicInteger;
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() {

8
server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
View File

@ -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 class NumberFieldMapperTests extends AbstractNumericFieldMapperTestCase {
}
}
@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 class NumberFieldMapperTests extends AbstractNumericFieldMapperTestCase {
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 {