On Thu, 3 Oct 2024 00:48:55 GMT, Chen Liang <li...@openjdk.org> wrote:
>> Please review this PR which improves the safety of equality checking for >> DecimalFormatSymbols. >> >> DecimalFormatSymbols via its setters, allows certain instance variables to >> be set as null. (Note that some variables are allowed to be null, and others >> are not.) However, non null safe comparisons are used for all variables >> during the equality check. This can lead to an unexpected NPE when >> `DecimalFormatSymbols.equals()` is invoked. >> >> The nullable variables in question should be equality checked via the static >> `Object.equals()`. An associated regression test is added to confirm the >> change. > > Should we add an explicit whitelist of methods allowed to throw NPE, so that > if we add new getter/setters to DFS, we can avoid this nasty null problem for > the future? This whitelist will be validated by tests for sure. Thanks for taking a look @liach . https://github.com/openjdk/jdk/pull/21315/commits/bd6b021cc84034d55eb6d262ade7ea19e520c1fe adds a test that ensures if a new NPE throwing setter is added without being whitelisted, this test will now fail. This should prevent mishaps like this in the future. While this feels a little overkill, this enforcement is done via a test, so it is fine by me. ------------- PR Comment: https://git.openjdk.org/jdk/pull/21315#issuecomment-2392323915
