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

Justin Lu jlu at openjdk.org
Tue Mar 5 00:32:48 UTC 2024


On Mon, 4 Mar 2024 16:03:55 GMT, Roger Riggs <rriggs 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
>
> src/java.base/share/classes/java/text/DecimalFormat.java line 3717:
> 
>> 3715:             // As maxFracDigits are fully displayed unlike maxIntDigits
>> 3716:             // Prevent OOME by setting to a much more reasonable value.
>> 3717:             setMaximumFractionDigits(DOUBLE_FRACTION_DIGITS);
> 
> Setting a reasonable default makes sense.  
> In other control paths, the max fraction digits come from the inputs or are explicitly set.
> 
> It might be a reasonable related change to use StringBuilder.repeat() instead of a loop at LIne 3312-3319, where the pattern char(s) are being appended to the result.

Thanks for taking a look. Updated the loop with `repeat` as you suggested, and decided to refactor the rest of the method while I was at it.

Additionally, I added some more tests, as it seems that there is a lack of pattern tests for DecimalFormat in general.

> In other control paths, the max fraction digits come from the inputs or are explicitly set.

Right, while `Integer.MAX_VALUE` can still be set by other control paths (and thus an OOME if toPattern() is invoked), this is something explicitly done by the user, and thus we decided we would still allow the behavior.

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

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


More information about the core-libs-dev mailing list