On Mon, 22 May 2023 15:58:14 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> Jamil Nimeh has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add OCSP readtimeout property
>
> src/java.base/share/classes/sun/security/action/GetPropertyAction.java line
> 186:
>
>> 184: }
>> 185:
>> 186: String propVal = System.getProperty(prop, "").trim();
>
> You should call `privilegedGetProperty` here instead of `System.getProperty`
> so the call is wrapped in `doPrivileged` when an SM is active.
Good catch. Will fix.
> src/java.base/share/classes/sun/security/action/GetPropertyAction.java line
> 202:
>
>> 200: // Next check to make sure the string is built only from digits
>> 201: if (propVal.matches("^\\d+$")) {
>> 202: int timeout = Integer.parseInt(propVal);
>
> Is this guaranteed never to throw `NumberFormatException`? It might be safer
> to catch it just in case.
I'll change this to catch NFE, but I'm pretty sure the pattern will only ever
return on true if the string is comprised solely of digits from start to end -
I could never get a string that would pass when it shouldn't. But point taken,
better safe than sorry.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13762#discussion_r1200729759
PR Review Comment: https://git.openjdk.org/jdk/pull/13762#discussion_r1200728908
