From 6efdaf8ddf2940bcd5f96e114fe05b951ace313b Mon Sep 17 00:00:00 2001 From: Justin Lu Date: Fri, 8 Mar 2024 18:09:42 +0000 Subject: [PATCH] 8326908: DecimalFormat::toPattern throws OutOfMemoryError when pattern is empty string Reviewed-by: naoto --- .../classes/java/text/DecimalFormat.java | 130 ++++++++++-------- .../DecimalFormat/ToLocalizedPatternTest.java | 33 +++-- .../Format/DecimalFormat/ToPatternTest.java | 130 ++++++++++++++++++ 3 files changed, 230 insertions(+), 63 deletions(-) create mode 100644 test/jdk/java/text/Format/DecimalFormat/ToPatternTest.java diff --git a/src/java.base/share/classes/java/text/DecimalFormat.java b/src/java.base/share/classes/java/text/DecimalFormat.java index ef6f0796363..c235cf5b790 100644 --- a/src/java.base/share/classes/java/text/DecimalFormat.java +++ b/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 0; --i) { + if (i != digitCount && isGroupingUsed() && groupingSize != 0 && + i % groupingSize == 0) { + result.append(groupingSymbol); } - result.append(localized ? symbols.getPatternSeparator() : - PATTERN_SEPARATOR); - } else appendAffix(result, negSuffixPattern, negativeSuffix, localized); + result.append(i <= getMinimumIntegerDigits() ? zeroSymbol : digitSymbol); + } + // 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 diff --git a/test/jdk/java/text/Format/DecimalFormat/ToLocalizedPatternTest.java b/test/jdk/java/text/Format/DecimalFormat/ToLocalizedPatternTest.java index 6d8ab9c9a0c..664f3fdd951 100644 --- a/test/jdk/java/text/Format/DecimalFormat/ToLocalizedPatternTest.java +++ b/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 - * @summary Verifies that toLocalizedPattern() method correctly returns - * monetary symbols in a currency formatter - * @run testng ToLocalizedPatternTest + * @bug 8282929 8326908 + * @summary Verify DecimalFormat::toLocalizedPattern correctness. + * @run junit ToLocalizedPatternTest */ -import static org.testng.Assert.assertEquals; -import org.testng.annotations.Test; +import org.junit.jupiter.api.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()); + } } diff --git a/test/jdk/java/text/Format/DecimalFormat/ToPatternTest.java b/test/jdk/java/text/Format/DecimalFormat/ToPatternTest.java new file mode 100644 index 00000000000..c9749c09326 --- /dev/null +++ b/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 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()); + } +}