On Fri, 8 Mar 2024 08:22:21 GMT, Hai-May Chao <hc...@openjdk.org> wrote:

>> Change was made to engineGetEntry() in PKCS12KeyStore to extract the key and 
>> certificate chain from Entry only once. This is because the entry may get 
>> updated between engineGetKey() and engineGetCertificateChain() which causes 
>> inconsistent result. A new test was added to assess and manipulate 
>> PKCS12KeyStore with read and write operations concurrently from multiple 
>> threads. Thanks!
>
> Hai-May Chao has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update engineDeleteEntry

Very nice improvement. Thank you!

src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 1331:

> 1329:             if (internalEngineIsCertificateEntry(entry)) {
> 1330:                 if (entry instanceof CertEntry &&
> 1331:                     ((CertEntry) entry).trustedKeyUsage != null) {

`internalEngineIsCertificateEntry` performs exactly the same checks; you can 
remove these lines.

src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 1368:

> 1366:                         entry.attributes);
> 1367:                 }
> 1368:             } else if (!internalEngineIsKeyEntry(entry)) {

Suggestion:

            } else {


(the matching `if` already checks `internalEngineIsKeyEntry(entry)`)

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

PR Review: https://git.openjdk.org/jdk/pull/18156#pullrequestreview-1924367027
PR Review Comment: https://git.openjdk.org/jdk/pull/18156#discussion_r1517406770
PR Review Comment: https://git.openjdk.org/jdk/pull/18156#discussion_r1517411548

Reply via email to