On Fri, 8 Mar 2024 21:15:45 GMT, Hai-May Chao <[email protected]> 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 with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains six additional
> commits since the last revision:
>
> - Merge
> - remove unneeded checks in engineGetEntry
> - Update engineDeleteEntry
> - Update engineIsKeyEntry and engineIsCertificateEntry
> - Update bug number in the test
> - 8327461: KeyStore getEntry is not thread-safe
src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 301:
> 299: Entry entry = entries.get(alias.toLowerCase(Locale.ENGLISH));
> 300: Key entryKey = internalGetKey(entry, password);
> 301: return entryKey;
Combine the 2 lines above.
src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 480:
> 478: Entry entry = entries.get(alias.toLowerCase(Locale.ENGLISH));
> 479: Certificate[] certChain = internalGetCertificateChain(entry);
> 480: return certChain;
Combine the two lines above.
src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 1027:
> 1025:
> 1026: Entry entry = entries.remove(alias.toLowerCase(Locale.ENGLISH));
> 1027: if (entry != null) {
No need to check `entry != null`.
src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 1082:
> 1080: }
> 1081:
> 1082: private boolean internalEngineIsKeyEntry(Entry entry) {
No need to have both `internal` and `Engine` in the method name. Use `internal`
only to follow the other method names. Same with
`internalEngineIsCertificateEntry` below.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18156#discussion_r1518348960
PR Review Comment: https://git.openjdk.org/jdk/pull/18156#discussion_r1518349769
PR Review Comment: https://git.openjdk.org/jdk/pull/18156#discussion_r1518350934
PR Review Comment: https://git.openjdk.org/jdk/pull/18156#discussion_r1518355653
