On Tue, 5 Mar 2024 00:32:47 GMT, Justin Lu <j...@openjdk.org> wrote: >> Please review this PR and corresponding CSR which prevents an >> OutOfMemoryError by restricting the initial maximum fraction digits for an >> empty pattern DecimalFormat. >> >> For an empty String pattern DecimalFormat, the maximum fraction digits is >> initialized to `Integer.MAX_VALUE`. When toPattern() is invoked, >> StringBuilder internally doubles capacity attempting to append >> Integer.MAX_VALUE digits until OOME occurs. CSR covers potential behavioral >> compatibility changes. > > Justin Lu has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains five additional commits since > the last revision: > > - cleanup/typo > - Merge branch 'master' into JDK-8326908-emptyPattern-OOME > - implement feedback + improve pattern related tests > - minor additions > - init
src/java.base/share/classes/java/text/DecimalFormat.java line 3367: > 3365: } > 3366: } > 3367: return false; can simply be a single return statement. test/jdk/java/text/Format/DecimalFormat/ToPatternTest.java line 60: > 58: dFmt.setMinimumFractionDigits(minFrac); > 59: > 60: String[] patterns = dFmt.toPattern().split("\\."); Instead of assuming/hardcoding the decimal separator, `DecimalFormatSymbols.getInstance(Locale.US).getDecimalSeparator()` may be better. test/jdk/java/text/Format/DecimalFormat/ToPatternTest.java line 110: > 108: // 8326908: Verify that an empty pattern DecimalFormat does not > throw an > 109: // OutOfMemoryError when toPattern() is invoked. Behavioral change of > 110: // MAXIMUM_INTEGER_DIGITS replaced with DOUBLE_FRACTION_DIGITS for > empty Should we explicitly check `new DecimalFormat("").getMaximumFractionDigits() == DOUBLE_FRACTION_DIGITS`? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18094#discussion_r1513356393 PR Review Comment: https://git.openjdk.org/jdk/pull/18094#discussion_r1513364152 PR Review Comment: https://git.openjdk.org/jdk/pull/18094#discussion_r1513378668