8326908: DecimalFormat::toPattern throws OutOfMemoryError when pattern is empty string

Reviewed-by: naoto

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

@@ -3283,68 +3283,88 @@ public class DecimalFormat extends NumberFormat {
     }
 
     /**
-     * Does the real work of generating a pattern.  */
+     * Implementation of producing a pattern. This method returns a positive and
+     * negative (if needed), pattern string in the form of : Prefix (optional)
+     * Number Suffix (optional). A NegativePattern is only produced if the
+     * prefix or suffix patterns differs.
+     */
     private String toPattern(boolean localized) {
+        // Determine symbol values; use DFS if localized
+        char zeroSymbol = localized ? symbols.getZeroDigit() : PATTERN_ZERO_DIGIT;
+        char digitSymbol = localized ? symbols.getDigit() : PATTERN_DIGIT;
+        char groupingSymbol = localized ?
+                (isCurrencyFormat ? symbols.getMonetaryGroupingSeparator() : symbols.getGroupingSeparator()) :
+                PATTERN_GROUPING_SEPARATOR;
+        char decimalSymbol = localized ?
+                (isCurrencyFormat ? symbols.getMonetaryDecimalSeparator() : symbols.getDecimalSeparator()) :
+                PATTERN_DECIMAL_SEPARATOR;
+        String exponentSymbol = localized ? symbols.getExponentSeparator() : PATTERN_EXPONENT;
+        char patternSeparator = localized ? symbols.getPatternSeparator() : PATTERN_SEPARATOR;
+
         StringBuilder result = new StringBuilder();
+        // j == 1 denotes PositivePattern, j == 0 denotes NegativePattern
         for (int j = 1; j >= 0; --j) {
-            if (j == 1)
-                appendAffix(result, posPrefixPattern, positivePrefix, localized);
-            else appendAffix(result, negPrefixPattern, negativePrefix, localized);
-            int i;
-            int digitCount = useExponentialNotation
-                        ? getMaximumIntegerDigits()
-                        : Math.max(groupingSize, getMinimumIntegerDigits())+1;
-            for (i = digitCount; i > 0; --i) {
-                if (i != digitCount && isGroupingUsed() && groupingSize != 0 &&
-                    i % groupingSize == 0) {
-                    result.append(localized ?
-                        (isCurrencyFormat ? symbols.getMonetaryGroupingSeparator() : symbols.getGroupingSeparator()) :
-                        PATTERN_GROUPING_SEPARATOR);
-                }
-                result.append(i <= getMinimumIntegerDigits()
-                    ? (localized ? symbols.getZeroDigit() : PATTERN_ZERO_DIGIT)
-                    : (localized ? symbols.getDigit() : PATTERN_DIGIT));
-            }
-            if (getMaximumFractionDigits() > 0 || decimalSeparatorAlwaysShown)
-                result.append(localized ?
-                    (isCurrencyFormat ? symbols.getMonetaryDecimalSeparator() : symbols.getDecimalSeparator()) :
-                    PATTERN_DECIMAL_SEPARATOR);
-            for (i = 0; i < getMaximumFractionDigits(); ++i) {
-                if (i < getMinimumFractionDigits()) {
-                    result.append(localized ? symbols.getZeroDigit() :
-                                  PATTERN_ZERO_DIGIT);
-                } else {
-                    result.append(localized ? symbols.getDigit() :
-                                  PATTERN_DIGIT);
-                }
-            }
-        if (useExponentialNotation)
-        {
-            result.append(localized ? symbols.getExponentSeparator() :
-                  PATTERN_EXPONENT);
-        for (i=0; i<minExponentDigits; ++i)
-                    result.append(localized ? symbols.getZeroDigit() :
-                                  PATTERN_ZERO_DIGIT);
-        }
             if (j == 1) {
-                appendAffix(result, posSuffixPattern, positiveSuffix, localized);
+                // Append positive and negative (if needed) prefix pattern
-                if ((negSuffixPattern == posSuffixPattern && // n == p == null
+                appendAffix(result, posPrefixPattern, positivePrefix, localized);
-                     negativeSuffix.equals(positiveSuffix))
+            } else {
-                    || (negSuffixPattern != null &&
+                appendAffix(result, negPrefixPattern, negativePrefix, localized);
-                        negSuffixPattern.equals(posSuffixPattern))) {
+            }
-                    if ((negPrefixPattern != null && posPrefixPattern != null &&
+            // Append integer digits
-                         negPrefixPattern.equals("'-" + posPrefixPattern)) ||
+            int digitCount = useExponentialNotation ? getMaximumIntegerDigits() :
-                        (negPrefixPattern == posPrefixPattern && // n == p == null
+                    Math.max(groupingSize, getMinimumIntegerDigits()) + 1;
-                         negativePrefix.equals(symbols.getMinusSignText() + positivePrefix)))
+            for (int i = digitCount; i > 0; --i) {
-                        break;
+                if (i != digitCount && isGroupingUsed() && groupingSize != 0 &&
+                        i % groupingSize == 0) {
+                    result.append(groupingSymbol);
                 }
-                result.append(localized ? symbols.getPatternSeparator() :
+                result.append(i <= getMinimumIntegerDigits() ? zeroSymbol : digitSymbol);
-                              PATTERN_SEPARATOR);
+            }
-            } else appendAffix(result, negSuffixPattern, negativeSuffix, localized);
+            // Append decimal symbol
+            if (getMaximumFractionDigits() > 0 || decimalSeparatorAlwaysShown) {
+                result.append(decimalSymbol);
+            }
+            // Append fraction digits
+            result.repeat(zeroSymbol, getMinimumFractionDigits());
+            result.repeat(digitSymbol, getMaximumFractionDigits() - getMinimumFractionDigits());
+            // Append exponent symbol and digits
+            if (useExponentialNotation) {
+                result.append(exponentSymbol);
+                result.repeat(zeroSymbol, minExponentDigits);
+            }
+            if (j == 1) {
+                // Append positive suffix pattern
+                appendAffix(result, posSuffixPattern, positiveSuffix, localized);
+                if (posEqualsNegPattern()) {
+                    // Negative pattern not needed if suffix/prefix are equivalent
+                    break;
+                }
+                result.append(patternSeparator);
+            } else {
+                appendAffix(result, negSuffixPattern, negativeSuffix, localized);
+            }
         }
         return result.toString();
     }
 
+    /**
+     * This method returns true if the positive and negative prefix/suffix
+     * values are equivalent. This is used to determine if an explicit NegativePattern
+     * is required.
+     */
+    private boolean posEqualsNegPattern() {
+        // Check suffix
+        return ((negSuffixPattern == posSuffixPattern && // n == p == null
+                negativeSuffix.equals(positiveSuffix))
+                || (negSuffixPattern != null &&
+                negSuffixPattern.equals(posSuffixPattern)))
+                && // Check prefix
+                ((negPrefixPattern != null && posPrefixPattern != null &&
+                negPrefixPattern.equals("'-" + posPrefixPattern)) ||
+                (negPrefixPattern == posPrefixPattern && // n == p == null
+                negativePrefix.equals(symbols.getMinusSignText() + positivePrefix)));
+    }
+
     /**
      * Apply the given pattern to this Format object.  A pattern is a
      * short-hand specification for the various formatting properties.
@@ -3712,7 +3732,9 @@ public class DecimalFormat extends NumberFormat {
             setMinimumIntegerDigits(0);
             setMaximumIntegerDigits(MAXIMUM_INTEGER_DIGITS);
             setMinimumFractionDigits(0);
-            setMaximumFractionDigits(MAXIMUM_FRACTION_DIGITS);
+            // As maxFracDigits are fully displayed unlike maxIntDigits
+            // Prevent OOME by setting to a much more reasonable value.
+            setMaximumFractionDigits(DOUBLE_FRACTION_DIGITS);
         }
 
         // If there was no negative pattern, or if the negative pattern is

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 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
@@ -23,25 +23,27 @@
 
 /*
  * @test
- * @bug 8282929
+ * @bug 8282929 8326908
- * @summary Verifies that toLocalizedPattern() method correctly returns
+ * @summary Verify DecimalFormat::toLocalizedPattern correctness.
- *          monetary symbols in a currency formatter
+ * @run junit ToLocalizedPatternTest
- * @run testng ToLocalizedPatternTest
  */
 
-import static org.testng.Assert.assertEquals;
+import org.junit.jupiter.api.Test;
-import org.testng.annotations.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
 import java.text.DecimalFormat;
 import java.text.DecimalFormatSymbols;
 import java.util.Locale;
 
-@Test
 public class ToLocalizedPatternTest {
     private static final char MONETARY_GROUPING = 'g';
     private static final char MONETARY_DECIMAL = 'd';
 
-    public void testToLocalizedPattern() {
+    // Verifies that the toLocalizedPattern() method correctly returns
+    // monetary symbols for a currency formatter.
+    @Test
+    public void monetarySymbolsTest() {
         var dfs = new DecimalFormatSymbols(Locale.US);
 
         // Customize the decimal format symbols
@@ -58,4 +60,17 @@ public class ToLocalizedPatternTest {
                .replace(',', MONETARY_GROUPING)
                .replace('.', MONETARY_DECIMAL));
     }
+
+    // Verify some common symbols are enforced with the DFS
+    @Test
+    public void useDFSWhenLocalizedTest() {
+        DecimalFormatSymbols dfs = new DecimalFormatSymbols();
+        dfs.setGroupingSeparator('a');
+        dfs.setDecimalSeparator('b');
+        dfs.setDigit('c');
+        dfs.setZeroDigit('d');
+        // Create a DecimalFormat that utilizes the above symbols
+        DecimalFormat dFmt = new DecimalFormat("###,##0.###", dfs);
+        assertEquals("caccdbccc", dFmt.toLocalizedPattern());
+    }
 }

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

@@ -0,0 +1,130 @@
+/*
+ * 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 8326908
+ * @summary Verify DecimalFormat::toPattern correctness.
+ * @run junit ToPatternTest
+ */
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.text.DecimalFormat;
+import java.util.stream.Stream;
+
+public class ToPatternTest {
+
+    // DecimalFormat constant
+    private static final int DOUBLE_FRACTION_DIGITS = 340;
+
+    // Ensure that toPattern() provides the correct amount of minimum
+    // and maximum digits for integer/fraction.
+    @ParameterizedTest
+    @MethodSource("minMaxDigits")
+    public void basicTest(int maxInt, int minInt, int maxFrac, int minFrac) {
+        DecimalFormat dFmt = new DecimalFormat();
+
+        dFmt.setMaximumIntegerDigits(maxInt);
+        dFmt.setMinimumIntegerDigits(minInt);
+        dFmt.setMaximumFractionDigits(maxFrac);
+        dFmt.setMinimumFractionDigits(minFrac);
+
+        // Non-localized separator always uses '.'
+        String[] patterns = dFmt.toPattern().split("\\.");
+        assertEquals(2, patterns.length,
+                dFmt.toPattern() + " should be split into an integer/fraction portion");
+        String integerPattern = patterns[0];
+        String fractionPattern = patterns[1];
+
+        // Count # and 0 explicitly (since there are grouping symbols)
+        assertEquals(integerPattern.chars().filter(ch -> ch == '0').count() +
+                integerPattern.chars().filter(ch -> ch == '#').count(),
+                Math.max(dFmt.getGroupingSize(), dFmt.getMinimumIntegerDigits()) + 1);
+        assertEquals(integerPattern.chars().filter(ch -> ch == '0').count(), dFmt.getMinimumIntegerDigits());
+        assertEquals(fractionPattern.length(), dFmt.getMaximumFractionDigits());
+        assertEquals(fractionPattern.chars().filter(ch -> ch == '0').count(), dFmt.getMinimumFractionDigits());
+    }
+
+    // General and edge cases for the min and max Integer/Fraction digits
+    private static Stream<Arguments> minMaxDigits() {
+        return Stream.of(
+                Arguments.of(10, 5, 10, 5),
+                Arguments.of(0, 0, 1, 1),
+                Arguments.of(1, 1, 1, 1),
+                Arguments.of(5, 5, 5, 5),
+                Arguments.of(5, 10, 5, 10),
+                Arguments.of(333, 27, 409, 3)
+        );
+    }
+
+    // Ensure that a NegativePattern is explicitly produced when required.
+    @Test
+    public void negativeSubPatternTest() {
+        DecimalFormat dFmt = new DecimalFormat();
+        dFmt.setPositivePrefix("foo");
+        dFmt.setPositiveSuffix("bar");
+        dFmt.setNegativePrefix("baz");
+        dFmt.setNegativeSuffix("qux");
+
+        String[] patterns = dFmt.toPattern().split(";");
+        assertEquals(2, patterns.length,
+                "There should be a positivePattern and negativePattern");
+        String positivePattern = patterns[0];
+        String negativePattern = patterns[1];
+
+        assertTrue(positivePattern.startsWith(dFmt.getPositivePrefix()));
+        assertTrue(positivePattern.endsWith(dFmt.getPositiveSuffix()));
+        assertTrue(negativePattern.startsWith(dFmt.getNegativePrefix()));
+        assertTrue(negativePattern.endsWith(dFmt.getNegativeSuffix()));
+    }
+
+    // 8326908: Verify that an empty pattern DecimalFormat does not throw an
+    // OutOfMemoryError when toPattern() is invoked. Behavioral change of
+    // MAXIMUM_INTEGER_DIGITS replaced with DOUBLE_FRACTION_DIGITS for empty
+    // pattern initialization.
+    @Test
+    public void emptyStringPatternTest() {
+        DecimalFormat empty = new DecimalFormat("");
+        // Verify new maximum fraction digits value
+        assertEquals(DOUBLE_FRACTION_DIGITS, empty.getMaximumFractionDigits());
+        // Verify no OOME for empty pattern
+        assertDoesNotThrow(empty::toPattern);
+        // Check toString for coverage, as it invokes toPattern
+        assertDoesNotThrow(empty::toString);
+    }
+
+    // Verify that only the last grouping interval is used for the grouping size.
+    @Test
+    public void groupingSizeTest() {
+        assertEquals(22,
+                new DecimalFormat( "###,####,"+"#".repeat(22)).getGroupingSize());
+    }
+}