On Thu, 7 Nov 2024 18:46:42 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> Prasadrao Koppula has updated the pull request with a new target base due to >> a merge or a rebase. The pull request now contains 11 commits: >> >> - Merge master >> - initialized storeName with empty string >> - Replaced Paths.get with Path.of >> - Removed unnecessary code >> - Removed unnecessary code >> - Handled nested wrappers around FileInputStream >> - Handled BIS case as well >> - JDK-8329251 >> - JDK-8329251 >> - JDK-8329251 >> - ... and 1 more: https://git.openjdk.org/jdk/compare/f2316f68...c90b4f30 > > src/java.base/share/classes/javax/net/ssl/TrustManagerFactory.java line 283: > >> 281: if (ks != null && SSLLogger.isOn && >> SSLLogger.isOn("trustmanager")) { >> 282: String keystorePath = SharedSecrets >> 283: .getJavaSecurityKeyStoreAccess() > > This code only works if `java.security.debug=keystore` is also enabled, > otherwise it will always return `null`. I think that dependency is not > intuitive, and will be hard for users to remember. I think you should change > `KeyStore.java` to always cache the filename, whether debug is on or not. The primary concern is the repeated calls to SharedSecrets to access the inputStream, even though the keystore name is needed in minimal cases ( only when debug enabled) . To optimize, we can cache the filename and access it only when debugging is enabled, reducing unnecessary overhead in the common case. I agree that dependency is not intuitive and hard for users to remember. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20414#discussion_r1841634533
