From 2d0e5c1f5d5f328a0ff857d870cb8276fce7f3c7 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Wed, 12 Jul 2017 09:47:11 +0300 Subject: [PATCH 01/16] validate half float values --- .../index/mapper/NumberFieldMapper.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) 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 ec9fcf54730e2..a1ca1f2f6c19d 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -236,6 +236,26 @@ public List createFields(String name, Number value, return fields; } + void validateParsed(Number value) throws IllegalArgumentException { + /** this checks mimics logic of {@link org.apache.lucene.document.HalfFloatPoint#halfFloatToShortBits} */ + int floatBits = Float.floatToIntBits(value.floatValue()); + int exp = (floatBits >>> 23) & 0xff; + if (exp == 0xff) { + throw new IllegalArgumentException("Infitity and NaN are not supported"); + } + if (exp == 0x00) { + return; + } + exp = exp - 127 + 15; + if (exp >= 0x1f) { + throw new IllegalArgumentException("Too large value"); + } + int shift = 23 - 10 - exp + 1; + if (exp <= 0 && shift >= 32) { + throw new IllegalArgumentException("Decimal part is too small"); + } + } + @Override FieldStats.Double stats(IndexReader reader, String fieldName, boolean isSearchable, boolean isAggregatable) throws IOException { @@ -863,6 +883,10 @@ Number valueForSearch(Number value) { return value; } + void validateParsed(Number value) throws IllegalArgumentException { + // todo override in all subclasses and then make abstract + } + /** * Returns true if the object is a number and has a decimal part */ @@ -1067,6 +1091,8 @@ protected void parseCreateField(ParseContext context, List field numericValue = fieldType().type.parse(value, coerce.value()); } + fieldType().type.validateParsed(numericValue); + if (includeInAll) { context.allEntries().addText(fieldType().name(), value.toString(), fieldType().boost()); } From 6e0a6ea344c07fc1fbee613628fc5f6e1b0a275e Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Mon, 17 Jul 2017 21:40:36 +0300 Subject: [PATCH 02/16] test upper bound for numeric mapper --- .../index/mapper/NumberFieldMapperTests.java | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) 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 871d62d8bd6be..e9da47b5588b9 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -21,6 +21,7 @@ import org.apache.lucene.index.DocValuesType; import org.apache.lucene.index.IndexableField; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; @@ -314,4 +315,43 @@ public void testEmptyName() throws IOException { assertThat(e.getMessage(), containsString("name cannot be empty string")); } } + + public void testOutOfRangeHalfFloatUpperBound() throws IOException { + DocumentMapper mapper = createDocumentMapper("half_float"); + ThrowingRunnable runnable = () -> mapper.parse(createIndexRequest("65505")); + MapperParsingException e = expectThrows(MapperParsingException.class, runnable); + assertThat(e.getCause().getMessage(), containsString("only supports finite values")); + } + + public void testOutOfRangeFloatUpperBound() throws IOException { + DocumentMapper mapper = createDocumentMapper("float"); + ThrowingRunnable runnable = () -> mapper.parse(createIndexRequest(Double.toString(Math.pow(2, 150)))); + MapperParsingException e = expectThrows(MapperParsingException.class, runnable); + assertThat(e.getCause().getMessage(), containsString("only supports finite values")); + } + + private DocumentMapper createDocumentMapper(String type) throws IOException { + String mapping = XContentFactory.jsonBuilder() + .startObject() + .startObject("type") + .startObject("properties") + .startObject("field") + .field("type", type) + .endObject() + .endObject() + .endObject() + .endObject() + .string(); + + return parser.parse("type", new CompressedXContent(mapping)); + } + + private SourceToParse createIndexRequest(String value) throws IOException { + return SourceToParse.source("test", "type", "1", XContentFactory.jsonBuilder() + .startObject() + .field("field", value) + .endObject() + .bytes(), + XContentType.JSON); + } } From d1ebd6fd57ac097095ec84acc87b8ebecf560aad Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Fri, 21 Jul 2017 10:59:39 +0300 Subject: [PATCH 03/16] test for upper bound for float, double and half_float --- .../index/mapper/NumberFieldMapper.java | 35 +++++++++++++++++++ .../index/mapper/NumberFieldMapperTests.java | 32 ++++++++++------- 2 files changed, 54 insertions(+), 13 deletions(-) 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 62aab947f9566..d78b7aa793b52 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -215,6 +215,19 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm, return query; } + @Override + void validateParsed(Number value) { + float val = value.floatValue(); + + if ( + !Float.isFinite(val) + || !Float.isFinite(HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(val))) + || val > 65504 + ) { + throw new IllegalArgumentException("[half_float] supports only finite values"); + } + } + @Override public List createFields(String name, Number value, boolean indexed, boolean docValued, boolean stored) { @@ -292,6 +305,13 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm, return query; } + @Override + void validateParsed(Number value) { + if (!Float.isFinite(value.floatValue())) { + throw new IllegalArgumentException("[float] supports only finite values"); + } + } + @Override public List createFields(String name, Number value, boolean indexed, boolean docValued, boolean stored) { @@ -369,6 +389,13 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm, return query; } + @Override + void validateParsed(Number value) { + if (!Double.isFinite(value.doubleValue())) { + throw new IllegalArgumentException("[double] supports only finite values, but got [" + value.toString() + "]"); + } + } + @Override public List createFields(String name, Number value, boolean indexed, boolean docValued, boolean stored) { @@ -750,6 +777,12 @@ Number valueForSearch(Number value) { return value; } + /** + * @throws IllegalArgumentException if value is not finite for this type + */ + void validateParsed(Number value) { + } + /** * Returns true if the object is a number and has a decimal part */ @@ -949,6 +982,8 @@ protected void parseCreateField(ParseContext context, List field numericValue = fieldType().type.parse(value, coerce.value()); } + fieldType().type.validateParsed(numericValue); + if (includeInAll) { context.allEntries().addText(fieldType().name(), value.toString(), fieldType().boost()); } 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 e9da47b5588b9..9040409a94381 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -21,14 +21,16 @@ import org.apache.lucene.index.DocValuesType; import org.apache.lucene.index.IndexableField; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; +import java.math.BigDecimal; import java.util.Arrays; +import java.util.HashMap; import java.util.HashSet; +import java.util.Map; import static org.hamcrest.Matchers.containsString; @@ -316,20 +318,24 @@ public void testEmptyName() throws IOException { } } - public void testOutOfRangeHalfFloatUpperBound() throws IOException { - DocumentMapper mapper = createDocumentMapper("half_float"); - ThrowingRunnable runnable = () -> mapper.parse(createIndexRequest("65505")); - MapperParsingException e = expectThrows(MapperParsingException.class, runnable); - assertThat(e.getCause().getMessage(), containsString("only supports finite values")); + public void testOutOfRangeValue() { + final Map outOfRangeValues = new HashMap<>(); + outOfRangeValues.put("float", new BigDecimal("3.4028235E39").toString()); + outOfRangeValues.put("double", new BigDecimal("1.7976931348623157E309").toString()); + outOfRangeValues.put("half_float", new BigDecimal("65504.1").toString()); + + outOfRangeValues.forEach((type, value) -> { + try { + createDocumentMapper(type).parse(createIndexRequest(value)); + fail("Mapper parsing exception expected for [" + type + "]"); + } catch (MapperParsingException e) { + assertThat(e.getCause().getMessage(), containsString("[" + type + "] supports only finite values")); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); } - public void testOutOfRangeFloatUpperBound() throws IOException { - DocumentMapper mapper = createDocumentMapper("float"); - ThrowingRunnable runnable = () -> mapper.parse(createIndexRequest(Double.toString(Math.pow(2, 150)))); - MapperParsingException e = expectThrows(MapperParsingException.class, runnable); - assertThat(e.getCause().getMessage(), containsString("only supports finite values")); - } - private DocumentMapper createDocumentMapper(String type) throws IOException { String mapping = XContentFactory.jsonBuilder() .startObject() From 44983d8a35cf7124a33c886e1742c5eb0255f704 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 23 Jul 2017 10:07:23 +0300 Subject: [PATCH 04/16] more tests on NaN and Infinity for NumberFieldMapper --- .../index/mapper/NumberFieldMapper.java | 112 ++++++++++-------- .../index/mapper/NumberFieldMapperTests.java | 109 +++++++++++++---- 2 files changed, 149 insertions(+), 72 deletions(-) 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 d78b7aa793b52..76947079a2184 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -162,12 +162,25 @@ public enum NumberType { HALF_FLOAT("half_float", NumericType.HALF_FLOAT) { @Override Float parse(Object value, boolean coerce) { - return (Float) FLOAT.parse(value, false); + final Float result; + + if (value instanceof Number) { + result = ((Number) value).floatValue(); + } else { + if (value instanceof BytesRef) { + value = ((BytesRef) value).utf8ToString(); + } + result = Float.parseFloat(value.toString()); + } + validateParsed(result); + return result; } @Override Float parse(XContentParser parser, boolean coerce) throws IOException { - return parser.floatValue(coerce); + Float parsed = parser.floatValue(coerce); + validateParsed(parsed); + return parsed; } @Override @@ -215,19 +228,6 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm, return query; } - @Override - void validateParsed(Number value) { - float val = value.floatValue(); - - if ( - !Float.isFinite(val) - || !Float.isFinite(HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(val))) - || val > 65504 - ) { - throw new IllegalArgumentException("[half_float] supports only finite values"); - } - } - @Override public List createFields(String name, Number value, boolean indexed, boolean docValued, boolean stored) { @@ -244,22 +244,39 @@ public List createFields(String name, Number value, } return fields; } + + private void validateParsed(Float value) { + if ( + value.isNaN() || value.isInfinite() + || value > 65504 + || !Float.isFinite(HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(value))) + ) { + throw new IllegalArgumentException("[half_float] supports only finite values, but [" + value + "] given"); + } + } }, FLOAT("float", NumericType.FLOAT) { @Override Float parse(Object value, boolean coerce) { + final Float result; + if (value instanceof Number) { - return ((Number) value).floatValue(); - } - if (value instanceof BytesRef) { - value = ((BytesRef) value).utf8ToString(); + result = ((Number) value).floatValue(); + } else { + if (value instanceof BytesRef) { + value = ((BytesRef) value).utf8ToString(); + } + result = Float.parseFloat(value.toString()); } - return Float.parseFloat(value.toString()); + validateParsed(result); + return result; } @Override Float parse(XContentParser parser, boolean coerce) throws IOException { - return parser.floatValue(coerce); + Float parsed = parser.floatValue(coerce); + validateParsed(parsed); + return parsed; } @Override @@ -305,13 +322,6 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm, return query; } - @Override - void validateParsed(Number value) { - if (!Float.isFinite(value.floatValue())) { - throw new IllegalArgumentException("[float] supports only finite values"); - } - } - @Override public List createFields(String name, Number value, boolean indexed, boolean docValued, boolean stored) { @@ -328,22 +338,35 @@ public List createFields(String name, Number value, } return fields; } + + private void validateParsed(Float value) { + if (value.isInfinite() || value.isNaN()) { + throw new IllegalArgumentException("[float] supports only finite values, but [" + value + "] given"); + } + } }, DOUBLE("double", NumericType.DOUBLE) { @Override Double parse(Object value, boolean coerce) { + final Double result; + if (value instanceof Number) { - return ((Number) value).doubleValue(); - } - if (value instanceof BytesRef) { - value = ((BytesRef) value).utf8ToString(); + result = ((Number) value).doubleValue(); + } else { + if (value instanceof BytesRef) { + value = ((BytesRef) value).utf8ToString(); + } + result = Double.parseDouble(value.toString()); } - return Double.parseDouble(value.toString()); + validateParsed(result); + return result; } @Override Double parse(XContentParser parser, boolean coerce) throws IOException { - return parser.doubleValue(coerce); + Double parsed = parser.doubleValue(coerce); + validateParsed(parsed); + return parsed; } @Override @@ -389,13 +412,6 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm, return query; } - @Override - void validateParsed(Number value) { - if (!Double.isFinite(value.doubleValue())) { - throw new IllegalArgumentException("[double] supports only finite values, but got [" + value.toString() + "]"); - } - } - @Override public List createFields(String name, Number value, boolean indexed, boolean docValued, boolean stored) { @@ -412,6 +428,12 @@ public List createFields(String name, Number value, } return fields; } + + private void validateParsed(Double value) { + if (value.isInfinite() || value.isNaN()) { + throw new IllegalArgumentException("[double] supports only finite values, but [" + value + "] given"); + } + } }, BYTE("byte", NumericType.BYTE) { @Override @@ -777,12 +799,6 @@ Number valueForSearch(Number value) { return value; } - /** - * @throws IllegalArgumentException if value is not finite for this type - */ - void validateParsed(Number value) { - } - /** * Returns true if the object is a number and has a decimal part */ @@ -982,8 +998,6 @@ protected void parseCreateField(ParseContext context, List field numericValue = fieldType().type.parse(value, coerce.value()); } - fieldType().type.validateParsed(numericValue); - if (includeInAll) { context.allEntries().addText(fieldType().name(), value.toString(), fieldType().boost()); } 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 9040409a94381..16ceaf133181d 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -21,16 +21,15 @@ import org.apache.lucene.index.DocValuesType; import org.apache.lucene.index.IndexableField; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; import java.math.BigDecimal; -import java.util.Arrays; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; +import java.math.BigInteger; +import java.util.*; import static org.hamcrest.Matchers.containsString; @@ -318,22 +317,91 @@ public void testEmptyName() throws IOException { } } - public void testOutOfRangeValue() { - final Map outOfRangeValues = new HashMap<>(); - outOfRangeValues.put("float", new BigDecimal("3.4028235E39").toString()); - outOfRangeValues.put("double", new BigDecimal("1.7976931348623157E309").toString()); - outOfRangeValues.put("half_float", new BigDecimal("65504.1").toString()); + public void testOutOfRangeValuesFromRequest() throws IOException { + final List> inputs = Arrays.asList( + new Tuple<>("half_float", "65504.1", "[half_float] supports only finite values"), + new Tuple<>("float", "3.4028235E39", "[float] supports only finite values"), + new Tuple<>("double", "1.7976931348623157E309", "[double] supports only finite values"), - outOfRangeValues.forEach((type, value) -> { + new Tuple<>("byte", "128", "is out of range for a byte"), + new Tuple<>("short", "32768", "is out of range for a short"), + new Tuple<>("integer", "2147483648", "For input string"), + new Tuple<>("long", "92233720368547758080", "For input string"), + + new Tuple<>("half_float", Float.NaN, "[half_float] supports only finite values"), + new Tuple<>("float", Float.NaN, "[float] supports only finite values"), + new Tuple<>("double", Double.NaN, "[double] supports only finite values"), + + new Tuple<>("half_float", Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), + new Tuple<>("float", Float.POSITIVE_INFINITY, "[float] supports only finite values"), + new Tuple<>("double", Double.POSITIVE_INFINITY, "[double] supports only finite values") + ); + + for(Tuple item: inputs) { try { - createDocumentMapper(type).parse(createIndexRequest(value)); - fail("Mapper parsing exception expected for [" + type + "]"); + parseRequest(item.type, createIndexRequest(item.value)); + fail("Mapper parsing exception expected for [" + item.type + "] with value [" + item.value + "]"); } catch (MapperParsingException e) { - assertThat(e.getCause().getMessage(), containsString("[" + type + "] supports only finite values")); - } catch (IOException e) { - throw new RuntimeException(e); + assertThat("Incorrect error message for [" + item.type + "] with value [" + item.value + "]", + e.getCause().getMessage(), containsString(item.message)); + } + } + } + + public void testOutOfRangeValuesFromObject() throws IOException { + final List> inputs = Arrays.asList( + new Tuple<>(NumberFieldMapper.NumberType.BYTE, "128", "Value out of range"), + new Tuple<>(NumberFieldMapper.NumberType.SHORT, "32768", "Value out of range"), + new Tuple<>(NumberFieldMapper.NumberType.INTEGER, "2147483648", "For input string"), + new Tuple<>(NumberFieldMapper.NumberType.LONG, "9223372036854775808", "For input string"), + + new Tuple<>(NumberFieldMapper.NumberType.BYTE, 128, "is out of range for a byte"), + new Tuple<>(NumberFieldMapper.NumberType.SHORT, 32768, "is out of range for a short"), + new Tuple<>(NumberFieldMapper.NumberType.INTEGER, 2147483648L, "is out of range for an integer"), + new Tuple<>(NumberFieldMapper.NumberType.LONG, new BigInteger("92233720368547758080"), " is out of range for a long"), + + new Tuple<>(NumberFieldMapper.NumberType.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), + new Tuple<>(NumberFieldMapper.NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), + new Tuple<>(NumberFieldMapper.NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"), + + new Tuple<>(NumberFieldMapper.NumberType.HALF_FLOAT, 65504.1, "[half_float] supports only finite values"), + new Tuple<>(NumberFieldMapper.NumberType.FLOAT, 3.4028235E39, "[float] supports only finite values"), + new Tuple<>(NumberFieldMapper.NumberType.DOUBLE, new BigDecimal("1.7976931348623157E309"), "[double] supports only finite values"), + + new Tuple<>(NumberFieldMapper.NumberType.HALF_FLOAT, Float.NaN, "[half_float] supports only finite values"), + new Tuple<>(NumberFieldMapper.NumberType.FLOAT, Float.NaN, "[float] supports only finite values"), + new Tuple<>(NumberFieldMapper.NumberType.DOUBLE, Double.NaN, "[double] supports only finite values"), + + new Tuple<>(NumberFieldMapper.NumberType.HALF_FLOAT, Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), + new Tuple<>(NumberFieldMapper.NumberType.FLOAT, Float.POSITIVE_INFINITY, "[float] supports only finite values"), + new Tuple<>(NumberFieldMapper.NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values") + ); + + for (Tuple item: inputs) { + try { + item.type.parse(item.value, false); + fail("Mapper parsing exception expected for [" + item.type + "] with value [" + item.value + "]"); + } catch (IllegalArgumentException e) { + assertThat("Incorrect error message for [" + item.type + "] with value [" + item.value + "]", + e.getMessage(), containsString(item.message)); } - }); + } + } + + private static class Tuple { + final K type; + final V value; + final E message; + + Tuple(K type, V value, E message) { + this.type = type; + this.value = value; + this.message = message; + } + } + + private void parseRequest(String type, BytesReference content) throws IOException { + createDocumentMapper(type).parse(SourceToParse.source("test", "type", "1", content, XContentType.JSON)); } private DocumentMapper createDocumentMapper(String type) throws IOException { @@ -352,12 +420,7 @@ private DocumentMapper createDocumentMapper(String type) throws IOException { return parser.parse("type", new CompressedXContent(mapping)); } - private SourceToParse createIndexRequest(String value) throws IOException { - return SourceToParse.source("test", "type", "1", XContentFactory.jsonBuilder() - .startObject() - .field("field", value) - .endObject() - .bytes(), - XContentType.JSON); + private BytesReference createIndexRequest(Object value) throws IOException { + return XContentFactory.jsonBuilder().startObject().field("field", value).endObject().bytes(); } } From ecf34245aa288bc1d8ac362f3c8e3bbcdea78598 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 23 Jul 2017 11:41:37 +0300 Subject: [PATCH 05/16] fix checkstyle errors --- .../index/mapper/NumberFieldMapperTests.java | 45 ++------------ .../index/mapper/NumberFieldTypeTests.java | 61 ++++++++++++++++++- 2 files changed, 63 insertions(+), 43 deletions(-) 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 16ceaf133181d..71a42c5b02dcd 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -29,7 +29,10 @@ import java.io.IOException; import java.math.BigDecimal; import java.math.BigInteger; -import java.util.*; +import java.util.List; +import java.util.Arrays; +import java.util.HashSet; + import static org.hamcrest.Matchers.containsString; @@ -348,46 +351,6 @@ public void testOutOfRangeValuesFromRequest() throws IOException { } } - public void testOutOfRangeValuesFromObject() throws IOException { - final List> inputs = Arrays.asList( - new Tuple<>(NumberFieldMapper.NumberType.BYTE, "128", "Value out of range"), - new Tuple<>(NumberFieldMapper.NumberType.SHORT, "32768", "Value out of range"), - new Tuple<>(NumberFieldMapper.NumberType.INTEGER, "2147483648", "For input string"), - new Tuple<>(NumberFieldMapper.NumberType.LONG, "9223372036854775808", "For input string"), - - new Tuple<>(NumberFieldMapper.NumberType.BYTE, 128, "is out of range for a byte"), - new Tuple<>(NumberFieldMapper.NumberType.SHORT, 32768, "is out of range for a short"), - new Tuple<>(NumberFieldMapper.NumberType.INTEGER, 2147483648L, "is out of range for an integer"), - new Tuple<>(NumberFieldMapper.NumberType.LONG, new BigInteger("92233720368547758080"), " is out of range for a long"), - - new Tuple<>(NumberFieldMapper.NumberType.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), - new Tuple<>(NumberFieldMapper.NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), - new Tuple<>(NumberFieldMapper.NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"), - - new Tuple<>(NumberFieldMapper.NumberType.HALF_FLOAT, 65504.1, "[half_float] supports only finite values"), - new Tuple<>(NumberFieldMapper.NumberType.FLOAT, 3.4028235E39, "[float] supports only finite values"), - new Tuple<>(NumberFieldMapper.NumberType.DOUBLE, new BigDecimal("1.7976931348623157E309"), "[double] supports only finite values"), - - new Tuple<>(NumberFieldMapper.NumberType.HALF_FLOAT, Float.NaN, "[half_float] supports only finite values"), - new Tuple<>(NumberFieldMapper.NumberType.FLOAT, Float.NaN, "[float] supports only finite values"), - new Tuple<>(NumberFieldMapper.NumberType.DOUBLE, Double.NaN, "[double] supports only finite values"), - - new Tuple<>(NumberFieldMapper.NumberType.HALF_FLOAT, Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), - new Tuple<>(NumberFieldMapper.NumberType.FLOAT, Float.POSITIVE_INFINITY, "[float] supports only finite values"), - new Tuple<>(NumberFieldMapper.NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values") - ); - - for (Tuple item: inputs) { - try { - item.type.parse(item.value, false); - fail("Mapper parsing exception expected for [" + item.type + "] with value [" + item.value + "]"); - } catch (IllegalArgumentException e) { - assertThat("Incorrect error message for [" + item.type + "] with value [" + item.value + "]", - e.getMessage(), containsString(item.message)); - } - } - } - private static class Tuple { final K type; final V value; 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 e4d95441b2573..a03d83c1ee23c 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java @@ -43,9 +43,14 @@ import org.junit.Before; import java.io.IOException; +import java.math.BigDecimal; +import java.math.BigInteger; import java.util.Arrays; +import java.util.List; import java.util.function.Supplier; +import static org.hamcrest.Matchers.containsString; + public class NumberFieldTypeTests extends FieldTypeTestCase { NumberType type; @@ -266,8 +271,8 @@ public void testHalfFloatRange() throws IOException { IndexSearcher searcher = newSearcher(reader); final int numQueries = 1000; for (int i = 0; i < numQueries; ++i) { - float l = (randomFloat() * 2 - 1) * 70000; - float u = (randomFloat() * 2 - 1) * 70000; + float l = (randomFloat() * 2 - 1) * 65504; + float u = (randomFloat() * 2 - 1) * 65504; boolean includeLower = randomBoolean(); boolean includeUpper = randomBoolean(); Query floatQ = NumberFieldMapper.NumberType.FLOAT.rangeQuery("float", l, u, includeLower, includeUpper, false); @@ -348,4 +353,56 @@ public void doTestDocValueRangeQueries(NumberType type, Supplier valueSu reader.close(); dir.close(); } + + public void testParseOutOfRangeValues() throws IOException { + final List> inputs = Arrays.asList( + new Tuple<>(NumberType.BYTE, "128", "Value out of range"), + new Tuple<>(NumberType.SHORT, "32768", "Value out of range"), + new Tuple<>(NumberType.INTEGER, "2147483648", "For input string"), + new Tuple<>(NumberType.LONG, "9223372036854775808", "For input string"), + + new Tuple<>(NumberType.BYTE, 128, "is out of range for a byte"), + new Tuple<>(NumberType.SHORT, 32768, "is out of range for a short"), + new Tuple<>(NumberType.INTEGER, 2147483648L, "is out of range for an integer"), + new Tuple<>(NumberType.LONG, new BigInteger("92233720368547758080"), " is out of range for a long"), + + new Tuple<>(NumberType.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), + new Tuple<>(NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), + new Tuple<>(NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"), + + new Tuple<>(NumberType.HALF_FLOAT, 65504.1, "[half_float] supports only finite values"), + new Tuple<>(NumberType.FLOAT, 3.4028235E39, "[float] supports only finite values"), + new Tuple<>(NumberType.DOUBLE, new BigDecimal("1.7976931348623157E309"), "[double] supports only finite values"), + + new Tuple<>(NumberType.HALF_FLOAT, Float.NaN, "[half_float] supports only finite values"), + new Tuple<>(NumberType.FLOAT, Float.NaN, "[float] supports only finite values"), + new Tuple<>(NumberType.DOUBLE, Double.NaN, "[double] supports only finite values"), + + new Tuple<>(NumberType.HALF_FLOAT, Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), + new Tuple<>(NumberType.FLOAT, Float.POSITIVE_INFINITY, "[float] supports only finite values"), + new Tuple<>(NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values") + ); + + for (Tuple item: inputs) { + try { + item.type.parse(item.value, false); + fail("Parsing exception expected for [" + item.type + "] with value [" + item.value + "]"); + } catch (IllegalArgumentException e) { + assertThat("Incorrect error message for [" + item.type + "] with value [" + item.value + "]", + e.getMessage(), containsString(item.message)); + } + } + } + + private static class Tuple { + final K type; + final V value; + final E message; + + Tuple(K type, V value, E message) { + this.type = type; + this.value = value; + this.message = message; + } + } } From b5d231d1be6bbd97439956a0554b6584bdd91520 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 23 Jul 2017 20:19:15 +0300 Subject: [PATCH 06/16] minor renaming --- .../index/mapper/NumberFieldMapper.java | 6 +- .../index/mapper/NumberFieldMapperTests.java | 52 +++++++------- .../index/mapper/NumberFieldTypeTests.java | 71 ++++++++++--------- 3 files changed, 68 insertions(+), 61 deletions(-) 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 76947079a2184..5fa62d599e9fb 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -251,7 +251,7 @@ private void validateParsed(Float value) { || value > 65504 || !Float.isFinite(HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(value))) ) { - throw new IllegalArgumentException("[half_float] supports only finite values, but [" + value + "] given"); + throw new IllegalArgumentException("[half_float] supports only finite values, but got [" + value + "]"); } } }, @@ -341,7 +341,7 @@ public List createFields(String name, Number value, private void validateParsed(Float value) { if (value.isInfinite() || value.isNaN()) { - throw new IllegalArgumentException("[float] supports only finite values, but [" + value + "] given"); + throw new IllegalArgumentException("[float] supports only finite values, but got [" + value + "]"); } } }, @@ -431,7 +431,7 @@ public List createFields(String name, Number value, private void validateParsed(Double value) { if (value.isInfinite() || value.isNaN()) { - throw new IllegalArgumentException("[double] supports only finite values, but [" + value + "] given"); + throw new IllegalArgumentException("[double] supports only finite values, but got [" + value + "]"); } } }, 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 71a42c5b02dcd..ffe6e7386774e 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -27,13 +27,10 @@ import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; -import java.math.BigDecimal; -import java.math.BigInteger; import java.util.List; import java.util.Arrays; import java.util.HashSet; - import static org.hamcrest.Matchers.containsString; public class NumberFieldMapperTests extends AbstractNumericFieldMapperTestCase { @@ -320,27 +317,27 @@ public void testEmptyName() throws IOException { } } - public void testOutOfRangeValuesFromRequest() throws IOException { - final List> inputs = Arrays.asList( - new Tuple<>("half_float", "65504.1", "[half_float] supports only finite values"), - new Tuple<>("float", "3.4028235E39", "[float] supports only finite values"), - new Tuple<>("double", "1.7976931348623157E309", "[double] supports only finite values"), + public void testOutOfRangeValues() throws IOException { + final List> inputs = Arrays.asList( + Triple.of("byte", "128", "is out of range for a byte"), + Triple.of("short", "32768", "is out of range for a short"), + Triple.of("integer", "2147483648", "For input string"), + Triple.of("long", "92233720368547758080", "For input string"), - new Tuple<>("byte", "128", "is out of range for a byte"), - new Tuple<>("short", "32768", "is out of range for a short"), - new Tuple<>("integer", "2147483648", "For input string"), - new Tuple<>("long", "92233720368547758080", "For input string"), + Triple.of("half_float", "65504.1", "[half_float] supports only finite values"), + Triple.of("float", "3.4028235E39", "[float] supports only finite values"), + Triple.of("double", "1.7976931348623157E309", "[double] supports only finite values"), - new Tuple<>("half_float", Float.NaN, "[half_float] supports only finite values"), - new Tuple<>("float", Float.NaN, "[float] supports only finite values"), - new Tuple<>("double", Double.NaN, "[double] supports only finite values"), + Triple.of("half_float", Float.NaN, "[half_float] supports only finite values"), + Triple.of("float", Float.NaN, "[float] supports only finite values"), + Triple.of("double", Double.NaN, "[double] supports only finite values"), - new Tuple<>("half_float", Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), - new Tuple<>("float", Float.POSITIVE_INFINITY, "[float] supports only finite values"), - new Tuple<>("double", Double.POSITIVE_INFINITY, "[double] supports only finite values") + Triple.of("half_float", Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), + Triple.of("float", Float.POSITIVE_INFINITY, "[float] supports only finite values"), + Triple.of("double", Double.POSITIVE_INFINITY, "[double] supports only finite values") ); - for(Tuple item: inputs) { + for(Triple item: inputs) { try { parseRequest(item.type, createIndexRequest(item.value)); fail("Mapper parsing exception expected for [" + item.type + "] with value [" + item.value + "]"); @@ -351,15 +348,20 @@ public void testOutOfRangeValuesFromRequest() throws IOException { } } - private static class Tuple { + private static class Triple { + final K type; final V value; - final E message; + final M message; + + static Triple of(K t, V v, M m) { + return new Triple<>(t, v, m); + } - Tuple(K type, V value, E message) { - this.type = type; - this.value = value; - this.message = message; + Triple(K t, V v, M m) { + type = t; + value = v; + message = m; } } 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 a03d83c1ee23c..603255a8137ce 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java @@ -355,35 +355,35 @@ public void doTestDocValueRangeQueries(NumberType type, Supplier valueSu } public void testParseOutOfRangeValues() throws IOException { - final List> inputs = Arrays.asList( - new Tuple<>(NumberType.BYTE, "128", "Value out of range"), - new Tuple<>(NumberType.SHORT, "32768", "Value out of range"), - new Tuple<>(NumberType.INTEGER, "2147483648", "For input string"), - new Tuple<>(NumberType.LONG, "9223372036854775808", "For input string"), - - new Tuple<>(NumberType.BYTE, 128, "is out of range for a byte"), - new Tuple<>(NumberType.SHORT, 32768, "is out of range for a short"), - new Tuple<>(NumberType.INTEGER, 2147483648L, "is out of range for an integer"), - new Tuple<>(NumberType.LONG, new BigInteger("92233720368547758080"), " is out of range for a long"), - - new Tuple<>(NumberType.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), - new Tuple<>(NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), - new Tuple<>(NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"), - - new Tuple<>(NumberType.HALF_FLOAT, 65504.1, "[half_float] supports only finite values"), - new Tuple<>(NumberType.FLOAT, 3.4028235E39, "[float] supports only finite values"), - new Tuple<>(NumberType.DOUBLE, new BigDecimal("1.7976931348623157E309"), "[double] supports only finite values"), - - new Tuple<>(NumberType.HALF_FLOAT, Float.NaN, "[half_float] supports only finite values"), - new Tuple<>(NumberType.FLOAT, Float.NaN, "[float] supports only finite values"), - new Tuple<>(NumberType.DOUBLE, Double.NaN, "[double] supports only finite values"), - - new Tuple<>(NumberType.HALF_FLOAT, Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), - new Tuple<>(NumberType.FLOAT, Float.POSITIVE_INFINITY, "[float] supports only finite values"), - new Tuple<>(NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values") + final List> inputs = Arrays.asList( + Triple.of(NumberType.BYTE, "128", "Value out of range"), + Triple.of(NumberType.SHORT, "32768", "Value out of range"), + Triple.of(NumberType.INTEGER, "2147483648", "For input string"), + Triple.of(NumberType.LONG, "9223372036854775808", "For input string"), + + Triple.of(NumberType.BYTE, 128, "is out of range for a byte"), + Triple.of(NumberType.SHORT, 32768, "is out of range for a short"), + Triple.of(NumberType.INTEGER, 2147483648L, "is out of range for an integer"), + Triple.of(NumberType.LONG, new BigInteger("92233720368547758080"), " is out of range for a long"), + + Triple.of(NumberType.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), + Triple.of(NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), + Triple.of(NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"), + + Triple.of(NumberType.HALF_FLOAT, 65504.1, "[half_float] supports only finite values"), + Triple.of(NumberType.FLOAT, 3.4028235E39, "[float] supports only finite values"), + Triple.of(NumberType.DOUBLE, new BigDecimal("1.7976931348623157E309"), "[double] supports only finite values"), + + Triple.of(NumberType.HALF_FLOAT, Float.NaN, "[half_float] supports only finite values"), + Triple.of(NumberType.FLOAT, Float.NaN, "[float] supports only finite values"), + Triple.of(NumberType.DOUBLE, Double.NaN, "[double] supports only finite values"), + + Triple.of(NumberType.HALF_FLOAT, Float.POSITIVE_INFINITY, "[half_float] supports only finite values"), + Triple.of(NumberType.FLOAT, Float.POSITIVE_INFINITY, "[float] supports only finite values"), + Triple.of(NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values") ); - for (Tuple item: inputs) { + for (Triple item: inputs) { try { item.type.parse(item.value, false); fail("Parsing exception expected for [" + item.type + "] with value [" + item.value + "]"); @@ -394,15 +394,20 @@ public void testParseOutOfRangeValues() throws IOException { } } - private static class Tuple { + private static class Triple { + final K type; final V value; - final E message; + final M message; + + static Triple of(K t, V v, M m) { + return new Triple<>(t, v, m); + } - Tuple(K type, V value, E message) { - this.type = type; - this.value = value; - this.message = message; + Triple(K t, V v, M m) { + type = t; + value = v; + message = m; } } } From 7423c4df4398d63ff098e591cb6a3cd5b7968172 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Thu, 27 Jul 2017 15:14:21 +0300 Subject: [PATCH 07/16] comments for disabled test --- .../org/elasticsearch/index/mapper/NumberFieldMapperTests.java | 3 ++- .../org/elasticsearch/index/mapper/NumberFieldTypeTests.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) 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 da2150421e2d5..522bc3d4544c7 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -345,10 +345,11 @@ public void testEmptyName() throws IOException { public void testOutOfRangeValues() throws IOException { final List> inputs = Arrays.asList( + // TODO fix min/max value validation for short, integer and long and uncomment tests 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", "For input string"), - OutOfRangeSpec.of(NumberType.LONG, "9223372036854775808", "For input string"), + //OutOfRangeSpec.of(NumberType.LONG, "9223372036854775808", "For input string"), OutOfRangeSpec.of(NumberType.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), OutOfRangeSpec.of(NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), 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 fdab23232e593..dce69243c3bac 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java @@ -390,8 +390,9 @@ public void doTestDocValueRangeQueries(NumberType type, Supplier valueSu public void testParseOutOfRangeValues() throws IOException { final List> inputs = Arrays.asList( + // TODO fix min/max value validation for long and uncomment tests OutOfRangeSpec.of(NumberType.BYTE, "128", "out of range for a byte"), - OutOfRangeSpec.of(NumberType.SHORT, "32768", "out of range for a short"), //@todo fix this value + OutOfRangeSpec.of(NumberType.SHORT, "32768", "out of range for a short"), OutOfRangeSpec.of(NumberType.INTEGER, "2147483648", "out of range for an integer"), //OutOfRangeSpec.of(NumberType.LONG, "9223372036854775808", "out of range for a long"), From bced91eabf0e18a23ce7e78a6e19fb7557289c8c Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Fri, 28 Jul 2017 11:19:01 +0300 Subject: [PATCH 08/16] tests for out of range values for long --- .../index/mapper/NumberFieldMapper.java | 40 +++++++++++-------- .../index/mapper/NumberFieldMapperTests.java | 2 +- .../index/mapper/NumberFieldTypeTests.java | 27 ++++++++++++- 3 files changed, 50 insertions(+), 19 deletions(-) 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 5bbcd4794e1d2..c78a4620aa379 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -51,6 +51,8 @@ import org.joda.time.DateTimeZone; import java.io.IOException; +import java.math.BigDecimal; +import java.math.BigInteger; import java.util.ArrayList; import java.util.Arrays; import java.util.Iterator; @@ -646,34 +648,38 @@ public List createFields(String name, Number value, } }, LONG("long", NumericType.LONG) { + + private final BigInteger MIN_VALUE = BigInteger.valueOf(Long.MIN_VALUE); + private final BigInteger MAX_VALUE = BigInteger.valueOf(Long.MAX_VALUE); + @Override Long parse(Object value, boolean coerce) { - double doubleValue = objectToDouble(value); + // longs need special handling so we don't lose precision while parsing + String stringValue = (value instanceof BytesRef) ? ((BytesRef) value).utf8ToString() : value.toString(); - if (doubleValue < Long.MIN_VALUE || doubleValue > Long.MAX_VALUE) { - throw new IllegalArgumentException("Value [" + value + "] is out of range for a long"); - } - if (!coerce && doubleValue % 1 != 0) { - throw new IllegalArgumentException("Value [" + value + "] has a decimal part"); - } + BigDecimal bigDecimalValue = new BigDecimal(stringValue); + final BigInteger bigIntegerValue; - if (value instanceof Number) { - return ((Number) value).longValue(); + if (!coerce) { + try { + bigIntegerValue = bigDecimalValue.toBigIntegerExact(); + } catch (ArithmeticException e) { + throw new IllegalArgumentException("Value [" + stringValue + "] has a decimal part"); + } + } else { + bigIntegerValue = bigDecimalValue.toBigInteger(); } - // 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); + if (bigIntegerValue.compareTo(MAX_VALUE) == 1 || bigIntegerValue.compareTo(MIN_VALUE) == -1) { + throw new IllegalArgumentException("Value [" + value + "] is out of range for a long"); } + return bigIntegerValue.longValue(); } @Override Long parse(XContentParser parser, boolean coerce) throws IOException { - return parser.longValue(coerce); + parser.longValue(coerce); // validation in parser + return parse(parser.objectText().toString(), coerce); } @Override 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 522bc3d4544c7..8fa1a7b3f88d3 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -349,7 +349,7 @@ public void testOutOfRangeValues() throws IOException { 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", "For input string"), - //OutOfRangeSpec.of(NumberType.LONG, "9223372036854775808", "For input string"), + OutOfRangeSpec.of(NumberType.LONG, "9223372036854775808", "out of range for a long"), OutOfRangeSpec.of(NumberType.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), OutOfRangeSpec.of(NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"), 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 dce69243c3bac..7a74f25f59c76 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java @@ -46,7 +46,6 @@ import java.io.IOException; import java.math.BigDecimal; -import java.math.BigInteger; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.List; @@ -389,6 +388,7 @@ public void doTestDocValueRangeQueries(NumberType type, Supplier valueSu } public void testParseOutOfRangeValues() throws IOException { + final List> inputs = Arrays.asList( // TODO fix min/max value validation for long and uncomment tests OutOfRangeSpec.of(NumberType.BYTE, "128", "out of range for a byte"), @@ -429,6 +429,31 @@ public void testParseOutOfRangeValues() throws IOException { } } + public void testLongOutOfRange() { + final List> inputs = Arrays.asList( + OutOfRangeSpec.of(NumberType.LONG, "9223372036854775806.99", "has a decimal part"), + OutOfRangeSpec.of(NumberType.LONG, "9223372036854775807.01", "has a decimal part"), + OutOfRangeSpec.of(NumberType.LONG, "9223372036854775808", "out of range for a long"), + + OutOfRangeSpec.of(NumberType.LONG, new BigDecimal("9223372036854775806.99"), "has a decimal part"), + OutOfRangeSpec.of(NumberType.LONG, new BigDecimal("9223372036854775807.01"), "has a decimal part"), + OutOfRangeSpec.of(NumberType.LONG, new BigDecimal("9223372036854775808"), "out of range for a long"), + + // lost precision, value is intended to be below Long.MAX_VALUE + OutOfRangeSpec.of(NumberType.LONG, 9223372036854775806d, "out of range for a long") + ); + + for (OutOfRangeSpec item: inputs) { + try { + item.type.parse(item.value, false); + fail("Parsing exception expected"); + } catch (IllegalArgumentException e) { + assertThat("Incorrect error message for value [" + item.value + "]", + e.getMessage(), containsString(item.message)); + } + } + } + static class OutOfRangeSpec { final NumberType type; From 7e07a6331ddf1ce4551c1da3a2860c4f0bdab26e Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sat, 29 Jul 2017 07:15:39 +0300 Subject: [PATCH 09/16] min/max validation for short and integer numbertypes --- .../index/mapper/NumberFieldMapper.java | 18 +++--- .../index/mapper/NumberFieldMapperTests.java | 5 +- .../index/mapper/NumberFieldTypeTests.java | 60 +++++++++---------- 3 files changed, 39 insertions(+), 44 deletions(-) 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 c78a4620aa379..5e8ad823c498b 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -505,7 +505,8 @@ Short parse(Object value, boolean coerce) { @Override Short parse(XContentParser parser, boolean coerce) throws IOException { - return parser.shortValue(coerce); + parser.shortValue(coerce); // keep initial validation + return parse(parser.objectText().toString(), coerce); } @Override @@ -557,7 +558,8 @@ Integer parse(Object value, boolean coerce) { @Override Integer parse(XContentParser parser, boolean coerce) throws IOException { - return parser.intValue(coerce); + parser.intValue(coerce); // keep initial validation + return parse(parser.objectText().toString(), coerce); } @Override @@ -660,14 +662,10 @@ Long parse(Object value, boolean coerce) { BigDecimal bigDecimalValue = new BigDecimal(stringValue); final BigInteger bigIntegerValue; - if (!coerce) { - try { - bigIntegerValue = bigDecimalValue.toBigIntegerExact(); - } catch (ArithmeticException e) { - throw new IllegalArgumentException("Value [" + stringValue + "] has a decimal part"); - } - } else { - bigIntegerValue = bigDecimalValue.toBigInteger(); + try { + bigIntegerValue = coerce ? bigDecimalValue.toBigInteger() : bigDecimalValue.toBigIntegerExact(); + } catch (ArithmeticException e) { + throw new IllegalArgumentException("Value [" + stringValue + "] has a decimal part"); } if (bigIntegerValue.compareTo(MAX_VALUE) == 1 || bigIntegerValue.compareTo(MIN_VALUE) == -1) { 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 8fa1a7b3f88d3..30eb0d6ba34cf 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -345,10 +345,9 @@ public void testEmptyName() throws IOException { public void testOutOfRangeValues() throws IOException { final List> inputs = Arrays.asList( - // TODO fix min/max value validation for short, integer and long and uncomment tests 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", "For input string"), + 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.HALF_FLOAT, "65504.1", "[half_float] supports only finite values"), 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 7a74f25f59c76..05a277a46f1de 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 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; @@ -390,23 +393,26 @@ public void doTestDocValueRangeQueries(NumberType type, Supplier valueSu public void testParseOutOfRangeValues() throws IOException { final List> inputs = Arrays.asList( - // TODO fix min/max value validation for long and uncomment tests OutOfRangeSpec.of(NumberType.BYTE, "128", "out of range for a byte"), - OutOfRangeSpec.of(NumberType.SHORT, "32768", "out of range for a short"), - OutOfRangeSpec.of(NumberType.INTEGER, "2147483648", "out of range for an integer"), - //OutOfRangeSpec.of(NumberType.LONG, "9223372036854775808", "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 for a short"), OutOfRangeSpec.of(NumberType.SHORT, 327684, "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.LONG, new BigInteger("9223372036854775808"), " is out of range for a long"), + + 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"), + // lost precision, value is intended to be below Long.MAX_VALUE + OutOfRangeSpec.of(NumberType.LONG, 9223372036854775806d, "out of range for a long"), OutOfRangeSpec.of(NumberType.HALF_FLOAT, "65504.1", "[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"), - OutOfRangeSpec.of(NumberType.HALF_FLOAT, 65504.1, "[half_float] supports only finite values"), - OutOfRangeSpec.of(NumberType.FLOAT, 3.4028235E39, "[float] supports only finite values"), + OutOfRangeSpec.of(NumberType.HALF_FLOAT, 65504.1D, "[half_float] supports only finite values"), + OutOfRangeSpec.of(NumberType.FLOAT, 3.4028235E39D, "[float] supports only finite values"), OutOfRangeSpec.of(NumberType.DOUBLE, new BigDecimal("1.7976931348623157E309"), "[double] supports only finite values"), OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.NaN, "[half_float] supports only finite values"), @@ -429,29 +435,21 @@ public void testParseOutOfRangeValues() throws IOException { } } - public void testLongOutOfRange() { - final List> inputs = Arrays.asList( - OutOfRangeSpec.of(NumberType.LONG, "9223372036854775806.99", "has a decimal part"), - OutOfRangeSpec.of(NumberType.LONG, "9223372036854775807.01", "has a decimal part"), - OutOfRangeSpec.of(NumberType.LONG, "9223372036854775808", "out of range for a long"), - - OutOfRangeSpec.of(NumberType.LONG, new BigDecimal("9223372036854775806.99"), "has a decimal part"), - OutOfRangeSpec.of(NumberType.LONG, new BigDecimal("9223372036854775807.01"), "has a decimal part"), - OutOfRangeSpec.of(NumberType.LONG, new BigDecimal("9223372036854775808"), "out of range for a long"), - - // lost precision, value is intended to be below Long.MAX_VALUE - OutOfRangeSpec.of(NumberType.LONG, 9223372036854775806d, "out of range for a long") - ); - - for (OutOfRangeSpec item: inputs) { - try { - item.type.parse(item.value, false); - fail("Parsing exception expected"); - } catch (IllegalArgumentException e) { - assertThat("Incorrect error message for value [" + item.value + "]", - e.getMessage(), containsString(item.message)); - } - } + public void testBorderValues() { + final Map inputs = new HashMap<>(); + inputs.put(NumberType.BYTE, "127"); + inputs.put(NumberType.BYTE, BigInteger.valueOf(127)); + inputs.put(NumberType.SHORT, "32767"); + inputs.put(NumberType.SHORT, BigInteger.valueOf(32767)); + inputs.put(NumberType.INTEGER, "2147483647"); + inputs.put(NumberType.INTEGER, BigInteger.valueOf(2147483647)); + inputs.put(NumberType.LONG, "9223372036854775806"); + inputs.put(NumberType.LONG, BigInteger.valueOf(9223372036854775806L)); + + inputs.forEach((type, value) -> { + Number result = type.parse(value, false); + assertEquals(value.toString(), result.toString()); + }); } static class OutOfRangeSpec { From 11ce7c86114735f58a6ac9b1e4115171bbc75825 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 30 Jul 2017 07:14:26 +0300 Subject: [PATCH 10/16] tests for byte/short/integer/long removed and will be added in separate PR --- .../index/mapper/NumberFieldMapperTests.java | 6 ------ .../index/mapper/NumberFieldTypeTests.java | 11 ----------- 2 files changed, 17 deletions(-) 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 522bc3d4544c7..f7e791cfb1c2c 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -345,12 +345,6 @@ public void testEmptyName() throws IOException { public void testOutOfRangeValues() throws IOException { final List> inputs = Arrays.asList( - // TODO fix min/max value validation for short, integer and long and uncomment tests - 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", "For input string"), - //OutOfRangeSpec.of(NumberType.LONG, "9223372036854775808", "For input string"), - OutOfRangeSpec.of(NumberType.HALF_FLOAT, "65504.1", "[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/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java index dce69243c3bac..5c534646aa131 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java @@ -390,17 +390,6 @@ public void doTestDocValueRangeQueries(NumberType type, Supplier valueSu public void testParseOutOfRangeValues() throws IOException { final List> inputs = Arrays.asList( - // TODO fix min/max value validation for long and uncomment tests - OutOfRangeSpec.of(NumberType.BYTE, "128", "out of range for a byte"), - OutOfRangeSpec.of(NumberType.SHORT, "32768", "out of range for a short"), - OutOfRangeSpec.of(NumberType.INTEGER, "2147483648", "out of range for an integer"), - //OutOfRangeSpec.of(NumberType.LONG, "9223372036854775808", "out of range for a long"), - - OutOfRangeSpec.of(NumberType.BYTE, 128, "is out of range for a byte"), - OutOfRangeSpec.of(NumberType.SHORT, 327684, "is out of range for a short"), - OutOfRangeSpec.of(NumberType.INTEGER, 2147483648L, "is out of range for an integer"), - //OutOfRangeSpec.of(NumberType.LONG, new BigInteger("9223372036854775808"), " is out of range for a long"), - OutOfRangeSpec.of(NumberType.HALF_FLOAT, "65504.1", "[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"), From 3902911f2a11051156735bcfffd8146b7e30eda0 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 30 Jul 2017 07:16:07 +0300 Subject: [PATCH 11/16] remove unused import --- .../org/elasticsearch/index/mapper/NumberFieldTypeTests.java | 1 - 1 file changed, 1 deletion(-) 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 5c534646aa131..66db603077c52 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java @@ -46,7 +46,6 @@ import java.io.IOException; import java.math.BigDecimal; -import java.math.BigInteger; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.List; From 1a32b0539c014b53131a30b76d0248d75e23e558 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Thu, 10 Aug 2017 12:44:17 +0300 Subject: [PATCH 12/16] tests for negative values --- .../index/mapper/NumberFieldMapper.java | 1 - .../index/mapper/NumberFieldMapperTests.java | 5 ++++ .../index/mapper/NumberFieldTypeTests.java | 25 ++++--------------- 3 files changed, 10 insertions(+), 21 deletions(-) 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 1d2706d8e157f..2b4370c922fad 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -181,7 +181,6 @@ Float parse(Object value, boolean coerce) { @Override Float parse(XContentParser parser, boolean coerce) throws IOException { float parsed = parser.floatValue(coerce); - validateParsed(parsed); return parsed; } 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 961992a924883..4acc72bd76cd7 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -350,6 +350,11 @@ public void testOutOfRangeValues() throws IOException { 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.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/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java index dc1e0daaa2c71..58ece7b507388 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java @@ -394,17 +394,19 @@ 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, 327684, "is 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"), - // lost precision, value is intended to be below Long.MAX_VALUE - OutOfRangeSpec.of(NumberType.LONG, 9223372036854775806d, "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"), @@ -442,23 +444,6 @@ public void testParseOutOfRangeValues() throws IOException { } } - public void testBorderValues() { - final Map inputs = new HashMap<>(); - inputs.put(NumberType.BYTE, "127"); - inputs.put(NumberType.BYTE, BigInteger.valueOf(127)); - inputs.put(NumberType.SHORT, "32767"); - inputs.put(NumberType.SHORT, BigInteger.valueOf(32767)); - inputs.put(NumberType.INTEGER, "2147483647"); - inputs.put(NumberType.INTEGER, BigInteger.valueOf(2147483647)); - inputs.put(NumberType.LONG, "9223372036854775806"); - inputs.put(NumberType.LONG, BigInteger.valueOf(9223372036854775806L)); - - inputs.forEach((type, value) -> { - Number result = type.parse(value, false); - assertEquals(value.toString(), result.toString()); - }); - } - static class OutOfRangeSpec { final NumberType type; From efe2adf1e8205166e46d2f65ef1e90b0da742d58 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sat, 12 Aug 2017 10:07:18 +0300 Subject: [PATCH 13/16] min/max validation for short/int/long --- .../support/AbstractXContentParser.java | 39 ++++++++++++++-- .../index/mapper/NumberFieldMapper.java | 45 +++++++++---------- .../index/mapper/NumberFieldMapperTests.java | 21 ++++++++- 3 files changed, 77 insertions(+), 28 deletions(-) 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 be128b2f21264..830700b2cb790 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 @@ -26,6 +26,8 @@ import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; +import java.math.BigDecimal; +import java.math.BigInteger; import java.util.ArrayList; import java.util.HashMap; import java.util.LinkedHashMap; @@ -42,6 +44,9 @@ public abstract class AbstractXContentParser implements XContentParser { // and change any code that needs to apply an alternative policy. public static final boolean DEFAULT_NUMBER_COERCE_POLICY = true; + private static final BigInteger MIN_LONG_VALUE = BigInteger.valueOf(Long.MIN_VALUE); + private static final BigInteger MAX_LONG_VALUE = BigInteger.valueOf(Long.MAX_VALUE); + private static void checkCoerceString(boolean coerce, Class clazz) { if (!coerce) { //Need to throw type IllegalArgumentException as current catch logic in @@ -133,7 +138,14 @@ public short shortValue(boolean coerce) throws IOException { 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 +164,13 @@ public int intValue(boolean coerce) throws IOException { 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); @@ -176,7 +194,7 @@ public long longValue(boolean coerce) throws IOException { try { return Long.parseLong(stringValue); } catch (NumberFormatException e) { - return (long) Double.parseDouble(stringValue); + return preciseLongValue(stringValue, coerce); } } long result = doLongValue(); @@ -184,6 +202,21 @@ public long longValue(boolean coerce) throws IOException { return result; } + public static long preciseLongValue(String stringValue, boolean coerce) { + BigDecimal bigDecimalValue = new BigDecimal(stringValue); + final BigInteger bigIntegerValue; + + try { + bigIntegerValue = coerce ? bigDecimalValue.toBigInteger() : bigDecimalValue.toBigIntegerExact(); + } catch (ArithmeticException e) { + throw new IllegalArgumentException("Value [" + stringValue + "] has a decimal part"); + } + 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(); + } + protected abstract long doLongValue() throws IOException; @Override 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 2b4370c922fad..e603cb442d0d9 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -43,6 +43,7 @@ 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; @@ -51,8 +52,6 @@ import org.joda.time.DateTimeZone; import java.io.IOException; -import java.math.BigDecimal; -import java.math.BigInteger; import java.util.ArrayList; import java.util.Arrays; import java.util.Iterator; @@ -501,8 +500,7 @@ Short parse(Object value, boolean coerce) { @Override Short parse(XContentParser parser, boolean coerce) throws IOException { - parser.shortValue(coerce); // keep initial validation - return parse(parser.objectText().toString(), coerce); + return parser.shortValue(coerce); } @Override @@ -554,8 +552,7 @@ Integer parse(Object value, boolean coerce) { @Override Integer parse(XContentParser parser, boolean coerce) throws IOException { - parser.intValue(coerce); // keep initial validation - return parse(parser.objectText().toString(), coerce); + return parser.intValue(coerce); } @Override @@ -646,34 +643,35 @@ public List createFields(String name, Number value, } }, LONG("long", NumericType.LONG) { - - private final BigInteger MIN_VALUE = BigInteger.valueOf(Long.MIN_VALUE); - private final BigInteger MAX_VALUE = BigInteger.valueOf(Long.MAX_VALUE); - @Override Long parse(Object value, boolean coerce) { + 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"); + } + if (!coerce && doubleValue % 1 != 0) { + throw new IllegalArgumentException("Value [" + value + "] has a decimal part"); + } + // longs need special handling so we don't lose precision while parsing String stringValue = (value instanceof BytesRef) ? ((BytesRef) value).utf8ToString() : value.toString(); - BigDecimal bigDecimalValue = new BigDecimal(stringValue); - final BigInteger bigIntegerValue; - try { - bigIntegerValue = coerce ? bigDecimalValue.toBigInteger() : bigDecimalValue.toBigIntegerExact(); - } catch (ArithmeticException e) { - throw new IllegalArgumentException("Value [" + stringValue + "] has a decimal part"); + return Long.parseLong(stringValue); + } catch (NumberFormatException e) { + return AbstractXContentParser.preciseLongValue(stringValue, coerce); } - - if (bigIntegerValue.compareTo(MAX_VALUE) == 1 || bigIntegerValue.compareTo(MIN_VALUE) == -1) { - throw new IllegalArgumentException("Value [" + value + "] is out of range for a long"); - } - return bigIntegerValue.longValue(); } @Override Long parse(XContentParser parser, boolean coerce) throws IOException { - parser.longValue(coerce); // validation in parser - return parse(parser.objectText().toString(), coerce); + return parser.longValue(coerce); } @Override @@ -840,7 +838,6 @@ private static double objectToDouble(Object value) { return doubleValue; } - } public static final class NumberFieldType extends MappedFieldType { 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 4acc72bd76cd7..de9f43af2468d 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.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; @@ -355,6 +357,16 @@ public void testOutOfRangeValues() throws IOException { 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"), @@ -408,6 +420,13 @@ private DocumentMapper createDocumentMapper(NumberType type) throws IOException } 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(); + } } } From d76e214b75952f517f6e6ba62f961e678f678ec2 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sat, 12 Aug 2017 11:25:34 +0300 Subject: [PATCH 14/16] fix checkstyle errors & return more informative message for malformed long --- .../common/xcontent/support/AbstractXContentParser.java | 4 +++- .../elasticsearch/index/mapper/NumberFieldMapperTests.java | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) 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 830700b2cb790..2fc1949bccffc 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 @@ -203,13 +203,15 @@ public long longValue(boolean coerce) throws IOException { } public static long preciseLongValue(String stringValue, boolean coerce) { - BigDecimal bigDecimalValue = new BigDecimal(stringValue); 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"); 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 de9f43af2468d..65ba83a7ab3f0 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -236,6 +236,7 @@ private void doTestIgnoreMalformed(String type) throws IOException { .bytes(), XContentType.JSON)); MapperParsingException e = expectThrows(MapperParsingException.class, runnable); + assertThat(e.getCause().getMessage(), containsString("For input string: \"a\"")); mapping = XContentFactory.jsonBuilder().startObject().startObject("type") @@ -359,12 +360,12 @@ public void testOutOfRangeValues() throws IOException { 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.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.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"), From cfe946191fb0c6898037ccf1d2799bfd65864688 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Thu, 17 Aug 2017 12:07:01 +0300 Subject: [PATCH 15/16] move toLong from AbstractXContentParser to Numbers --- .../org/elasticsearch/common/Numbers.java | 30 ++++++++++++++++++ .../support/AbstractXContentParser.java | 31 ++----------------- .../index/mapper/NumberFieldMapper.java | 8 ++--- .../elasticsearch/common/NumbersTests.java | 15 +++++++++ 4 files changed, 49 insertions(+), 35 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/Numbers.java b/core/src/main/java/org/elasticsearch/common/Numbers.java index 1735a0dfa6570..2c4d700c92ce3 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 @@ */ 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 static long toLongExact(Number n) { } } + /** 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 2fc1949bccffc..9aae1ca396c12 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,12 +22,11 @@ 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; import java.io.IOException; -import java.math.BigDecimal; -import java.math.BigInteger; import java.util.ArrayList; import java.util.HashMap; import java.util.LinkedHashMap; @@ -44,9 +43,6 @@ public abstract class AbstractXContentParser implements XContentParser { // and change any code that needs to apply an alternative policy. public static final boolean DEFAULT_NUMBER_COERCE_POLICY = true; - private static final BigInteger MIN_LONG_VALUE = BigInteger.valueOf(Long.MIN_VALUE); - private static final BigInteger MAX_LONG_VALUE = BigInteger.valueOf(Long.MAX_VALUE); - private static void checkCoerceString(boolean coerce, Class clazz) { if (!coerce) { //Need to throw type IllegalArgumentException as current catch logic in @@ -189,36 +185,13 @@ public long longValue(boolean coerce) throws IOException { 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 preciseLongValue(stringValue, coerce); - } + return Numbers.toLong(text(), coerce); } long result = doLongValue(); ensureNumberConversion(coerce, result, Long.class); return result; } - public static long preciseLongValue(String stringValue, boolean coerce) { - 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(); - } - protected abstract long doLongValue() throws IOException; @Override 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 e603cb442d0d9..c4d3669b32da3 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.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; @@ -661,12 +662,7 @@ Long parse(Object value, boolean coerce) { // 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 AbstractXContentParser.preciseLongValue(stringValue, coerce); - } + return Numbers.toLong(stringValue, coerce); } @Override diff --git a/core/src/test/java/org/elasticsearch/common/NumbersTests.java b/core/src/test/java/org/elasticsearch/common/NumbersTests.java index e5563993ad5fd..46378ccc9e9fb 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 @@ 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))); From 3d8521b7c7742bbc45c2e76ebed7cd6854029cc7 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Thu, 17 Aug 2017 17:48:35 +0300 Subject: [PATCH 16/16] fix test --- .../org/elasticsearch/index/reindex/ReindexFailureTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 09af00ca36664..f101b12538289 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 void testFailuresCauseAbortDefault() throws Exception { .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\"]")); } }