Skip to content

Commit 5eac1cf

Browse files
committed
Fix issues handling high-precision numbers
The Double.isFinite method was incorrectly used to detect if a value would fit in a double. The method handles this properly, but it's not actually what we want - we want to know if the value will be accurately represented by a double. To resolve this, a NumberUtils class is added with a method that validates precision compatibility. Tests are added for this and all call sites to ensure the behavior remains consistent.
1 parent 4491592 commit 5eac1cf

File tree

10 files changed

+226
-8
lines changed

10 files changed

+226
-8
lines changed

smithy-jmespath/build.gradle.kts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,7 @@ description = "A standalone JMESPath parser"
1010

1111
extra["displayName"] = "Smithy :: JMESPath"
1212
extra["moduleName"] = "software.amazon.smithy.jmespath"
13+
14+
dependencies {
15+
api(project(":smithy-utils"))
16+
}

smithy-jmespath/src/main/java/software/amazon/smithy/jmespath/Lexer.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,14 @@
44
*/
55
package software.amazon.smithy.jmespath;
66

7+
import java.math.BigDecimal;
78
import java.util.ArrayList;
89
import java.util.List;
910
import java.util.Objects;
1011
import java.util.function.Predicate;
1112
import software.amazon.smithy.jmespath.ast.LiteralExpression;
1213
import software.amazon.smithy.jmespath.evaluation.JmespathRuntime;
14+
import software.amazon.smithy.utils.NumberUtils;
1315

1416
final class Lexer<T> {
1517

@@ -470,7 +472,17 @@ private Token parseNumber() {
470472
try {
471473
Number number;
472474
if (lexeme.contains(".") || lexeme.toLowerCase().contains("e")) {
473-
number = Double.parseDouble(lexeme);
475+
Double doubleValue = Double.parseDouble(lexeme);
476+
if (doubleValue.isInfinite() || doubleValue.isNaN()) {
477+
number = doubleValue;
478+
} else {
479+
BigDecimal bigDecimalValue = new BigDecimal(lexeme);
480+
if (NumberUtils.isDoublePrecisionCompatible(bigDecimalValue, doubleValue)) {
481+
number = doubleValue;
482+
} else {
483+
number = bigDecimalValue;
484+
}
485+
}
474486
} else {
475487
number = Long.parseLong(lexeme);
476488
}
@@ -481,6 +493,8 @@ private Token parseNumber() {
481493
}
482494
}
483495

496+
497+
484498
private String createInvalidNumberString(int startPosition, String message) {
485499
String lexeme = sliceFrom(startPosition);
486500
return String.format("Invalid number '%s': %s", lexeme, message);

smithy-jmespath/src/test/java/software/amazon/smithy/jmespath/LexerTest.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import static org.hamcrest.Matchers.empty;
1111
import static org.hamcrest.Matchers.equalTo;
1212
import static org.hamcrest.Matchers.hasSize;
13+
import static org.hamcrest.Matchers.instanceOf;
1314
import static org.hamcrest.Matchers.is;
1415

1516
import java.util.ArrayList;
@@ -542,6 +543,27 @@ public void parsesComplexNumbers() {
542543
assertThat(tokens.get(2).column, equalTo(25));
543544
}
544545

546+
@Test
547+
public void parsesDoublePrecisionCompatibleNumber() {
548+
List<Token> tokens = tokenize("1.5");
549+
550+
assertThat(tokens.get(0).type, equalTo(TokenType.NUMBER));
551+
Number value = tokens.get(0).value.expectNumberValue();
552+
assertThat(value, is(instanceOf(Double.class)));
553+
assertThat(value.doubleValue(), equalTo(1.5));
554+
}
555+
556+
@Test
557+
public void parsesDoublePrecisionIncompatibleNumberAsBigDecimal() {
558+
// 2^53 + 1 with a decimal triggers the float path but loses precision as a double.
559+
List<Token> tokens = tokenize("9007199254740993.0");
560+
561+
assertThat(tokens.get(0).type, equalTo(TokenType.NUMBER));
562+
Number value = tokens.get(0).value.expectNumberValue();
563+
assertThat(value, is(instanceOf(java.math.BigDecimal.class)));
564+
assertThat(value.toString(), equalTo("9007199254740993.0"));
565+
}
566+
545567
@Test
546568
public void detectsTopLevelInvalidSyntax() {
547569
Assertions.assertThrows(JmespathException.class, () -> Lexer.tokenize("~"));

smithy-model/src/main/java/software/amazon/smithy/model/loader/DefaultTokenizer.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import java.math.BigInteger;
99
import java.util.NoSuchElementException;
1010
import software.amazon.smithy.model.SourceLocation;
11+
import software.amazon.smithy.utils.NumberUtils;
1112
import software.amazon.smithy.utils.SimpleParser;
1213

1314
class DefaultTokenizer implements IdlTokenizer {
@@ -320,11 +321,16 @@ private IdlToken parseNumber() {
320321
try {
321322
String lexeme = ParserUtils.parseNumber(parser);
322323
if (lexeme.contains("e") || lexeme.contains("E") || lexeme.contains(".")) {
323-
double value = Double.parseDouble(lexeme);
324-
if (Double.isFinite(value)) {
324+
Double value = Double.valueOf(lexeme);
325+
if (value.isInfinite() || value.isNaN()) {
325326
currentTokenNumber = value;
326327
} else {
327-
currentTokenNumber = new BigDecimal(lexeme);
328+
BigDecimal bigDecimalValue = new BigDecimal(lexeme);
329+
if (NumberUtils.isDoublePrecisionCompatible(bigDecimalValue, value)) {
330+
currentTokenNumber = value;
331+
} else {
332+
currentTokenNumber = bigDecimalValue;
333+
}
328334
}
329335
} else {
330336
try {
@@ -348,6 +354,8 @@ private IdlToken parseNumber() {
348354
}
349355
}
350356

357+
358+
351359
private IdlToken parseIdentifier() {
352360
try {
353361
ParserUtils.consumeIdentifier(parser);

smithy-model/src/main/java/software/amazon/smithy/model/node/NumberNode.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,13 @@ public NumberNode(Number value, SourceLocation sourceLocation) {
3434
this.value = toBigDecimal(originalValue);
3535
}
3636

37+
public NumberNode(Number value, BigDecimal bigDecimalValue, SourceLocation sourceLocation) {
38+
super(sourceLocation);
39+
originalValue = value;
40+
stringCache = value.toString();
41+
this.value = bigDecimalValue;
42+
}
43+
3744
private BigDecimal toBigDecimal(Number value) {
3845
if (value instanceof BigDecimal) {
3946
return (BigDecimal) value;

smithy-model/src/main/java/software/amazon/smithy/model/node/internal/NodeHandler.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import software.amazon.smithy.model.node.NumberNode;
1717
import software.amazon.smithy.model.node.ObjectNode;
1818
import software.amazon.smithy.model.node.StringNode;
19+
import software.amazon.smithy.utils.NumberUtils;
1920
import software.amazon.smithy.utils.SmithyInternalApi;
2021

2122
@SmithyInternalApi
@@ -70,19 +71,23 @@ void endString(String string, SourceLocation location) {
7071
@Override
7172
void endNumber(String string, SourceLocation location) {
7273
if (string.contains("e") || string.contains("E") || string.contains(".")) {
73-
double doubleValue = Double.parseDouble(string);
74-
if (Double.isFinite(doubleValue)) {
74+
Double doubleValue = Double.valueOf(string);
75+
if (doubleValue.isInfinite() || doubleValue.isNaN()) {
7576
value = new NumberNode(doubleValue, location);
7677
} else {
77-
value = new NumberNode(new BigDecimal(string), location);
78+
BigDecimal bigDecimalValue = new BigDecimal(string);
79+
if (NumberUtils.isDoublePrecisionCompatible(bigDecimalValue, doubleValue)) {
80+
value = new NumberNode(doubleValue, bigDecimalValue, location);
81+
} else {
82+
value = new NumberNode(bigDecimalValue, bigDecimalValue, location);
83+
}
7884
}
7985
} else {
8086
try {
8187
value = new NumberNode(Long.parseLong(string), location);
8288
} catch (NumberFormatException e) {
8389
value = new NumberNode(new BigInteger(string), location);
8490
}
85-
8691
}
8792
}
8893

smithy-model/src/test/java/software/amazon/smithy/model/loader/TokenizerTest.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import static org.hamcrest.MatcherAssert.assertThat;
88
import static org.hamcrest.Matchers.equalTo;
9+
import static org.hamcrest.Matchers.instanceOf;
910
import static org.hamcrest.Matchers.is;
1011

1112
import java.util.NoSuchElementException;
@@ -199,6 +200,27 @@ public void storesCurrentTokenNumber() {
199200
assertThat(tokenizer.getCurrentTokenNumberValue(), equalTo(10L));
200201
}
201202

203+
@Test
204+
public void parsesDoublePrecisionCompatibleFloat() {
205+
IdlTokenizer tokenizer = IdlTokenizer.create("1.5");
206+
207+
assertThat(tokenizer.next(), is(IdlToken.NUMBER));
208+
Number value = tokenizer.getCurrentTokenNumberValue();
209+
assertThat(value, instanceOf(Double.class));
210+
assertThat(value.doubleValue(), equalTo(1.5));
211+
}
212+
213+
@Test
214+
public void parsesDoublePrecisionIncompatibleFloatAsBigDecimal() {
215+
// 2^53 + 1 with a decimal triggers the float path but loses precision as a double.
216+
IdlTokenizer tokenizer = IdlTokenizer.create("9007199254740993.0");
217+
218+
assertThat(tokenizer.next(), is(IdlToken.NUMBER));
219+
Number value = tokenizer.getCurrentTokenNumberValue();
220+
assertThat(value, instanceOf(java.math.BigDecimal.class));
221+
assertThat(value.toString(), equalTo("9007199254740993.0"));
222+
}
223+
202224
@Test
203225
public void throwsWhenAccessingStringValue() {
204226
IdlTokenizer tokenizer = IdlTokenizer.create("10");

smithy-model/src/test/java/software/amazon/smithy/model/node/NodeParserTest.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import static org.hamcrest.MatcherAssert.assertThat;
88
import static org.hamcrest.Matchers.equalTo;
9+
import static org.hamcrest.Matchers.instanceOf;
910
import static org.hamcrest.Matchers.is;
1011
import static org.hamcrest.Matchers.startsWith;
1112

@@ -69,11 +70,25 @@ public void parsesFloatNode() {
6970
Node result = Node.parse("1.5");
7071

7172
assertThat(result.isNumberNode(), is(true));
73+
assertThat(result.expectNumberNode().getValue(), is(instanceOf(Double.class)));
7274
assertThat(result.expectNumberNode().getValue(), equalTo(1.5));
7375
assertThat(result.getSourceLocation().getLine(), is(1));
7476
assertThat(result.getSourceLocation().getColumn(), is(1));
7577
}
7678

79+
@Test
80+
public void parsesDoublePrecisionIncompatibleNumberAsBigDecimal() {
81+
// 2^53 + 1 with a decimal triggers the float path but loses precision as a double.
82+
Node result = Node.parse("9007199254740993.0");
83+
assertThat(result.isNumberNode(), is(true));
84+
85+
NumberNode numberNode = result.expectNumberNode();
86+
assertThat(numberNode.getValue(), is(instanceOf(java.math.BigDecimal.class)));
87+
assertThat(numberNode.getValue().toString(), equalTo("9007199254740993.0"));
88+
assertThat(numberNode.getSourceLocation().getLine(), is(1));
89+
assertThat(numberNode.getSourceLocation().getColumn(), is(1));
90+
}
91+
7792
@Test
7893
public void parsesEmptyArray() {
7994
Node result = Node.parse("[]");
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
package software.amazon.smithy.utils;
6+
7+
import java.math.BigDecimal;
8+
9+
/**
10+
* Functions that make working with numbers easier.
11+
*/
12+
public final class NumberUtils {
13+
private NumberUtils() {}
14+
15+
/**
16+
* Checks whether the bigDecimalValue is represented in the doubleValue without
17+
* a loss of precision due to IEEE-754 floating-point representation.
18+
*
19+
* @param bigDecimalValue The value as a BigDecimal.
20+
* @param doubleValue The value as a double.
21+
* @return true if the bigDecimalValue is precisely represented by the doubleValue.
22+
*/
23+
public static boolean isDoublePrecisionCompatible(BigDecimal bigDecimalValue, Double doubleValue) {
24+
return bigDecimalValue.compareTo(BigDecimal.valueOf(doubleValue)) == 0;
25+
}
26+
}
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
package software.amazon.smithy.utils;
6+
7+
import static org.hamcrest.MatcherAssert.assertThat;
8+
import static org.hamcrest.Matchers.is;
9+
10+
import java.math.BigDecimal;
11+
import java.util.stream.Stream;
12+
import org.junit.jupiter.params.ParameterizedTest;
13+
import org.junit.jupiter.params.provider.Arguments;
14+
import org.junit.jupiter.params.provider.MethodSource;
15+
16+
public class NumberUtilsTest {
17+
18+
@ParameterizedTest
19+
@MethodSource("compatibleValues")
20+
public void detectsDoublePrecisionCompatibleValues(BigDecimal bd, Double d) {
21+
assertThat(NumberUtils.isDoublePrecisionCompatible(bd, d), is(true));
22+
}
23+
24+
static Stream<Arguments> compatibleValues() {
25+
return Stream.of(
26+
// Zero
27+
Arguments.of(new BigDecimal("0"), 0.0),
28+
// Negative zero is mathematically equal to zero
29+
Arguments.of(new BigDecimal("0"), -0.0),
30+
// Small positive integer
31+
Arguments.of(new BigDecimal("1"), 1.0),
32+
// Trailing zeros in BigDecimal scale don't affect compareTo
33+
Arguments.of(new BigDecimal("1.00"), 1.0),
34+
// Negative integer
35+
Arguments.of(new BigDecimal("-42"), -42.0),
36+
// Simple decimal that doubles represent exactly
37+
Arguments.of(new BigDecimal("0.5"), 0.5),
38+
// Power-of-two fraction
39+
Arguments.of(new BigDecimal("0.25"), 0.25),
40+
// Another power-of-two fraction
41+
Arguments.of(new BigDecimal("0.125"), 0.125),
42+
// Larger exact integer within double range
43+
Arguments.of(new BigDecimal("1048576"), 1048576.0),
44+
// Max safe integer for doubles (2^53)
45+
Arguments.of(new BigDecimal("9007199254740992"), 9007199254740992.0),
46+
// Negative max safe integer
47+
Arguments.of(new BigDecimal("-9007199254740992"), -9007199254740992.0),
48+
// Small negative decimal
49+
Arguments.of(new BigDecimal("-0.5"), -0.5),
50+
// Double.MAX_VALUE round-trips through Double.toString
51+
Arguments.of(new BigDecimal(String.valueOf(Double.MAX_VALUE)), Double.MAX_VALUE),
52+
// Double.MIN_VALUE round-trips through Double.toString
53+
Arguments.of(new BigDecimal(String.valueOf(Double.MIN_VALUE)), Double.MIN_VALUE),
54+
// 0.1 round-trips: BigDecimal.valueOf(0.1) equals BigDecimal("0.1")
55+
Arguments.of(new BigDecimal("0.1"), 0.1),
56+
// 0.2 round-trips cleanly
57+
Arguments.of(new BigDecimal("0.2"), 0.2),
58+
// 0.3 round-trips cleanly
59+
Arguments.of(new BigDecimal("0.3"), 0.3),
60+
// Negative decimal round-trips cleanly
61+
Arguments.of(new BigDecimal("-0.1"), -0.1)
62+
);
63+
}
64+
65+
@ParameterizedTest
66+
@MethodSource("incompatibleValues")
67+
public void detectsDoublePrecisionIncompatibleValues(BigDecimal bd, Double d) {
68+
assertThat(NumberUtils.isDoublePrecisionCompatible(bd, d), is(false));
69+
}
70+
71+
static Stream<Arguments> incompatibleValues() {
72+
return Stream.of(
73+
// Integer beyond 2^53 loses precision
74+
Arguments.of(new BigDecimal("9007199254740993"), 9007199254740993.0),
75+
// Large integer that rounds in double
76+
Arguments.of(new BigDecimal("9007199254740995"), 9007199254740995.0),
77+
// High-precision decimal beyond what double can distinguish
78+
Arguments.of(new BigDecimal("1.0000000000000001"), 1.0000000000000001),
79+
// Precision loss in negative large integer
80+
Arguments.of(new BigDecimal("-9007199254740993"), -9007199254740993.0),
81+
// Decimal with many significant digits
82+
Arguments.of(new BigDecimal("123456789.123456789"), 123456789.123456789),
83+
// Small value with precision beyond double's capability
84+
Arguments.of(new BigDecimal("0.10000000000000001"), 0.10000000000000001),
85+
// Precision beyond what Double.toString preserves
86+
Arguments.of(new BigDecimal("3.141592653589793238"), 3.141592653589793238),
87+
// Very small difference from an integer, not representable at stated precision
88+
Arguments.of(new BigDecimal("2.00000000000000005"), 2.00000000000000005),
89+
// Large number with fractional precision loss
90+
Arguments.of(new BigDecimal("1e308").add(new BigDecimal("1")), 1e308),
91+
// Negative high-precision decimal beyond double's digits
92+
Arguments.of(new BigDecimal("-0.123456789012345678"), -0.123456789012345678)
93+
);
94+
}
95+
}

0 commit comments

Comments
 (0)