fix: upgrade icu4j 63.1 -> 71.1 to address CVE-2018-18928

[yesamer]

Upgrade com.ibm.icu:icu4j from 63.1 to 71.1 to fix CVE-2018-18928, an integer overflow in DecimalQuantity::toScientificString() present in ICU 63.1. The patch applies to both the C/C++ and Java ports and was first shipped in the 64.1 release.

Closes #9959

---

Tools repository PR: gwtproject/tools#43

[zbynek]

As is, this will break the build. Could you please file a PR for https://github.com/gwtproject/tools to add the new version, then update the CI files here, similar to #10115 ?

[yesamer]

As is, this will break the build. Could you please file a PR for https://github.com/gwtproject/tools to add the new version, then update the CI files here, similar to #10115 ?

@zbynek Sure, I will.

[yesamer]

Done! @zbynek

[zbynek]

Thanks. Also, after checking the available versions, the latest is 78.3. Is there any reason for picking 64.1? Note that GWT in general is not constrained to Java 8 anymore, any version compatible with Java 17 should be fine.

[yesamer]

@zbynek my strategy is to upgrade to the first version that is not affected by the CVE.
If you think it would be worthwhile, I can also investigate the impact of upgrading to the latest version.

[zbynek]

@niloc132 any peference for the used version?

[niloc132]

I'm content with "not vulnerable", though I dont think we're vulnerable anyway? If you're including malicious source in your project anyway, it can already do anything. How is an int overflow a 9.8 anyway though?

We can definitely do another bump later to something recent, not targeting to fix a vuln.

[yesamer]

@zbynek @niloc132
Sharing my findings:
We can safely upgrade up to version 74.2 with no breaking changes detected.

Since 75.1, a breaking change impacts GenerateGwtCldrData: com.ibm.icu.dev.tool.UOption was removed from ICU4J as part of ICU-21757 ("split out utilities.jar"), which eliminated utilities.jar entirely. As a consequence, GenerateGwtCldrData would require refactoring to replace UOption with an alternative CLI parser (ICU themselves migrated to Apache Commons CLI).

Based on this, I think upgrading to 74.2 is the best compromise for this scope. WDYT?

[yesamer]

Thank you for the fix @zbynek

[zbynek]

The failing tests seem relevant, though the error messages are misleading:

🧪 - gwt/user/test/com/google/gwt/i18n/I18N2Test.gwt.xml | expected: <in timezone: 2/1/2010, 7:04:05 PM>, actual: <in timezone: 2/1/2010, 7:04:05 PM>
🧪 - gwt/user/test/com/google/gwt/i18n/I18N2Test.gwt.xml | expected: <in GMT: 2/2/2010, 3:04:05 AM>, actual: <in GMT: 2/2/2010, 3:04:05 AM>

The expected string has an ASCII space, the returned string a \u202f.

[yesamer]

@zbynek
Latest ICU versions intentionally changed the en time pattern — the separator between the time and the AM/PM marker is now U+202F instead of U+0020.

The pattern is produced at GWT compile time by DateTimePatternGenerator.getBestPattern("hms"), which delegates to ICU's DateTimePatternGenerator. The result (h:mm:ss\u202fa) gets baked into generated code. At runtime, GWT's DateTimeFormat.parsePattern() only recognises plain ASCII space as whitespace, so \u202f passes through as a literal character and appears verbatim in the formatted output.

I'm investigating possible solutions, feel free to suggest any, in case.

[zbynek]

, feel free to suggest any,

I guess we have 3 options:

1. say this is an improvement and update the test -- @niloc132 any concern about that?
2. check if there is a version of ICU that does not trigger security warnings and keeps the old format
3. add some String::replaceAll to emulate the old behavior

[niloc132]

On the one hand, it makes good sense to force a nonbreaking space between the time and the AM/PM marker, but it is a change that might surprise some teams.

I do like emulating old behavior, but I think it is also reasonable to default to new behavior? It is adding complexity though.

Aside from something relatively heavy like a config property to control this, could we let this be customizable in some way? It looks like "no" at a quick glance, or at least not in our DateTimePatternGenerator wrapper type.

@yesamer if you don't want to try to work out some of this, maybe try with 64.1 as you initially did, and see if that still breaks this test, then we can split off the two changes (UOption and this nbsp issue) for later followup?

[yesamer]

@zbynek @niloc132, thank you for your guidance and suggestions. I had the same concerns about how to handle this.
At first glance, updating the test seemed like a reasonable option, as the change is technically valid. However, I was concerned that doing so could introduce a breaking change and affect compatibility.
Another possibility would be to replace the old whitespace character with the new one when delegating to ICU, but that feels more like a workaround than a proper solution.
I also investigated whether ICU could be configured to preserve the 63.1 behavior, but I was unable to find a suitable option.
Given that, my proposal would be to upgrade to ICU 71.1, which appears to be the latest version that still preserves the old whitespace behavior.