On Mon, 22 May 2023 17:17:26 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> Jamil Nimeh has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Use privilegedGetProperty, catch NFE following string match > > src/java.base/share/classes/sun/security/action/GetPropertyAction.java line > 211: > >> 209: " for timeout value " + propVal + >> 210: ". Using default value of " + def + " >> msec."); >> 211: } > > This would also be useful debug for the else case on line 214-216 if the > value is not an integer. Sounds good, I can add that. > src/java.base/share/classes/sun/security/provider/certpath/URICertStore.java > line 131: > >> 129: private static final int DEFAULT_CRL_READ_TIMEOUT = 15000; >> 130: >> 131: // Default connect and read timeouts for CA certificate fetching >> (15 sec) > > Does 15 seconds make sense as the default timeout, especially for certs? CRLs > are generally larger than certs, so a longer read timeout makes sense. > > I'm ok with keeping these default values the same for consistency, but I > think we should re-evaluate each of these default timeouts and compare them > to other products/technologies to see if some adjustments may be needed - can > you file a follow-on RFE for that? Yes, I can make a follow on for that. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13762#discussion_r1200831410 PR Review Comment: https://git.openjdk.org/jdk/pull/13762#discussion_r1200832770
