On Sun, 17 Nov 2024 14:52:06 GMT, Eirik Bjørsnøs <[email protected]> wrote:
> Please review this PR which removes exceptional control flow in > `Boolean::getBoolean`, `Integer::getInteger` and `Long::getLong`. > > These methods are catching `IllegalArgumentException` and > `NullPointerException`, thrown by `System::getProperty` via > `System::checkKey`. This PR replaces the exceptional control flow with > explicit checks that the system property name is non-null and non-empty > before calling into `System::getProperty`. > > As JDK-8178966 points out, there is a possibility that System.getProperty > could throw one of NPE and IAE for other reasons than the name being null or > empty. This risk is reduced now that custom security managers cannot > interfere. Adding to that, if such exceptions are thrown today, they are > masked by these methods catching and swallowing them by returning false or > default values. It's better to expose such bugs if they exist. > > GHA results pending. Local tier2 ran successfully. Hello Eirik, this looks reasonable to me. Is this something that you want to add Peter as a contributor to, given his original proposal in the issue? I can't think of any reason not to do this change, but it would be good to have one more review for this one. So please wait for one before integrating. ------------- Marked as reviewed by jpai (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/22183#pullrequestreview-2442238986
