-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Reject out of range numbers for float, double and half_float #25826
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
2d0e5c1
c0fff6f
6e0a6ea
7d4f315
d1ebd6f
1358fed
44983d8
d072e69
ecf3424
b5d231d
187982a
d236158
7423c4d
8e88c9d
11ce7c8
3902911
656cf63
cd38455
b578b5a
3c666a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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<Field> 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<Field> 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<Field> 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<IndexableField> field | |
| numericValue = fieldType().type.parse(value, coerce.value()); | ||
| } | ||
|
|
||
| fieldType().type.validateParsed(numericValue); | ||
|
|
||
| if (includeInAll) { | ||
| context.allEntries().addText(fieldType().name(), value.toString(), fieldType().boost()); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,8 +26,11 @@ | |
| 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; | ||
|
|
||
|
|
@@ -314,4 +317,47 @@ public void testEmptyName() throws IOException { | |
| assertThat(e.getMessage(), containsString("name cannot be empty string")); | ||
| } | ||
| } | ||
|
|
||
| public void testOutOfRangeValue() { | ||
| final Map<String, String> 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); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| 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); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use this message format where we show what we got as an invalid value in the error messages for the other types too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated message format.