Skip to content

Commit

Permalink
Reject out of range numbers for float, double and half_float (#25826)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@fred84 @colings86
fred84 authored and colings86 committed Aug 9, 2017
1 parent 15598f2 commit d8ff6e9
Show file tree
Hide file tree
Showing 4 changed files with 208 additions and 20 deletions.
62 changes: 52 additions & 10 deletions core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -231,22 +244,35 @@ public List<Field> createFields(String name, Number value,
}
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();
}
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
Expand Down Expand Up @@ -308,16 +334,26 @@ public List<Field> createFields(String name, Number value,
}
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
Expand Down Expand Up @@ -379,6 +415,12 @@ public List<Field> createFields(String name, Number value,
}
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
Expand Down
42 changes: 34 additions & 8 deletions core/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
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;
Expand Down Expand Up @@ -144,7 +145,7 @@ public Mapper.Builder<?,?> parse(String name, Map<String, Object> node,
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));
Expand All @@ -153,7 +154,7 @@ public Mapper.Builder<?,?> parse(String name, Map<String, Object> node,
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();
}
}
Expand Down Expand Up @@ -207,7 +208,7 @@ public void checkCompatibility(MappedFieldType other, List<String> conflicts, bo
@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) {
Expand All @@ -221,7 +222,7 @@ public Query termsQuery(List<?> values, QueryShardContext context) {
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);
}
Expand All @@ -237,15 +238,15 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower
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);
}
lo = Math.round(Math.ceil(dValue * scalingFactor));
}
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);
}
Expand Down Expand Up @@ -366,7 +367,7 @@ protected void parseCreateField(ParseContext context, List<IndexableField> field
value = null;
} else {
try {
numericValue = NumberFieldMapper.NumberType.DOUBLE.parse(parser, coerce.value());
numericValue = parse(parser, coerce.value());
} catch (IllegalArgumentException e) {
if (ignoreMalformed.value()) {
return;
Expand All @@ -390,7 +391,7 @@ protected void parseCreateField(ParseContext context, List<IndexableField> field
}

if (numericValue == null) {
numericValue = NumberFieldMapper.NumberType.DOUBLE.parse(value, false);
numericValue = parse(value);
}

if (includeInAll) {
Expand Down Expand Up @@ -451,6 +452,31 @@ protected void doXContentBody(XContentBuilder builder, boolean includeDefaults,
}
}

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;
Expand Down
61 changes: 61 additions & 0 deletions core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +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 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;

Expand Down Expand Up @@ -339,4 +343,61 @@ public void testEmptyName() throws IOException {
}
}

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();
}
}
63 changes: 61 additions & 2 deletions core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,14 @@
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;
Expand Down Expand Up @@ -300,8 +304,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);
Expand Down Expand Up @@ -382,4 +386,59 @@ public void doTestDocValueRangeQueries(NumberType type, Supplier<Number> valueSu
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;
}
}
}

0 comments on commit d8ff6e9

Please sign in to comment.