Reject out of range numbers for float, double and half_float (#25826)

* validate half float values

* test upper bound for numeric mapper

* test for upper bound for float, double and half_float

* more tests on NaN and Infinity for NumberFieldMapper

* fix checkstyle errors

* minor renaming

* comments for disabled test

* tests for byte/short/integer/long removed and will be added in separate PR

* remove unused import

* Fix scaledfloat out of range validation message

* 1) delayed autoboxing in numbertype.parse(...)
2) no redudant checks in half_float validation
3) tests with negative values for half_float/float/double
This commit is contained in:
Sergey Galkin 2017-08-09 14:44:57 +03:00 committed by Colin Goodheart-Smithe
parent 15598f2174
commit d8ff6e9831
4 changed files with 208 additions and 20 deletions

View File

@ -162,12 +162,25 @@ public class NumberFieldMapper extends FieldMapper {
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
@ -231,22 +244,35 @@ public class NumberFieldMapper extends FieldMapper {
}
return fields;
}
private void validateParsed(float value) {
if (!Float.isFinite(HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(value)))) {
throw new IllegalArgumentException("[half_float] supports only finite values, but got [" + value + "]");
}
}
},
FLOAT("float", NumericType.FLOAT) {
@Override
Float parse(Object value, boolean coerce) {
final float result;
if (value instanceof Number) {
return ((Number) value).floatValue();
result = ((Number) value).floatValue();
} else {
if (value instanceof BytesRef) {
value = ((BytesRef) value).utf8ToString();
}
result = Float.parseFloat(value.toString());
}
if (value instanceof BytesRef) {
value = ((BytesRef) value).utf8ToString();
}
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
@ -308,16 +334,26 @@ public class NumberFieldMapper extends FieldMapper {
}
return fields;
}
private void validateParsed(float value) {
if (!Float.isFinite(value)) {
throw new IllegalArgumentException("[float] supports only finite values, but got [" + value + "]");
}
}
},
DOUBLE("double", NumericType.DOUBLE) {
@Override
Double parse(Object value, boolean coerce) {
return objectToDouble(value);
double parsed = objectToDouble(value);
validateParsed(parsed);
return parsed;
}
@Override
Double parse(XContentParser parser, boolean coerce) throws IOException {
return parser.doubleValue(coerce);
double parsed = parser.doubleValue(coerce);
validateParsed(parsed);
return parsed;
}
@Override
@ -379,6 +415,12 @@ public class NumberFieldMapper extends FieldMapper {
}
return fields;
}
private void validateParsed(double value) {
if (!Double.isFinite(value)) {
throw new IllegalArgumentException("[double] supports only finite values, but got [" + value + "]");
}
}
},
BYTE("byte", NumericType.BYTE) {
@Override

View File

@ -28,6 +28,7 @@ import org.apache.lucene.index.SortedNumericDocValues;
import org.apache.lucene.search.BoostQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.SortField;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.common.Explicit;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.settings.Setting;
@ -144,7 +145,7 @@ public class ScaledFloatFieldMapper extends FieldMapper {
if (propNode == null) {
throw new MapperParsingException("Property [null_value] cannot be null.");
}
builder.nullValue(NumberFieldMapper.NumberType.DOUBLE.parse(propNode, false));
builder.nullValue(ScaledFloatFieldMapper.parse(propNode));
iterator.remove();
} else if (propName.equals("ignore_malformed")) {
builder.ignoreMalformed(TypeParsers.nodeBooleanValue(name, "ignore_malformed", propNode, parserContext));
@ -153,7 +154,7 @@ public class ScaledFloatFieldMapper extends FieldMapper {
builder.coerce(TypeParsers.nodeBooleanValue(name, "coerce", propNode, parserContext));
iterator.remove();
} else if (propName.equals("scaling_factor")) {
builder.scalingFactor(NumberFieldMapper.NumberType.DOUBLE.parse(propNode, false).doubleValue());
builder.scalingFactor(ScaledFloatFieldMapper.parse(propNode));
iterator.remove();
}
}
@ -207,7 +208,7 @@ public class ScaledFloatFieldMapper extends FieldMapper {
@Override
public Query termQuery(Object value, QueryShardContext context) {
failIfNotIndexed();
double queryValue = NumberFieldMapper.NumberType.DOUBLE.parse(value, false).doubleValue();
double queryValue = parse(value);
long scaledValue = Math.round(queryValue * scalingFactor);
Query query = NumberFieldMapper.NumberType.LONG.termQuery(name(), scaledValue);
if (boost() != 1f) {
@ -221,7 +222,7 @@ public class ScaledFloatFieldMapper extends FieldMapper {
failIfNotIndexed();
List<Long> scaledValues = new ArrayList<>(values.size());
for (Object value : values) {
double queryValue = NumberFieldMapper.NumberType.DOUBLE.parse(value, false).doubleValue();
double queryValue = parse(value);
long scaledValue = Math.round(queryValue * scalingFactor);
scaledValues.add(scaledValue);
}
@ -237,7 +238,7 @@ public class ScaledFloatFieldMapper extends FieldMapper {
failIfNotIndexed();
Long lo = null;
if (lowerTerm != null) {
double dValue = NumberFieldMapper.NumberType.DOUBLE.parse(lowerTerm, false).doubleValue();
double dValue = parse(lowerTerm);
if (includeLower == false) {
dValue = Math.nextUp(dValue);
}
@ -245,7 +246,7 @@ public class ScaledFloatFieldMapper extends FieldMapper {
}
Long hi = null;
if (upperTerm != null) {
double dValue = NumberFieldMapper.NumberType.DOUBLE.parse(upperTerm, false).doubleValue();
double dValue = parse(upperTerm);
if (includeUpper == false) {
dValue = Math.nextDown(dValue);
}
@ -366,7 +367,7 @@ public class ScaledFloatFieldMapper extends FieldMapper {
value = null;
} else {
try {
numericValue = NumberFieldMapper.NumberType.DOUBLE.parse(parser, coerce.value());
numericValue = parse(parser, coerce.value());
} catch (IllegalArgumentException e) {
if (ignoreMalformed.value()) {
return;
@ -390,7 +391,7 @@ public class ScaledFloatFieldMapper extends FieldMapper {
}
if (numericValue == null) {
numericValue = NumberFieldMapper.NumberType.DOUBLE.parse(value, false);
numericValue = parse(value);
}
if (includeInAll) {
@ -451,6 +452,31 @@ public class ScaledFloatFieldMapper extends FieldMapper {
}
}
static Double parse(Object value) {
return objectToDouble(value);
}
private static Double parse(XContentParser parser, boolean coerce) throws IOException {
return parser.doubleValue(coerce);
}
/**
* Converts an Object to a double by checking it against known types first
*/
private static double objectToDouble(Object value) {
double doubleValue;
if (value instanceof Number) {
doubleValue = ((Number) value).doubleValue();
} else if (value instanceof BytesRef) {
doubleValue = Double.parseDouble(((BytesRef) value).utf8ToString());
} else {
doubleValue = Double.parseDouble(value.toString());
}
return doubleValue;
}
private static class ScaledFloatIndexFieldData implements IndexNumericFieldData {
private final IndexNumericFieldData scaledFieldData;

View File

@ -21,11 +21,15 @@ package org.elasticsearch.index.mapper;
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 org.elasticsearch.index.mapper.NumberFieldMapper.NumberType;
import org.elasticsearch.index.mapper.NumberFieldTypeTests.OutOfRangeSpec;
import java.io.IOException;
import java.util.List;
import java.util.Arrays;
import java.util.HashSet;
@ -339,4 +343,61 @@ public class NumberFieldMapperTests extends AbstractNumericFieldMapperTestCase {
}
}
public void testOutOfRangeValues() throws IOException {
final List<OutOfRangeSpec<Object>> inputs = Arrays.asList(
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"),
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"),
OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.NaN, "[half_float] supports only finite values"),
OutOfRangeSpec.of(NumberType.FLOAT, Float.NaN, "[float] supports only finite values"),
OutOfRangeSpec.of(NumberType.DOUBLE, Double.NaN, "[double] supports only finite values"),
OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.POSITIVE_INFINITY, "[half_float] supports only finite values"),
OutOfRangeSpec.of(NumberType.FLOAT, Float.POSITIVE_INFINITY, "[float] supports only finite values"),
OutOfRangeSpec.of(NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values"),
OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.NEGATIVE_INFINITY, "[half_float] supports only finite values"),
OutOfRangeSpec.of(NumberType.FLOAT, Float.NEGATIVE_INFINITY, "[float] supports only finite values"),
OutOfRangeSpec.of(NumberType.DOUBLE, Double.NEGATIVE_INFINITY, "[double] supports only finite values")
);
for(OutOfRangeSpec<Object> item: inputs) {
try {
parseRequest(item.type, createIndexRequest(item.value));
fail("Mapper parsing exception expected for [" + item.type + "] with value [" + item.value + "]");
} catch (MapperParsingException e) {
assertThat("Incorrect error message for [" + item.type + "] with value [" + item.value + "]",
e.getCause().getMessage(), containsString(item.message));
}
}
}
private void parseRequest(NumberType type, BytesReference content) throws IOException {
createDocumentMapper(type).parse(SourceToParse.source("test", "type", "1", content, XContentType.JSON));
}
private DocumentMapper createDocumentMapper(NumberType type) throws IOException {
String mapping = XContentFactory.jsonBuilder()
.startObject()
.startObject("type")
.startObject("properties")
.startObject("field")
.field("type", type.typeName())
.endObject()
.endObject()
.endObject()
.endObject()
.string();
return parser.parse("type", new CompressedXContent(mapping));
}
private BytesReference createIndexRequest(Object value) throws IOException {
return XContentFactory.jsonBuilder().startObject().field("field", value).endObject().bytes();
}
}

View File

@ -45,10 +45,14 @@ import org.hamcrest.Matchers;
import org.junit.Before;
import java.io.IOException;
import java.math.BigDecimal;
import java.nio.charset.StandardCharsets;
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;
@ -300,8 +304,8 @@ public class NumberFieldTypeTests extends FieldTypeTestCase {
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);
@ -382,4 +386,59 @@ public class NumberFieldTypeTests extends FieldTypeTestCase {
reader.close();
dir.close();
}
public void testParseOutOfRangeValues() throws IOException {
final List<OutOfRangeSpec<Object>> inputs = Arrays.asList(
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"),
OutOfRangeSpec.of(NumberType.HALF_FLOAT, 65520f, "[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, -65520f, "[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"),
OutOfRangeSpec.of(NumberType.FLOAT, Float.NaN, "[float] supports only finite values"),
OutOfRangeSpec.of(NumberType.DOUBLE, Double.NaN, "[double] supports only finite values"),
OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.POSITIVE_INFINITY, "[half_float] supports only finite values"),
OutOfRangeSpec.of(NumberType.FLOAT, Float.POSITIVE_INFINITY, "[float] supports only finite values"),
OutOfRangeSpec.of(NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values"),
OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.NEGATIVE_INFINITY, "[half_float] supports only finite values"),
OutOfRangeSpec.of(NumberType.FLOAT, Float.NEGATIVE_INFINITY, "[float] supports only finite values"),
OutOfRangeSpec.of(NumberType.DOUBLE, Double.NEGATIVE_INFINITY, "[double] supports only finite values")
);
for (OutOfRangeSpec<Object> 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));
}
}
}
static class OutOfRangeSpec<V> {
final NumberType type;
final V value;
final String message;
static <V> OutOfRangeSpec<V> of(NumberType t, V v, String m) {
return new OutOfRangeSpec<>(t, v, m);
}
OutOfRangeSpec(NumberType t, V v, String m) {
type = t;
value = v;
message = m;
}
}
}