<i18n dev> RFR: JDK-8326908: DecimalFormat::toPattern throws OutOfMemoryError when pattern is empty string [v2]

Justin Lu jlu at openjdk.org
Tue Mar 5 21:27:11 UTC 2024


On Tue, 5 Mar 2024 19:10:19 GMT, Naoto Sato <naoto at openjdk.org> wrote:

>> 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
>
> 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.

Actually, since `toPattern()` is non-localized, it will always use "." as specified by DecimalFormat.

I removed the misleading comment `// Use a US dFmt, which uses '.' as the decimal separator` that indicated we needed to use a DecimalFormatSymbols where "." was the decimal separator. Can just simply use a DecimalFormat returned by the default constructor.

> 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`?

Thanks for the review, added an explicit check

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18094#discussion_r1513517348
PR Review Comment: https://git.openjdk.org/jdk/pull/18094#discussion_r1513517336


More information about the i18n-dev mailing list