Skip to content

Commit

Permalink
Stricter validation for min/max values for whole numbers (#26137)
Browse files Browse the repository at this point in the history
  • Loading branch information
fred84 authored and jpountz committed Aug 21, 2017
1 parent c30d6eb commit 9a3216d
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 23 deletions.
30 changes: 30 additions & 0 deletions core/src/main/java/org/elasticsearch/common/Numbers.java
Original file line number Diff line number Diff line change
Expand Up @@ -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() {

}
Expand Down Expand Up @@ -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. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
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;

Expand Down Expand Up @@ -133,7 +134,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);
Expand All @@ -152,7 +160,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);
Expand All @@ -171,13 +185,7 @@ 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 (long) Double.parseDouble(stringValue);
}
return Numbers.toLong(text(), coerce);
}
long result = doLongValue();
ensureNumberConversion(coerce, result, Long.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,15 @@
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;
import org.elasticsearch.common.settings.Settings;
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;
Expand Down Expand Up @@ -644,27 +646,23 @@ public List<Field> createFields(String name, Number value,
LONG("long", NumericType.LONG) {
@Override
Long parse(Object value, boolean coerce) {
double doubleValue = objectToDouble(value);
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");
}

if (value instanceof Number) {
return ((Number) value).longValue();
}

// 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);
}
return Numbers.toLong(stringValue, coerce);
}

@Override
Expand Down Expand Up @@ -836,7 +834,6 @@ private static double objectToDouble(Object value) {

return doubleValue;
}

}

public static final class NumberFieldType extends MappedFieldType {
Expand Down
15 changes: 15 additions & 0 deletions core/src/test/java/org/elasticsearch/common/NumbersTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -234,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")
Expand Down Expand Up @@ -345,6 +348,26 @@ public void testEmptyName() throws IOException {

public void testOutOfRangeValues() throws IOException {
final List<OutOfRangeSpec<Object>> inputs = Arrays.asList(
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", "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.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"),
Expand Down Expand Up @@ -398,6 +421,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();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -389,6 +392,22 @@ public void doTestDocValueRangeQueries(NumberType type, Supplier<Number> valueSu

public void testParseOutOfRangeValues() throws IOException {
final List<OutOfRangeSpec<Object>> 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, 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"),
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"),
OutOfRangeSpec.of(NumberType.DOUBLE, "1.7976931348623157E309", "[double] supports only finite values"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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\"]"));
}
}

Expand Down

0 comments on commit 9a3216d

Please sign in to comment.