On Thu, 14 Nov 2024 06:28:25 GMT, Prasadrao Koppula <pkopp...@openjdk.org> 
wrote:

>> 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.

Perhaps you could cache the `InputStream` instead in `KeyStore`, and only use 
shared secrets to get the pathname as needed when logging is enabled in the TLS 
code.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20414#discussion_r1844521411

Reply via email to