On Fri, 8 Mar 2024 21:15:45 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 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