8331485: Odd Results when Parsing Scientific Notation with Large Exponent

src/java.base/share/classes/java/text/DecimalFormat.java

@@ -2433,9 +2433,12 @@ int subparseNumber(String text, int position,
                     symbols.getGroupingSeparator();
             String exponentString = symbols.getExponentSeparator();
             boolean sawDecimal = false;
-            boolean sawExponent = false;
             boolean sawDigit = false;
-            int exponent = 0; // Set to the exponent value, if any
+            // Storing as long allows us to maintain accuracy of exponent
+            // when the exponent value as well as the decimalAt nears
+            // Integer.MAX/MIN value. However, the final expressed value is an int
+            long exponent = 0;
+            boolean[] expStat = new boolean[STATUS_LENGTH];
 
             // We have to track digitCount ourselves, because digits.count will
             // pin when the maximum allowable digits is reached.
@@ -2505,21 +2508,27 @@ int subparseNumber(String text, int position,
                     // require that they be followed by a digit.  Otherwise
                     // we backup and reprocess them.
                     backup = position;
-                } else if (checkExponent && !isExponent && text.regionMatches(position, exponentString, 0, exponentString.length())
-                        && !sawExponent) {
+                } else if (checkExponent && !isExponent
+                        && text.regionMatches(position, exponentString, 0, exponentString.length())) {
                     // Process the exponent by recursively calling this method.
                     ParsePosition pos = new ParsePosition(position + exponentString.length());
-                    boolean[] stat = new boolean[STATUS_LENGTH];
                     DigitList exponentDigits = new DigitList();
 
-                    if (subparse(text, pos, "", symbols.getMinusSignText(), exponentDigits, true, stat) &&
-                            exponentDigits.fitsIntoLong(stat[STATUS_POSITIVE], true)) {
-                        position = pos.index; // Advance past the exponent
-                        exponent = (int)exponentDigits.getLong();
-                        if (!stat[STATUS_POSITIVE]) {
-                            exponent = -exponent;
+                    if (subparse(text, pos, "", symbols.getMinusSignText(), exponentDigits, true, expStat)) {
+                        // We parse the exponent with isExponent == true, thus fitsIntoLong()
+                        // only returns false here if the exponent DigitList value exceeds
+                        // Long.MAX_VALUE. We do not need to worry about false being
+                        // returned for faulty values as they are ignored by DigitList.
+                        if (exponentDigits.fitsIntoLong(expStat[STATUS_POSITIVE], true)) {
+                            exponent = exponentDigits.getLong();
+                            if (!expStat[STATUS_POSITIVE]) {
+                                exponent = -exponent;
+                            }
+                        } else {
+                            exponent = expStat[STATUS_POSITIVE] ?
+                                    Long.MAX_VALUE : Long.MIN_VALUE;
                         }
-                        sawExponent = true;
+                        position = pos.index; // Advance past the exponent
                     }
                     break; // Whether we fail or succeed, we exit this loop
                 } else {
@@ -2537,7 +2546,9 @@ int subparseNumber(String text, int position,
             }
 
             // Adjust for exponent, if any
-            digits.decimalAt += exponent;
+            if (exponent != 0) {
+                digits.decimalAt = shiftDecimalAt(digits.decimalAt, exponent);
+            }
 
             // If none of the text string was recognized.  For example, parse
             // "x" with pattern "#0.00" (return index and error index both 0)
@@ -2548,7 +2559,52 @@ int subparseNumber(String text, int position,
             }
         }
         return position;
+    }
 
+    // Calculate the final decimal position based off the exponent value
+    // and the existing decimalAt position. If overflow/underflow, the value
+    // should be set as either Integer.MAX/MIN
+    private int shiftDecimalAt(int decimalAt, long exponent) {
+        try {
+            exponent = Math.addExact(decimalAt, exponent);
+        } catch (ArithmeticException ex) {
+            // If we under/overflow a Long do not bother with the decimalAt
+            // As it can only shift up to Integer.MAX/MIN which has no affect
+            if (exponent > 0 && decimalAt > 0) {
+                return Integer.MAX_VALUE;
+            } else {
+                return Integer.MIN_VALUE;
+            }
+        }
+        try {
+            decimalAt = Math.toIntExact(exponent);
+        } catch (ArithmeticException ex) {
+            decimalAt = exponent > 0 ? Integer.MAX_VALUE : Integer.MIN_VALUE;
+        }
+        return decimalAt;
+    }
+
+    // Checks to make sure grouping size is not violated. Used when strict.
+    private boolean isGroupingViolation(int pos, int prevGroupingPos) {
+        return isGroupingUsed() && // Only violates if using grouping
+                // Checks if a previous grouping symbol was seen.
+                prevGroupingPos != -groupingSize &&
+                // The check itself, - 1 to account for grouping/decimal symbol
+                pos - 1 != prevGroupingPos + groupingSize;
+    }
+
+    // Calculates the index that violated the grouping size
+    // Violation can be over or under the grouping size
+    // under - Current group has a grouping size of less than the expected
+    // over - Current group has a grouping size of more than the expected
+    private int groupingViolationIndex(int pos, int prevGroupingPos) {
+        // Both examples assume grouping size of 3 and 0 indexed
+        // under ex: "1,23,4". (4) OR "1,,2". (2) When under, violating char is grouping symbol
+        // over ex: "1,2345,6. (5) When over, violating char is the excess digit
+        // This method is only evaluated when a grouping symbol is found, thus
+        // we can take the minimum of either the current pos, or where we expect
+        // the current group to have ended
+        return Math.min(pos, prevGroupingPos + groupingSize + 1);
     }
 
     /**
@@ -2566,7 +2622,6 @@ public DecimalFormatSymbols getDecimalFormatSymbols() {
         }
     }
 
-
     /**
      * Sets the decimal format symbols, which is generally not changed
      * by the programmer or user.

test/jdk/java/text/Format/DecimalFormat/LargeExponentsTest.java

@@ -0,0 +1,193 @@
+/*
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8331485
+ * @summary Ensure correctness when parsing large (+/-) exponent values that
+ *          exceed Integer.MAX_VALUE and Long.MAX_VALUE.
+ * @run junit/othervm --add-opens java.base/java.text=ALL-UNNAMED LargeExponentsTest
+ */
+
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.text.DecimalFormat;
+import java.text.NumberFormat;
+import java.text.ParseException;
+import java.text.ParsePosition;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Locale;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+// We prevent odd results when parsing large exponent values by ensuring
+// that we properly handle overflow in the implementation of DigitList
+public class LargeExponentsTest {
+
+    // Exponent symbol is 'E'
+    private static final NumberFormat FMT = NumberFormat.getInstance(Locale.US);
+
+    // Check that the parsed value and parse position index are both equal to the expected values.
+    // We are mainly checking that an exponent > Integer.MAX_VALUE no longer
+    // parses to 0 and that an exponent > Long.MAX_VALUE no longer parses to the mantissa.
+    @ParameterizedTest
+    @MethodSource({"largeExponentValues", "smallExponentValues", "bugReportValues", "edgeCases"})
+    public void overflowTest(String parseString, Double expectedValue) throws ParseException {
+        checkParse(parseString, expectedValue);
+        checkParseWithPP(parseString, expectedValue);
+    }
+
+    // A separate white-box test to avoid the memory consumption of testing cases
+    // when the String is near Integer.MAX_LENGTH
+    @ParameterizedTest
+    @MethodSource
+    public void largeDecimalAtExponentTest(int expected, int decimalAt, long expVal)
+            throws NoSuchMethodException, InvocationTargetException, IllegalAccessException {
+        DecimalFormat df = new DecimalFormat();
+        Method m = df.getClass().getDeclaredMethod(
+                "shiftDecimalAt", int.class, long.class);
+        m.setAccessible(true);
+        assertEquals(expected, m.invoke(df, decimalAt, expVal));
+    }
+
+    // Cases where we can test behavior when the String is near Integer.MAX_LENGTH
+    private static Stream<Arguments> largeDecimalAtExponentTest() {
+        return Stream.of(
+                // Equivalent to testing Arguments.of("0."+"0".repeat(Integer.MAX_VALUE-20)+"1"+"E2147483650", 1.0E22)
+                // This is an absurdly long decimal string with length close to Integer.MAX_VALUE
+                // where the decimal position correctly negates the exponent value, even if it exceeds Integer.MAX_VALUE
+                Arguments.of(23, -(Integer.MAX_VALUE-20), 3L+Integer.MAX_VALUE),
+                Arguments.of(-23, Integer.MAX_VALUE-20, -(3L+Integer.MAX_VALUE)),
+                Arguments.of(Integer.MIN_VALUE, -(Integer.MAX_VALUE-20), -(3L+Integer.MAX_VALUE)),
+                Arguments.of(Integer.MAX_VALUE, Integer.MAX_VALUE-20, 3L+Integer.MAX_VALUE),
+                Arguments.of(Integer.MAX_VALUE, -(Integer.MAX_VALUE-20), Long.MAX_VALUE),
+                Arguments.of(Integer.MIN_VALUE, Integer.MAX_VALUE-20, Long.MIN_VALUE)
+        );
+    }
+
+    // Checks the parse(String, ParsePosition) method
+    public void checkParse(String parseString, Double expectedValue) {
+        ParsePosition pp = new ParsePosition(0);
+        Number actualValue = FMT.parse(parseString, pp);
+        assertEquals(expectedValue, (double)actualValue);
+        assertEquals(parseString.length(), pp.getIndex());
+    }
+
+    // Checks the parse(String) method
+    public void checkParseWithPP(String parseString, Double expectedValue)
+            throws ParseException {
+        Number actualValue = FMT.parse(parseString);
+        assertEquals(expectedValue, (double)actualValue);
+    }
+
+    // Generate large enough exponents that should all be parsed as infinity
+    // when positive. This includes exponents that exceed Long.MAX_VALUE
+    private static List<Arguments> largeExponentValues() {
+        return createExponentValues(false);
+    }
+
+    // Same as previous provider but for negative exponent values, so expecting
+    // a parsed value of 0.
+    private static List<Arguments> smallExponentValues() {
+        return createExponentValues(true);
+    }
+
+    // Programmatically generate some large parse values that are expected
+    // to be parsed as infinity or 0
+    private static List<Arguments> createExponentValues(boolean negative) {
+        List<Arguments> args = new ArrayList<>();
+        // Start with a base value that should be parsed as infinity
+        String baseValue = "12234.123E1100";
+        // Continuously add to the String until we trigger the overflow condition
+        for (int i = 0; i < 100; i++) {
+            StringBuilder bldr = new StringBuilder();
+            // Add to exponent
+            bldr.append(baseValue).append("1".repeat(i));
+            // Add to mantissa
+            bldr.insert(0, "1".repeat(i));
+            args.add(Arguments.of(
+                    // Prepend "-" to exponent if negative
+                    negative ? bldr.insert(bldr.indexOf("E")+1, "-").toString() : bldr.toString(),
+                    // Expect 0 if negative, else infinity
+                    negative ? 0.0 : Double.POSITIVE_INFINITY));
+        }
+        return args;
+    }
+
+    // The provided values are all from the JBS issue
+    // These contain exponents that exceed Integer.MAX_VALUE, but not Long.MAX_VALUE
+    private static Stream<Arguments> bugReportValues() {
+        return Stream.of(
+                Arguments.of("0.123E1", 1.23),
+                Arguments.of("0.123E309", 1.23E308),
+                Arguments.of("0.123E310", Double.POSITIVE_INFINITY),
+                Arguments.of("0.123E2147483647", Double.POSITIVE_INFINITY),
+                Arguments.of("0.123E2147483648", Double.POSITIVE_INFINITY),
+                Arguments.of("0.0123E2147483648", Double.POSITIVE_INFINITY),
+                Arguments.of("0.0123E2147483649", Double.POSITIVE_INFINITY),
+                Arguments.of("1.23E2147483646", Double.POSITIVE_INFINITY),
+                Arguments.of("1.23E2147483647", Double.POSITIVE_INFINITY),
+                Arguments.of("0.123E4294967296", Double.POSITIVE_INFINITY),
+                Arguments.of("0.123E-322", 9.9E-324),
+                Arguments.of("0.123E-323", 0.0),
+                Arguments.of("0.123E-2147483647", 0.0),
+                Arguments.of("0.123E-2147483648", 0.0),
+                Arguments.of("0.123E-2147483649", 0.0)
+        );
+    }
+
+    // Some other edge case values to ensure parse correctness
+    private static Stream<Arguments> edgeCases() {
+        return Stream.of(
+                // Exponent itself does not cause underflow, but decimalAt adjustment
+                // based off mantissa should. decimalAt(-1) + exponent(Integer.MIN_VALUE) = underflow
+                Arguments.of("0.0123E-2147483648", 0.0),
+                // 0 exponent
+                Arguments.of("1.23E0", 1.23),
+                // Leading zeroes
+                Arguments.of("1.23E0000123", 1.23E123),
+                // Leading zeroes - Past Long.MAX_VALUE length
+                Arguments.of("1.23E00000000000000000000000000000000000000000123", 1.23E123),
+                // Trailing zeroes
+                Arguments.of("1.23E100", 1.23E100),
+                // Long.MAX_VALUE length
+                Arguments.of("1.23E1234567891234567800", Double.POSITIVE_INFINITY),
+                // Long.MAX_VALUE with trailing zeroes
+                Arguments.of("1.23E9223372036854775807000", Double.POSITIVE_INFINITY),
+                // Long.MIN_VALUE
+                Arguments.of("1.23E-9223372036854775808", 0.0),
+                // Exponent value smaller than Long.MIN_VALUE
+                Arguments.of("1.23E-9223372036854775809", 0.0),
+                // Exponent value equal to Long.MAX_VALUE
+                Arguments.of("1.23E9223372036854775807", Double.POSITIVE_INFINITY),
+                // Exponent value larger than Long.MAX_VALUE
+                Arguments.of("1.23E9223372036854775808", Double.POSITIVE_INFINITY)
+        );
+    }
+}