diff --git a/core/src/main/java/org/elasticsearch/common/Numbers.java b/core/src/main/java/org/elasticsearch/common/Numbers.java index 1735a0dfa65..2c4d700c92c 100644 --- a/core/src/main/java/org/elasticsearch/common/Numbers.java +++ b/core/src/main/java/org/elasticsearch/common/Numbers.java @@ -29,6 +29,9 @@ import java.math.BigInteger; */ public final class Numbers { + private static final BigInteger MAX_LONG_VALUE = BigInteger.valueOf(Long.MAX_VALUE); + private static final BigInteger MIN_LONG_VALUE = BigInteger.valueOf(Long.MIN_VALUE); + private Numbers() { } @@ -205,6 +208,33 @@ public final class Numbers { } } + /** 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. */ + public static long toLong(String stringValue, boolean coerce) { + try { + return Long.parseLong(stringValue); + } catch (NumberFormatException e) { + // we will try again with BigDecimal + } + + final BigInteger bigIntegerValue; + try { + BigDecimal bigDecimalValue = new BigDecimal(stringValue); + 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(MAX_LONG_VALUE) > 0 || bigIntegerValue.compareTo(MIN_LONG_VALUE) < 0) { + throw new IllegalArgumentException("Value [" + stringValue + "] is out of range for a long"); + } + + return bigIntegerValue.longValue(); + } + /** Return the int that {@code n} stores, or throws an exception if the * stored value cannot be converted to an int that stores the exact same * value. */ diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java index be128b2f212..9aae1ca396c 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/support/AbstractXContentParser.java @@ -22,6 +22,7 @@ package org.elasticsearch.common.xcontent.support; import org.apache.lucene.util.BytesRef; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.Booleans; +import org.elasticsearch.common.Numbers; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentParser; @@ -133,7 +134,14 @@ public abstract class AbstractXContentParser implements XContentParser { Token token = currentToken(); if (token == Token.VALUE_STRING) { checkCoerceString(coerce, Short.class); - return (short) Double.parseDouble(text()); + + double doubleValue = Double.parseDouble(text()); + + if (doubleValue < Short.MIN_VALUE || doubleValue > Short.MAX_VALUE) { + throw new IllegalArgumentException("Value [" + text() + "] is out of range for a short"); + } + + return (short) doubleValue; } short result = doShortValue(); ensureNumberConversion(coerce, result, Short.class); @@ -152,7 +160,13 @@ public abstract class AbstractXContentParser implements XContentParser { Token token = currentToken(); if (token == Token.VALUE_STRING) { checkCoerceString(coerce, Integer.class); - return (int) Double.parseDouble(text()); + double doubleValue = Double.parseDouble(text()); + + if (doubleValue < Integer.MIN_VALUE || doubleValue > Integer.MAX_VALUE) { + throw new IllegalArgumentException("Value [" + text() + "] is out of range for an integer"); + } + + return (int) doubleValue; } int result = doIntValue(); ensureNumberConversion(coerce, result, Integer.class); @@ -171,13 +185,7 @@ public abstract class AbstractXContentParser implements XContentParser { Token token = currentToken(); if (token == Token.VALUE_STRING) { checkCoerceString(coerce, Long.class); - // longs need special handling so we don't lose precision while parsing - String stringValue = text(); - try { - return Long.parseLong(stringValue); - } catch (NumberFormatException e) { - return (long) Double.parseDouble(stringValue); - } + return Numbers.toLong(text(), coerce); } long result = doLongValue(); ensureNumberConversion(coerce, result, Long.class); diff --git a/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index 8d8845be531..c4d3669b32d 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -36,6 +36,7 @@ import org.apache.lucene.search.Query; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.NumericUtils; import org.elasticsearch.common.Explicit; +import org.elasticsearch.common.Numbers; import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; @@ -43,6 +44,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentParser.Token; +import org.elasticsearch.common.xcontent.support.AbstractXContentParser; import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.fielddata.IndexNumericFieldData.NumericType; import org.elasticsearch.index.fielddata.plain.DocValuesIndexFieldData; @@ -644,8 +646,13 @@ public class NumberFieldMapper extends FieldMapper { LONG("long", NumericType.LONG) { @Override Long parse(Object value, boolean coerce) { - double doubleValue = objectToDouble(value); + if (value instanceof Long) { + return (Long)value; + } + double doubleValue = objectToDouble(value); + // this check does not guarantee that value is inside MIN_VALUE/MAX_VALUE because values up to 9223372036854776832 will + // be equal to Long.MAX_VALUE after conversion to double. More checks ahead. if (doubleValue < Long.MIN_VALUE || doubleValue > Long.MAX_VALUE) { throw new IllegalArgumentException("Value [" + value + "] is out of range for a long"); } @@ -653,18 +660,9 @@ public class NumberFieldMapper extends FieldMapper { throw new IllegalArgumentException("Value [" + value + "] has a decimal part"); } - if (value instanceof Number) { - return ((Number) value).longValue(); - } - // longs need special handling so we don't lose precision while parsing String stringValue = (value instanceof BytesRef) ? ((BytesRef) value).utf8ToString() : value.toString(); - - try { - return Long.parseLong(stringValue); - } catch (NumberFormatException e) { - return (long) Double.parseDouble(stringValue); - } + return Numbers.toLong(stringValue, coerce); } @Override @@ -836,7 +834,6 @@ public class NumberFieldMapper extends FieldMapper { return doubleValue; } - } public static final class NumberFieldType extends MappedFieldType { diff --git a/core/src/test/java/org/elasticsearch/common/NumbersTests.java b/core/src/test/java/org/elasticsearch/common/NumbersTests.java index e5563993ad5..46378ccc9e9 100644 --- a/core/src/test/java/org/elasticsearch/common/NumbersTests.java +++ b/core/src/test/java/org/elasticsearch/common/NumbersTests.java @@ -27,6 +27,21 @@ import java.util.concurrent.atomic.AtomicInteger; public class NumbersTests extends ESTestCase { + 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)); + + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> Numbers.toLong("9223372036854775808", false)); + assertEquals("Value [9223372036854775808] is out of range for a long", e.getMessage()); + + e = expectThrows(IllegalArgumentException.class, + () -> Numbers.toLong("-9223372036854775809", false)); + assertEquals("Value [-9223372036854775809] is out of range for a long", e.getMessage()); + } + public void testToLongExact() { assertEquals(3L, Numbers.toLongExact(Long.valueOf(3L))); assertEquals(3L, Numbers.toLongExact(Integer.valueOf(3))); diff --git a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java index 3ace8c04513..65ba83a7ab3 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -28,7 +28,9 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType; import org.elasticsearch.index.mapper.NumberFieldTypeTests.OutOfRangeSpec; +import java.io.ByteArrayInputStream; import java.io.IOException; +import java.math.BigInteger; import java.util.List; import java.util.Arrays; import java.util.HashSet; @@ -234,6 +236,7 @@ public class NumberFieldMapperTests extends AbstractNumericFieldMapperTestCase { .bytes(), XContentType.JSON)); MapperParsingException e = expectThrows(MapperParsingException.class, runnable); + assertThat(e.getCause().getMessage(), containsString("For input string: \"a\"")); mapping = XContentFactory.jsonBuilder().startObject().startObject("type") @@ -345,6 +348,26 @@ public class NumberFieldMapperTests extends AbstractNumericFieldMapperTestCase { public void testOutOfRangeValues() throws IOException { final List> 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.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.BYTE, 128, "is out of range for a byte"), + OutOfRangeSpec.of(NumberType.SHORT, 32768, "out of range of Java short"), + OutOfRangeSpec.of(NumberType.INTEGER, 2147483648L, " out of range of int"), + OutOfRangeSpec.of(NumberType.LONG, new BigInteger("9223372036854775808"), "out of range of long"), + + OutOfRangeSpec.of(NumberType.BYTE, -129, "is out of range for a byte"), + OutOfRangeSpec.of(NumberType.SHORT, -32769, "out of range of Java short"), + OutOfRangeSpec.of(NumberType.INTEGER, -2147483649L, " out of range of int"), + OutOfRangeSpec.of(NumberType.LONG, new BigInteger("-9223372036854775809"), "out of range of long"), + OutOfRangeSpec.of(NumberType.HALF_FLOAT, "65520", "[half_float] supports only finite values"), OutOfRangeSpec.of(NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), OutOfRangeSpec.of(NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"), @@ -398,6 +421,13 @@ public class NumberFieldMapperTests extends AbstractNumericFieldMapperTestCase { } private BytesReference createIndexRequest(Object value) throws IOException { - return XContentFactory.jsonBuilder().startObject().field("field", value).endObject().bytes(); + if (value instanceof BigInteger) { + return XContentFactory.jsonBuilder() + .startObject() + .rawField("field", new ByteArrayInputStream(value.toString().getBytes("UTF-8")), XContentType.JSON) + .endObject().bytes(); + } else { + return XContentFactory.jsonBuilder().startObject().field("field", value).endObject().bytes(); + } } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java index 13e5e35df68..58ece7b5073 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java @@ -46,9 +46,12 @@ import org.junit.Before; import java.io.IOException; import java.math.BigDecimal; +import java.math.BigInteger; import java.nio.charset.StandardCharsets; import java.util.Arrays; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.function.Supplier; import static org.hamcrest.Matchers.containsString; @@ -389,6 +392,22 @@ public class NumberFieldTypeTests extends FieldTypeTestCase { public void testParseOutOfRangeValues() throws IOException { final List> inputs = Arrays.asList( + OutOfRangeSpec.of(NumberType.BYTE, "128", "out of range for a byte"), + OutOfRangeSpec.of(NumberType.BYTE, 128, "is out of range for a byte"), + OutOfRangeSpec.of(NumberType.BYTE, -129, "is out of range for a byte"), + + OutOfRangeSpec.of(NumberType.SHORT, "32768", "out of range for a short"), + OutOfRangeSpec.of(NumberType.SHORT, 32768, "is out of range for a short"), + OutOfRangeSpec.of(NumberType.SHORT, -32769, "is out of range for a short"), + + OutOfRangeSpec.of(NumberType.INTEGER, "2147483648", "out of range for an integer"), + OutOfRangeSpec.of(NumberType.INTEGER, 2147483648L, "is out of range for an integer"), + OutOfRangeSpec.of(NumberType.INTEGER, -2147483649L, "is out of range for an integer"), + + OutOfRangeSpec.of(NumberType.LONG, "9223372036854775808", "out of range for a long"), + OutOfRangeSpec.of(NumberType.LONG, new BigInteger("9223372036854775808"), " is out of range for a long"), + OutOfRangeSpec.of(NumberType.LONG, new BigInteger("-9223372036854775809"), " is out of range for a long"), + OutOfRangeSpec.of(NumberType.HALF_FLOAT, "65520", "[half_float] supports only finite values"), OutOfRangeSpec.of(NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), OutOfRangeSpec.of(NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"), diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFailureTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFailureTests.java index 09af00ca366..f101b125382 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFailureTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFailureTests.java @@ -63,7 +63,7 @@ public class ReindexFailureTests extends ReindexTestCase { .batches(1) .failures(both(greaterThan(0)).and(lessThanOrEqualTo(maximumNumberOfShards())))); for (Failure failure: response.getBulkFailures()) { - assertThat(failure.getMessage(), containsString("NumberFormatException[For input string: \"words words\"]")); + assertThat(failure.getMessage(), containsString("IllegalArgumentException[For input string: \"words words\"]")); } }