Skip to content

Commit 5a033ae

Browse files
fred84colings86
fred84
authored and
colings86
committed
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
1 parent 04c9694 commit 5a033ae

File tree

4 files changed

+208
-20
lines changed

4 files changed

+208
-20
lines changed

‎core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java

Lines changed: 52 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -162,12 +162,25 @@ public enum NumberType {
162162
HALF_FLOAT("half_float", NumericType.HALF_FLOAT) {
163163
@Override
164164
Float parse(Object value, boolean coerce) {
165-
return (Float) FLOAT.parse(value, false);
165+
final float result;
166+
167+
if (value instanceof Number) {
168+
result = ((Number) value).floatValue();
169+
} else {
170+
if (value instanceof BytesRef) {
171+
value = ((BytesRef) value).utf8ToString();
172+
}
173+
result = Float.parseFloat(value.toString());
174+
}
175+
validateParsed(result);
176+
return result;
166177
}
167178

168179
@Override
169180
Float parse(XContentParser parser, boolean coerce) throws IOException {
170-
return parser.floatValue(coerce);
181+
float parsed = parser.floatValue(coerce);
182+
validateParsed(parsed);
183+
return parsed;
171184
}
172185

173186
@Override
@@ -231,22 +244,35 @@ public List<Field> createFields(String name, Number value,
231244
}
232245
return fields;
233246
}
247+
248+
private void validateParsed(float value) {
249+
if (!Float.isFinite(HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(value)))) {
250+
throw new IllegalArgumentException("[half_float] supports only finite values, but got [" + value + "]");
251+
}
252+
}
234253
},
235254
FLOAT("float", NumericType.FLOAT) {
236255
@Override
237256
Float parse(Object value, boolean coerce) {
257+
final float result;
258+
238259
if (value instanceof Number) {
239-
return ((Number) value).floatValue();
240-
}
241-
if (value instanceof BytesRef) {
242-
value = ((BytesRef) value).utf8ToString();
260+
result = ((Number) value).floatValue();
261+
} else {
262+
if (value instanceof BytesRef) {
263+
value = ((BytesRef) value).utf8ToString();
264+
}
265+
result = Float.parseFloat(value.toString());
243266
}
244-
return Float.parseFloat(value.toString());
267+
validateParsed(result);
268+
return result;
245269
}
246270

247271
@Override
248272
Float parse(XContentParser parser, boolean coerce) throws IOException {
249-
return parser.floatValue(coerce);
273+
float parsed = parser.floatValue(coerce);
274+
validateParsed(parsed);
275+
return parsed;
250276
}
251277

252278
@Override
@@ -308,16 +334,26 @@ public List<Field> createFields(String name, Number value,
308334
}
309335
return fields;
310336
}
337+
338+
private void validateParsed(float value) {
339+
if (!Float.isFinite(value)) {
340+
throw new IllegalArgumentException("[float] supports only finite values, but got [" + value + "]");
341+
}
342+
}
311343
},
312344
DOUBLE("double", NumericType.DOUBLE) {
313345
@Override
314346
Double parse(Object value, boolean coerce) {
315-
return objectToDouble(value);
347+
double parsed = objectToDouble(value);
348+
validateParsed(parsed);
349+
return parsed;
316350
}
317351

318352
@Override
319353
Double parse(XContentParser parser, boolean coerce) throws IOException {
320-
return parser.doubleValue(coerce);
354+
double parsed = parser.doubleValue(coerce);
355+
validateParsed(parsed);
356+
return parsed;
321357
}
322358

323359
@Override
@@ -379,6 +415,12 @@ public List<Field> createFields(String name, Number value,
379415
}
380416
return fields;
381417
}
418+
419+
private void validateParsed(double value) {
420+
if (!Double.isFinite(value)) {
421+
throw new IllegalArgumentException("[double] supports only finite values, but got [" + value + "]");
422+
}
423+
}
382424
},
383425
BYTE("byte", NumericType.BYTE) {
384426
@Override

‎core/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.apache.lucene.search.BoostQuery;
2929
import org.apache.lucene.search.Query;
3030
import org.apache.lucene.search.SortField;
31+
import org.apache.lucene.util.BytesRef;
3132
import org.elasticsearch.common.Explicit;
3233
import org.elasticsearch.common.Nullable;
3334
import org.elasticsearch.common.settings.Setting;
@@ -144,7 +145,7 @@ public Mapper.Builder<?,?> parse(String name, Map<String, Object> node,
144145
if (propNode == null) {
145146
throw new MapperParsingException("Property [null_value] cannot be null.");
146147
}
147-
builder.nullValue(NumberFieldMapper.NumberType.DOUBLE.parse(propNode, false));
148+
builder.nullValue(ScaledFloatFieldMapper.parse(propNode));
148149
iterator.remove();
149150
} else if (propName.equals("ignore_malformed")) {
150151
builder.ignoreMalformed(TypeParsers.nodeBooleanValue(name, "ignore_malformed", propNode, parserContext));
@@ -153,7 +154,7 @@ public Mapper.Builder<?,?> parse(String name, Map<String, Object> node,
153154
builder.coerce(TypeParsers.nodeBooleanValue(name, "coerce", propNode, parserContext));
154155
iterator.remove();
155156
} else if (propName.equals("scaling_factor")) {
156-
builder.scalingFactor(NumberFieldMapper.NumberType.DOUBLE.parse(propNode, false).doubleValue());
157+
builder.scalingFactor(ScaledFloatFieldMapper.parse(propNode));
157158
iterator.remove();
158159
}
159160
}
@@ -207,7 +208,7 @@ public void checkCompatibility(MappedFieldType other, List<String> conflicts, bo
207208
@Override
208209
public Query termQuery(Object value, QueryShardContext context) {
209210
failIfNotIndexed();
210-
double queryValue = NumberFieldMapper.NumberType.DOUBLE.parse(value, false).doubleValue();
211+
double queryValue = parse(value);
211212
long scaledValue = Math.round(queryValue * scalingFactor);
212213
Query query = NumberFieldMapper.NumberType.LONG.termQuery(name(), scaledValue);
213214
if (boost() != 1f) {
@@ -221,7 +222,7 @@ public Query termsQuery(List<?> values, QueryShardContext context) {
221222
failIfNotIndexed();
222223
List<Long> scaledValues = new ArrayList<>(values.size());
223224
for (Object value : values) {
224-
double queryValue = NumberFieldMapper.NumberType.DOUBLE.parse(value, false).doubleValue();
225+
double queryValue = parse(value);
225226
long scaledValue = Math.round(queryValue * scalingFactor);
226227
scaledValues.add(scaledValue);
227228
}
@@ -237,15 +238,15 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower
237238
failIfNotIndexed();
238239
Long lo = null;
239240
if (lowerTerm != null) {
240-
double dValue = NumberFieldMapper.NumberType.DOUBLE.parse(lowerTerm, false).doubleValue();
241+
double dValue = parse(lowerTerm);
241242
if (includeLower == false) {
242243
dValue = Math.nextUp(dValue);
243244
}
244245
lo = Math.round(Math.ceil(dValue * scalingFactor));
245246
}
246247
Long hi = null;
247248
if (upperTerm != null) {
248-
double dValue = NumberFieldMapper.NumberType.DOUBLE.parse(upperTerm, false).doubleValue();
249+
double dValue = parse(upperTerm);
249250
if (includeUpper == false) {
250251
dValue = Math.nextDown(dValue);
251252
}
@@ -366,7 +367,7 @@ protected void parseCreateField(ParseContext context, List<IndexableField> field
366367
value = null;
367368
} else {
368369
try {
369-
numericValue = NumberFieldMapper.NumberType.DOUBLE.parse(parser, coerce.value());
370+
numericValue = parse(parser, coerce.value());
370371
} catch (IllegalArgumentException e) {
371372
if (ignoreMalformed.value()) {
372373
return;
@@ -390,7 +391,7 @@ protected void parseCreateField(ParseContext context, List<IndexableField> field
390391
}
391392

392393
if (numericValue == null) {
393-
numericValue = NumberFieldMapper.NumberType.DOUBLE.parse(value, false);
394+
numericValue = parse(value);
394395
}
395396

396397
if (includeInAll) {
@@ -451,6 +452,31 @@ protected void doXContentBody(XContentBuilder builder, boolean includeDefaults,
451452
}
452453
}
453454

455+
static Double parse(Object value) {
456+
return objectToDouble(value);
457+
}
458+
459+
private static Double parse(XContentParser parser, boolean coerce) throws IOException {
460+
return parser.doubleValue(coerce);
461+
}
462+
463+
/**
464+
* Converts an Object to a double by checking it against known types first
465+
*/
466+
private static double objectToDouble(Object value) {
467+
double doubleValue;
468+
469+
if (value instanceof Number) {
470+
doubleValue = ((Number) value).doubleValue();
471+
} else if (value instanceof BytesRef) {
472+
doubleValue = Double.parseDouble(((BytesRef) value).utf8ToString());
473+
} else {
474+
doubleValue = Double.parseDouble(value.toString());
475+
}
476+
477+
return doubleValue;
478+
}
479+
454480
private static class ScaledFloatIndexFieldData implements IndexNumericFieldData {
455481

456482
private final IndexNumericFieldData scaledFieldData;

‎core/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,15 @@
2121

2222
import org.apache.lucene.index.DocValuesType;
2323
import org.apache.lucene.index.IndexableField;
24+
import org.elasticsearch.common.bytes.BytesReference;
2425
import org.elasticsearch.common.compress.CompressedXContent;
2526
import org.elasticsearch.common.xcontent.XContentFactory;
2627
import org.elasticsearch.common.xcontent.XContentType;
28+
import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType;
29+
import org.elasticsearch.index.mapper.NumberFieldTypeTests.OutOfRangeSpec;
2730

2831
import java.io.IOException;
32+
import java.util.List;
2933
import java.util.Arrays;
3034
import java.util.HashSet;
3135

@@ -339,4 +343,61 @@ public void testEmptyName() throws IOException {
339343
}
340344
}
341345

346+
public void testOutOfRangeValues() throws IOException {
347+
final List<OutOfRangeSpec<Object>> inputs = Arrays.asList(
348+
OutOfRangeSpec.of(NumberType.HALF_FLOAT, "65520", "[half_float] supports only finite values"),
349+
OutOfRangeSpec.of(NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"),
350+
OutOfRangeSpec.of(NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"),
351+
352+
OutOfRangeSpec.of(NumberType.HALF_FLOAT, "-65520", "[half_float] supports only finite values"),
353+
OutOfRangeSpec.of(NumberType.FLOAT, "-3.4028235E39", "[float] supports only finite values"),
354+
OutOfRangeSpec.of(NumberType.DOUBLE, "-1.7976931348623157E309", "[double] supports only finite values"),
355+
356+
OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.NaN, "[half_float] supports only finite values"),
357+
OutOfRangeSpec.of(NumberType.FLOAT, Float.NaN, "[float] supports only finite values"),
358+
OutOfRangeSpec.of(NumberType.DOUBLE, Double.NaN, "[double] supports only finite values"),
359+
360+
OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.POSITIVE_INFINITY, "[half_float] supports only finite values"),
361+
OutOfRangeSpec.of(NumberType.FLOAT, Float.POSITIVE_INFINITY, "[float] supports only finite values"),
362+
OutOfRangeSpec.of(NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values"),
363+
364+
OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.NEGATIVE_INFINITY, "[half_float] supports only finite values"),
365+
OutOfRangeSpec.of(NumberType.FLOAT, Float.NEGATIVE_INFINITY, "[float] supports only finite values"),
366+
OutOfRangeSpec.of(NumberType.DOUBLE, Double.NEGATIVE_INFINITY, "[double] supports only finite values")
367+
);
368+
369+
for(OutOfRangeSpec<Object> item: inputs) {
370+
try {
371+
parseRequest(item.type, createIndexRequest(item.value));
372+
fail("Mapper parsing exception expected for [" + item.type + "] with value [" + item.value + "]");
373+
} catch (MapperParsingException e) {
374+
assertThat("Incorrect error message for [" + item.type + "] with value [" + item.value + "]",
375+
e.getCause().getMessage(), containsString(item.message));
376+
}
377+
}
378+
}
379+
380+
private void parseRequest(NumberType type, BytesReference content) throws IOException {
381+
createDocumentMapper(type).parse(SourceToParse.source("test", "type", "1", content, XContentType.JSON));
382+
}
383+
384+
private DocumentMapper createDocumentMapper(NumberType type) throws IOException {
385+
String mapping = XContentFactory.jsonBuilder()
386+
.startObject()
387+
.startObject("type")
388+
.startObject("properties")
389+
.startObject("field")
390+
.field("type", type.typeName())
391+
.endObject()
392+
.endObject()
393+
.endObject()
394+
.endObject()
395+
.string();
396+
397+
return parser.parse("type", new CompressedXContent(mapping));
398+
}
399+
400+
private BytesReference createIndexRequest(Object value) throws IOException {
401+
return XContentFactory.jsonBuilder().startObject().field("field", value).endObject().bytes();
402+
}
342403
}

‎core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,14 @@
4545
import org.junit.Before;
4646

4747
import java.io.IOException;
48+
import java.math.BigDecimal;
4849
import java.nio.charset.StandardCharsets;
4950
import java.util.Arrays;
51+
import java.util.List;
5052
import java.util.function.Supplier;
5153

54+
import static org.hamcrest.Matchers.containsString;
55+
5256
public class NumberFieldTypeTests extends FieldTypeTestCase {
5357

5458
NumberType type;
@@ -300,8 +304,8 @@ public void testHalfFloatRange() throws IOException {
300304
IndexSearcher searcher = newSearcher(reader);
301305
final int numQueries = 1000;
302306
for (int i = 0; i < numQueries; ++i) {
303-
float l = (randomFloat() * 2 - 1) * 70000;
304-
float u = (randomFloat() * 2 - 1) * 70000;
307+
float l = (randomFloat() * 2 - 1) * 65504;
308+
float u = (randomFloat() * 2 - 1) * 65504;
305309
boolean includeLower = randomBoolean();
306310
boolean includeUpper = randomBoolean();
307311
Query floatQ = NumberFieldMapper.NumberType.FLOAT.rangeQuery("float", l, u, includeLower, includeUpper, false);
@@ -382,4 +386,59 @@ public void doTestDocValueRangeQueries(NumberType type, Supplier<Number> valueSu
382386
reader.close();
383387
dir.close();
384388
}
389+
390+
public void testParseOutOfRangeValues() throws IOException {
391+
final List<OutOfRangeSpec<Object>> inputs = Arrays.asList(
392+
OutOfRangeSpec.of(NumberType.HALF_FLOAT, "65520", "[half_float] supports only finite values"),
393+
OutOfRangeSpec.of(NumberType.FLOAT, "3.4028235E39", "[float] supports only finite values"),
394+
OutOfRangeSpec.of(NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"),
395+
396+
OutOfRangeSpec.of(NumberType.HALF_FLOAT, 65520f, "[half_float] supports only finite values"),
397+
OutOfRangeSpec.of(NumberType.FLOAT, 3.4028235E39d, "[float] supports only finite values"),
398+
OutOfRangeSpec.of(NumberType.DOUBLE, new BigDecimal("1.7976931348623157E309"), "[double] supports only finite values"),
399+
400+
OutOfRangeSpec.of(NumberType.HALF_FLOAT, -65520f, "[half_float] supports only finite values"),
401+
OutOfRangeSpec.of(NumberType.FLOAT, -3.4028235E39d, "[float] supports only finite values"),
402+
OutOfRangeSpec.of(NumberType.DOUBLE, new BigDecimal("-1.7976931348623157E309"), "[double] supports only finite values"),
403+
404+
OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.NaN, "[half_float] supports only finite values"),
405+
OutOfRangeSpec.of(NumberType.FLOAT, Float.NaN, "[float] supports only finite values"),
406+
OutOfRangeSpec.of(NumberType.DOUBLE, Double.NaN, "[double] supports only finite values"),
407+
408+
OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.POSITIVE_INFINITY, "[half_float] supports only finite values"),
409+
OutOfRangeSpec.of(NumberType.FLOAT, Float.POSITIVE_INFINITY, "[float] supports only finite values"),
410+
OutOfRangeSpec.of(NumberType.DOUBLE, Double.POSITIVE_INFINITY, "[double] supports only finite values"),
411+
412+
OutOfRangeSpec.of(NumberType.HALF_FLOAT, Float.NEGATIVE_INFINITY, "[half_float] supports only finite values"),
413+
OutOfRangeSpec.of(NumberType.FLOAT, Float.NEGATIVE_INFINITY, "[float] supports only finite values"),
414+
OutOfRangeSpec.of(NumberType.DOUBLE, Double.NEGATIVE_INFINITY, "[double] supports only finite values")
415+
);
416+
417+
for (OutOfRangeSpec<Object> item: inputs) {
418+
try {
419+
item.type.parse(item.value, false);
420+
fail("Parsing exception expected for [" + item.type + "] with value [" + item.value + "]");
421+
} catch (IllegalArgumentException e) {
422+
assertThat("Incorrect error message for [" + item.type + "] with value [" + item.value + "]",
423+
e.getMessage(), containsString(item.message));
424+
}
425+
}
426+
}
427+
428+
static class OutOfRangeSpec<V> {
429+
430+
final NumberType type;
431+
final V value;
432+
final String message;
433+
434+
static <V> OutOfRangeSpec<V> of(NumberType t, V v, String m) {
435+
return new OutOfRangeSpec<>(t, v, m);
436+
}
437+
438+
OutOfRangeSpec(NumberType t, V v, String m) {
439+
type = t;
440+
value = v;
441+
message = m;
442+
}
443+
}
385444
}

0 commit comments

Comments
 (0)