On Tue, 2 Sep 2025 18:55:23 GMT, Justin Lu <j...@openjdk.org> wrote:
>> This PR addresses a JCK test failure related to `DecimalFormatSymbols`
>> de-serialization. While the current public API of DFS disallows a null
>> locale, it was possible to set in the past. Thus, the
>> `loadNumberData(locale)` call currently throws NPE when locale is null in
>> the stream. The call should be guarded with a null check, such that if
>> locale is null, then `lenientMinusSigns` defaults to `minusSignText`.
>>
>> Defaulting the locale field when `null` to Locale.ROOT is also a reasonable
>> solution, but I think that the current one is preferable as a user would not
>> expect locale data related logic to occur if locale is `null`.
>
> Justin Lu has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Nested class rename
> - Add JDK regression test
Thanks for providing new tests.
test/jdk/java/text/Format/DecimalFormat/DFSSerializationTest.java line 140:
> 138: }
> 139:
> 140: // Previous versions of DFS could contain a null locale
"Previous" suggests it would not allow null with this change, which is not the
case.
test/jdk/java/text/Format/DecimalFormat/DFSSerializationTest.java line 224:
> 222: ObjectInputStream ois = new
> ObjectInputStream(byteArrayInputStream)) {
> 223: return (DecimalFormatSymbols) ois.readObject();
> 224: }
I guess if an error occured in this try block, the test should fail. Just
wondering the reason it did not use `assertDoesNotThrow()` method.
-------------
PR Review: https://git.openjdk.org/jdk/pull/27008#pullrequestreview-3177975352
PR Review Comment: https://git.openjdk.org/jdk/pull/27008#discussion_r2317077376
PR Review Comment: https://git.openjdk.org/jdk/pull/27008#discussion_r2317062931
