Integrated: Merge jdk16
On Thu, 21 Jan 2021 04:33:47 GMT, Jesper Wilhelmsson wrote: > Forwardport JDK 16 -> JDK 17 This pull request has now been integrated. Changeset: 133bcb09 Author:Jesper Wilhelmsson URL: https://git.openjdk.java.net/jdk/commit/133bcb09 Stats: 296 lines in 31 files changed: 200 ins; 16 del; 80 mod Merge - PR: https://git.openjdk.java.net/jdk/pull/2176
RFR: Merge jdk16
Forwardport JDK 16 -> JDK 17 - Commit messages: - Merge - 8259757: add a regression test for 8259353 and 8259601 - 8259732: JDK 16 L10n resource file update - msg drop 10 The webrevs contain the adjustments done while merging with regards to each parent branch: - master: https://webrevs.openjdk.java.net/?repo=jdk=2176=00.0 - jdk16: https://webrevs.openjdk.java.net/?repo=jdk=2176=00.1 Changes: https://git.openjdk.java.net/jdk/pull/2176/files Stats: 296 lines in 31 files changed: 200 ins; 16 del; 80 mod Patch: https://git.openjdk.java.net/jdk/pull/2176.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2176/head:pull/2176 PR: https://git.openjdk.java.net/jdk/pull/2176
Integrated: 8259498: Reduce overhead of MD5 and SHA digests
On Sun, 20 Dec 2020 20:27:03 GMT, Claes Redestad wrote: > - The MD5 intrinsics added by > [JDK-8250902](https://bugs.openjdk.java.net/browse/JDK-8250902) shows that > the `int[] x` isn't actually needed. This also applies to the SHA intrinsics > from which the MD5 intrinsic takes inspiration > - Using VarHandles we can simplify the code in `ByteArrayAccess` enough to > make it acceptable to use inline and replace the array in MD5 wholesale. This > improves performance both in the presence and the absence of the intrinsic > optimization. > - Doing the exact same thing in the SHA impls would be unwieldy (64+ element > arrays), but allocating the array lazily gets most of the speed-up in the > presence of an intrinsic while being neutral in its absence. > > Baseline: > (digesterName) (length)Cnt Score > Error Units > MessageDigests.digestMD516 15 > 2714.307 ± 21.133 ops/ms > MessageDigests.digestMD5 1024 15 > 318.087 ±0.637 ops/ms > MessageDigests.digest SHA-116 15 > 1387.266 ± 40.932 ops/ms > MessageDigests.digest SHA-1 1024 15 > 109.273 ±0.149 ops/ms > MessageDigests.digestSHA-25616 15 > 995.566 ± 21.186 ops/ms > MessageDigests.digestSHA-256 1024 15 > 89.104 ±0.079 ops/ms > MessageDigests.digestSHA-51216 15 > 803.030 ± 15.722 ops/ms > MessageDigests.digestSHA-512 1024 15 > 115.611 ±0.234 ops/ms > MessageDigests.getAndDigest MD516 15 > 2190.367 ± 97.037 ops/ms > MessageDigests.getAndDigest MD5 1024 15 > 302.903 ±1.809 ops/ms > MessageDigests.getAndDigestSHA-116 15 > 1262.656 ± 43.751 ops/ms > MessageDigests.getAndDigestSHA-1 1024 15 > 104.889 ±3.554 ops/ms > MessageDigests.getAndDigest SHA-25616 15 > 914.541 ± 55.621 ops/ms > MessageDigests.getAndDigest SHA-256 1024 15 > 85.708 ±1.394 ops/ms > MessageDigests.getAndDigest SHA-51216 15 > 737.719 ± 53.671 ops/ms > MessageDigests.getAndDigest SHA-512 1024 15 > 112.307 ±1.950 ops/ms > > GC: > MessageDigests.getAndDigest:·gc.alloc.rate.norm MD516 15 > 312.011 ±0.005B/op > MessageDigests.getAndDigest:·gc.alloc.rate.normSHA-116 15 > 584.020 ±0.006B/op > MessageDigests.getAndDigest:·gc.alloc.rate.norm SHA-25616 15 > 544.019 ±0.016B/op > MessageDigests.getAndDigest:·gc.alloc.rate.norm SHA-51216 15 > 1056.037 ±0.003B/op > > Target: > Benchmark (digesterName) (length)Cnt > Score Error Units > MessageDigests.digestMD516 15 > 3134.462 ± 43.685 ops/ms > MessageDigests.digestMD5 1024 15 > 323.667 ±0.633 ops/ms > MessageDigests.digest SHA-116 15 > 1418.742 ± 38.223 ops/ms > MessageDigests.digest SHA-1 1024 15 > 110.178 ±0.788 ops/ms > MessageDigests.digestSHA-25616 15 > 1037.949 ± 21.214 ops/ms > MessageDigests.digestSHA-256 1024 15 > 89.671 ±0.228 ops/ms > MessageDigests.digestSHA-51216 15 > 812.028 ± 39.489 ops/ms > MessageDigests.digestSHA-512 1024 15 > 116.738 ±0.249 ops/ms > MessageDigests.getAndDigest MD516 15 > 2314.379 ± 229.294 ops/ms > MessageDigests.getAndDigest MD5 1024 15 > 307.835 ±5.730 ops/ms > MessageDigests.getAndDigestSHA-116 15 > 1326.887 ± 63.263 ops/ms > MessageDigests.getAndDigestSHA-1 1024 15 > 106.611 ±2.292 ops/ms > MessageDigests.getAndDigest SHA-25616 15 > 961.589 ± 82.052 ops/ms > MessageDigests.getAndDigest SHA-256 1024 15 > 88.646 ±0.194 ops/ms > MessageDigests.getAndDigest SHA-51216 15 > 775.417 ± 56.775 ops/ms > MessageDigests.getAndDigest SHA-512 1024 15 > 112.904 ±2.014 ops/ms > > GC > MessageDigests.getAndDigest:·gc.alloc.rate.norm MD516 15 > 232.009 ±
Integrated: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension
On Mon, 11 Jan 2021 21:41:56 GMT, Hai-May Chao wrote: > This enhancement adds support for the nonce extension in OCSP request > extensions by system property jdk.security.certpath.ocspNonce. > > Please review the CSR at: > https://bugs.openjdk.java.net/browse/JDK-8257766 This pull request has now been integrated. Changeset: 8b95d954 Author:Hai-May Chao Committer: Sean Mullan URL: https://git.openjdk.java.net/jdk/commit/8b95d954 Stats: 83 lines in 5 files changed: 59 ins; 6 del; 18 mod 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension Reviewed-by: jnimeh, mullan - PR: https://git.openjdk.java.net/jdk/pull/2039
Re: RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension [v7]
> This enhancement adds support for the nonce extension in OCSP request > extensions by system property jdk.security.certpath.ocspNonce. > > Please review the CSR at: > https://bugs.openjdk.java.net/browse/JDK-8257766 Hai-May Chao has updated the pull request incrementally with one additional commit since the last revision: Set tmpExtensions to null instead of using emptyList() - Changes: - all: https://git.openjdk.java.net/jdk/pull/2039/files - new: https://git.openjdk.java.net/jdk/pull/2039/files/b4b3a485..b334a5d0 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2039=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2039=05-06 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2039.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2039/head:pull/2039 PR: https://git.openjdk.java.net/jdk/pull/2039
Re: RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension [v6]
On Wed, 20 Jan 2021 21:40:12 GMT, Sean Mullan wrote: >> Hai-May Chao has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Change to save memory by List.of > > src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java > line 749: > >> 747: } >> 748: >> 749: List tmpExtensions = >> Collections.emptyList(); > > One other comment - I think you can just set this to null. Fixed. - PR: https://git.openjdk.java.net/jdk/pull/2039
Re: RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension [v6]
On Wed, 20 Jan 2021 21:37:10 GMT, Hai-May Chao wrote: >> This enhancement adds support for the nonce extension in OCSP request >> extensions by system property jdk.security.certpath.ocspNonce. >> >> Please review the CSR at: >> https://bugs.openjdk.java.net/browse/JDK-8257766 > > Hai-May Chao has updated the pull request incrementally with one additional > commit since the last revision: > > Change to save memory by List.of src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java line 749: > 747: } > 748: > 749: List tmpExtensions = > Collections.emptyList(); One other comment - I think you can just set this to null. - PR: https://git.openjdk.java.net/jdk/pull/2039
Re: RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension [v6]
> This enhancement adds support for the nonce extension in OCSP request > extensions by system property jdk.security.certpath.ocspNonce. > > Please review the CSR at: > https://bugs.openjdk.java.net/browse/JDK-8257766 Hai-May Chao has updated the pull request incrementally with one additional commit since the last revision: Change to save memory by List.of - Changes: - all: https://git.openjdk.java.net/jdk/pull/2039/files - new: https://git.openjdk.java.net/jdk/pull/2039/files/a14a2bdb..b4b3a485 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2039=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2039=04-05 Stats: 4 lines in 1 file changed: 1 ins; 2 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2039.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2039/head:pull/2039 PR: https://git.openjdk.java.net/jdk/pull/2039
Re: RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension [v5]
On Wed, 20 Jan 2021 20:57:49 GMT, Sean Mullan wrote:
>> Hai-May Chao has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add nonce to the list of extensions
>
> src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java
> line 762:
>
>> 760: }
>> 761:
>> 762: tmpExtensions.add(nonceExt);
>
> If you only need the nonce, you could use List.of and save a little bit of
> memory, ex:
>
> if (ocspExtensions.size() > 0) {
> tmpExtensions = new
> ArrayList(ocspExtensions);
> tmpExtensions.add(nonceExt);
> } else {
> tmpExtensions = List.of(nonceExt);
> }
Thanks for the review. Updated as suggested.
-
PR: https://git.openjdk.java.net/jdk/pull/2039
Re: RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension [v5]
On Wed, 20 Jan 2021 19:37:10 GMT, Hai-May Chao wrote:
>> This enhancement adds support for the nonce extension in OCSP request
>> extensions by system property jdk.security.certpath.ocspNonce.
>>
>> Please review the CSR at:
>> https://bugs.openjdk.java.net/browse/JDK-8257766
>
> Hai-May Chao has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Add nonce to the list of extensions
Marked as reviewed by mullan (Reviewer).
src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java
line 762:
> 760: }
> 761:
> 762: tmpExtensions.add(nonceExt);
If you only need the nonce, you could use List.of and save a little bit of
memory, ex:
if (ocspExtensions.size() > 0) {
tmpExtensions = new
ArrayList(ocspExtensions);
tmpExtensions.add(nonceExt);
} else {
tmpExtensions = List.of(nonceExt);
}
-
PR: https://git.openjdk.java.net/jdk/pull/2039
Re: RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension [v4]
On Wed, 20 Jan 2021 13:46:58 GMT, Sean Mullan wrote: >> Hai-May Chao has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Nonce creation is done in checkOCSP method > > src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java > line 755: > >> 753: // create the 16-byte nonce by default >> 754: Extension nonceExt = new >> OCSPNonceExtension(DEFAULT_NONCE_BYTES); >> 755: tmpExtensions.add(nonceExt); > > I think you should add the OCSPNonce extension to the list of extensions that > the application passed in, as there may be other extensions that have been > specified and should be sent in the OCSP response, ex: > > `ocspExtensions.add(new OCSPNonceExtension(DEFAULT_NONCE_BYTES));` > > This means you don't need the `tmpExtensions` variable. During testing, I got the "java.base/java.util.Collections$UnmodifiableCollection.add(Collections.java:1062) exception with this line of change. I've changed to use a tmpExtensions variable when setting the OCSP nonce to the extension sets instead of modifying the ocspExtensions. > src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java > line 779: > >> 777: response = OCSP.check(Collections.singletonList(certId), >> 778: responderURI, issuerInfo, responderCert, null, >> 779: rp.ocspNonce ? tmpExtensions : ocspExtensions, >> params.variant()); > > Here you can just pass in `ocspExtensions` since it will contain the nonce if > the property has been set. No change as tmpExtensions is needed. - PR: https://git.openjdk.java.net/jdk/pull/2039
Re: RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension [v4]
On Wed, 20 Jan 2021 13:41:04 GMT, Sean Mullan wrote:
>> Hai-May Chao has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Nonce creation is done in checkOCSP method
>
> src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java
> line 762:
>
>> 760: } catch (IOException e) {
>> 761: throw new
>> CertPathValidatorException("Failed to create the default nonce " +
>> 762: "in OCSP entensions");
>
> Typo: s/entensions/extensions/
>
> Also, use the `CertPathValidatorException(String, Throwable)` ctor instead
> and pass the `IOException` as the 2nd parameter.
Fixed.
-
PR: https://git.openjdk.java.net/jdk/pull/2039
Re: RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension [v5]
> This enhancement adds support for the nonce extension in OCSP request > extensions by system property jdk.security.certpath.ocspNonce. > > Please review the CSR at: > https://bugs.openjdk.java.net/browse/JDK-8257766 Hai-May Chao has updated the pull request incrementally with one additional commit since the last revision: Add nonce to the list of extensions - Changes: - all: https://git.openjdk.java.net/jdk/pull/2039/files - new: https://git.openjdk.java.net/jdk/pull/2039/files/da6e5bab..a14a2bdb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2039=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2039=03-04 Stats: 9 lines in 1 file changed: 7 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2039.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2039/head:pull/2039 PR: https://git.openjdk.java.net/jdk/pull/2039
Integrated: JDK-8259786: initialize last parameter of getpwuid_r
On Fri, 15 Jan 2021 11:24:49 GMT, Matthias Baesken wrote: > We have a couple of calls to getpwuid_r in the codebase, like > g= getpwuid_r(getuid(), , pwd_buf, sizeof(pwd_buf), ); > > Usually we NULL-check pwd after the call because we do not fully trust the > return code of the function (it is documented in the codebase why we do not > fully trust the return code) . However we miss to initialize pwd at some > places before the call, which might we a little problematic and should be > improved (at other places we already initialize it). > > This triggers also Sonar warnings like : > https://sonarcloud.io/project/issues?id=jdk=AXaE0dsA8L9hkQskGEbA=false=BUG > > > Aside from this issue , should we in other issue , unify the OS versions of > static char* get_user_name(uid_t uid)in posix code (currently we have it > for bsd, linux, aix but the functions look very similar ? This pull request has now been integrated. Changeset: 52ed2aab Author:Matthias Baesken URL: https://git.openjdk.java.net/jdk/commit/52ed2aab Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod 8259786: initialize last parameter of getpwuid_r Reviewed-by: mdoerr, hseigel - PR: https://git.openjdk.java.net/jdk/pull/2098
[jdk16] Integrated: JDK-8259732: JDK 16 L10n resource file update - msg drop 10
On Thu, 14 Jan 2021 14:00:00 GMT, Leo Jiang wrote: > This is the changes for JDK 16 msg drop 10. This pull request has now been integrated. Changeset: 01205109 Author:Leo Jiang URL: https://git.openjdk.java.net/jdk16/commit/01205109 Stats: 215 lines in 30 files changed: 118 ins; 16 del; 81 mod 8259732: JDK 16 L10n resource file update - msg drop 10 Reviewed-by: naoto - PR: https://git.openjdk.java.net/jdk16/pull/123
Re: RFR: 8258833: Cancel multi-part cipher operations in SunPKCS11 after failures [v7]
On Wed, 20 Jan 2021 13:47:13 GMT, Martin Balao wrote:
>> When a multi-part cipher operation fails in SunPKCS11 (i.e. because of an
>> invalid block size), we now cancel the operation before returning the
>> underlying Session to the Session Manager. This allows to use the returned
>> Session for a different purpose. Otherwise, an CKR_OPERATION_ACTIVE error
>> would be raised from the PKCS#11 library.
>>
>> The jdk/sun/security/pkcs11/Cipher/CancelMultipart.java regression test is
>> introduced as part of this PR.
>>
>> No regressions found in jdk/sun/security/pkcs11.
>
> Martin Balao has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Align doCancel pattern in 'P11Cipher::implDoFinal(byte[]..' to
> 'P11Cipher::implDoFinal(ByteBuffer..'. Better documentation in P11Cipher.
> Copyright date updated.
> - Copyright dates updated to 2021 on modified files
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java line
793:
> 791: // only after this point. See JDK-8258833 for further
> 792: // information.
> 793: doCancel = false;
@valeriepeng I made a code change here so I'd like you to have a final look and
validate. I'm just aligning the 'P11Cipher::implDoFinal(byte[]..' function to
'P11Cipher::implDoFinal(ByteBuffer..'. The rationale is that 'doFalse = false'
can be placed before the C_EncryptFinal call because any error on it does not
require a cancel (it already cancels the operation)
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java line
812:
> 810: // only after this point. See JDK-8258833 for further
> 811: // information.
> 812: doCancel = false;
Same comment that for line 793 of P11Cipher
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java line
820:
> 818: System.arraycopy(padBuffer, 0, out, outOfs, k);
> 819: } else {
> 820: doCancel = false;
Same comment than for line 793 of P11Cipher
-
PR: https://git.openjdk.java.net/jdk/pull/1901
Re: RFR: JDK-8259786: initialize last parameter of getpwuid_r [v4]
On Wed, 20 Jan 2021 12:05:03 GMT, Matthias Baesken wrote: >> We have a couple of calls to getpwuid_r in the codebase, like >> g= getpwuid_r(getuid(), , pwd_buf, sizeof(pwd_buf), ); >> >> Usually we NULL-check pwd after the call because we do not fully trust the >> return code of the function (it is documented in the codebase why we do not >> fully trust the return code) . However we miss to initialize pwd at some >> places before the call, which might we a little problematic and should be >> improved (at other places we already initialize it). >> >> This triggers also Sonar warnings like : >> https://sonarcloud.io/project/issues?id=jdk=AXaE0dsA8L9hkQskGEbA=false=BUG >> >> >> Aside from this issue , should we in other issue , unify the OS versions >> of static char* get_user_name(uid_t uid)in posix code (currently we >> have it for bsd, linux, aix but the functions look very similar ? > > Matthias Baesken has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains five commits: > > - bring initialization to perfMemory_posix > - Merge branch 'master' into JDK-8259786 > - More revert > - revert perfMemory changes > - JDK-8259786 Changes look good! Thanks, Harold - Marked as reviewed by hseigel (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2098
Re: RFR: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension [v4]
On Fri, 15 Jan 2021 23:09:26 GMT, Hai-May Chao wrote:
>> This enhancement adds support for the nonce extension in OCSP request
>> extensions by system property jdk.security.certpath.ocspNonce.
>>
>> Please review the CSR at:
>> https://bugs.openjdk.java.net/browse/JDK-8257766
>
> Hai-May Chao has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Nonce creation is done in checkOCSP method
Changes requested by mullan (Reviewer).
src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java
line 762:
> 760: } catch (IOException e) {
> 761: throw new CertPathValidatorException("Failed
> to create the default nonce " +
> 762: "in OCSP entensions");
Typo: s/entensions/extensions/
Also, use the `CertPathValidatorException(String, Throwable)` ctor instead and
pass the `IOException` as the 2nd parameter.
src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java
line 755:
> 753: // create the 16-byte nonce by default
> 754: Extension nonceExt = new
> OCSPNonceExtension(DEFAULT_NONCE_BYTES);
> 755: tmpExtensions.add(nonceExt);
I think you should add the OCSPNonce extension to the list of extensions that
the application passed in, as there may be other extensions that have been
specified and should be sent in the OCSP response, ex:
`ocspExtensions.add(new OCSPNonceExtension(DEFAULT_NONCE_BYTES));`
This means you don't need the `tmpExtensions` variable.
src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java
line 779:
> 777: response = OCSP.check(Collections.singletonList(certId),
> 778: responderURI, issuerInfo, responderCert, null,
> 779: rp.ocspNonce ? tmpExtensions : ocspExtensions,
> params.variant());
Here you can just pass in `ocspExtensions` since it will contain the nonce if
the property has been set.
-
PR: https://git.openjdk.java.net/jdk/pull/2039
Re: RFR: 8258833: Cancel multi-part cipher operations in SunPKCS11 after failures [v7]
> When a multi-part cipher operation fails in SunPKCS11 (i.e. because of an > invalid block size), we now cancel the operation before returning the > underlying Session to the Session Manager. This allows to use the returned > Session for a different purpose. Otherwise, an CKR_OPERATION_ACTIVE error > would be raised from the PKCS#11 library. > > The jdk/sun/security/pkcs11/Cipher/CancelMultipart.java regression test is > introduced as part of this PR. > > No regressions found in jdk/sun/security/pkcs11. Martin Balao has updated the pull request incrementally with two additional commits since the last revision: - Align doCancel pattern in 'P11Cipher::implDoFinal(byte[]..' to 'P11Cipher::implDoFinal(ByteBuffer..'. Better documentation in P11Cipher. Copyright date updated. - Copyright dates updated to 2021 on modified files - Changes: - all: https://git.openjdk.java.net/jdk/pull/1901/files - new: https://git.openjdk.java.net/jdk/pull/1901/files/4c892a44..bb52a064 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1901=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1901=05-06 Stats: 40 lines in 6 files changed: 31 ins; 3 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/1901.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1901/head:pull/1901 PR: https://git.openjdk.java.net/jdk/pull/1901
Re: RFR: JDK-8259786: initialize last parameter of getpwuid_r [v4]
On Wed, 20 Jan 2021 12:05:03 GMT, Matthias Baesken wrote: >> We have a couple of calls to getpwuid_r in the codebase, like >> g= getpwuid_r(getuid(), , pwd_buf, sizeof(pwd_buf), ); >> >> Usually we NULL-check pwd after the call because we do not fully trust the >> return code of the function (it is documented in the codebase why we do not >> fully trust the return code) . However we miss to initialize pwd at some >> places before the call, which might we a little problematic and should be >> improved (at other places we already initialize it). >> >> This triggers also Sonar warnings like : >> https://sonarcloud.io/project/issues?id=jdk=AXaE0dsA8L9hkQskGEbA=false=BUG >> >> >> Aside from this issue , should we in other issue , unify the OS versions >> of static char* get_user_name(uid_t uid)in posix code (currently we >> have it for bsd, linux, aix but the functions look very similar ? > > Matthias Baesken has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains five commits: > > - bring initialization to perfMemory_posix > - Merge branch 'master' into JDK-8259786 > - More revert > - revert perfMemory changes > - JDK-8259786 Marked as reviewed by mdoerr (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2098
Re: RFR: 8258833: Cancel multi-part cipher operations in SunPKCS11 after failures [v5]
On Wed, 20 Jan 2021 06:00:41 GMT, Valerie Peng wrote:
>> Martin Balao has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Minor documentation improvement in P11Mac regarding Cancel Operation
>
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java line
> 802:
>
>> 800: System.arraycopy(padBuffer, 0, out, outOfs, k);
>> 801: } else {
>> 802: k = token.p11.C_DecryptFinal(session.id(), 0, out,
>> outOfs,
>
> Comment for line 638 apply here as well.
I agree, but the comment (in my opinion) should be right after the
C_EncryptUpdate/C_DecryptUpdate call when doCancel is set to false.
-
PR: https://git.openjdk.java.net/jdk/pull/1901
Re: RFR: 8258833: Cancel multi-part cipher operations in SunPKCS11 after failures [v6]
On Wed, 20 Jan 2021 05:58:49 GMT, Valerie Peng wrote: >> Martin Balao has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Removing the encryption-update path in CancelMultipart test as it depends >> on a know bug to cause a PKCS#11 error. > > src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java line > 638: > >> 636: (new ShortBufferException().initCause(e)); >> 637: } >> 638: reset(true); > > Per PKCS#11 spec, "A call to C_EncryptUpdate which results in an error other > than CKR_BUFFER_TOO_SMALL terminates the current encryption operation.", so > I'd expect comment here to explain why we are doing reset(true). If not > mentioning the known NSS behavior which triggered this change, at least > comment the bug id so we don't lost track of the reason for the switch. Yes, makes sense. Thanks - PR: https://git.openjdk.java.net/jdk/pull/1901
Re: RFR: 8258833: Cancel multi-part cipher operations in SunPKCS11 after failures [v6]
On Wed, 20 Jan 2021 05:55:26 GMT, Valerie Peng wrote: >> Martin Balao has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Removing the encryption-update path in CancelMultipart test as it depends >> on a know bug to cause a PKCS#11 error. > > src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11AEADCipher.java > line 631: > >> 629: // these cases are not expected here because the output >> length >> 630: // is checked in the OpenJDK side before making the PKCS#11 >> call. >> 631: // Thus, doCancel can safely be 'false'. > > Since the code is following the spec, I am not sure if this comment provides > additional info? Fine to leave it if you prefer to have it. Just a thought. > This goes for the same comments for other classes where we are not changing > the behavior. I wish we could keep the comment and make the previous assumption more explicit, even when someone can read the code above and arrive to the same conclusion. If the code around ever changes, this comment is something we must consider and, eventually, take action. - PR: https://git.openjdk.java.net/jdk/pull/1901
Re: RFR: 8258833: Cancel multi-part cipher operations in SunPKCS11 after failures [v6]
On Wed, 20 Jan 2021 03:16:32 GMT, Valerie Peng wrote:
>> Martin Balao has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Removing the encryption-update path in CancelMultipart test as it depends
>> on a know bug to cause a PKCS#11 error.
>
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11AEADCipher.java
> line 353:
>
>> 351: }
>> 352: } catch (PKCS11Exception e) {
>> 353: if (e.getErrorCode() == CKR_OPERATION_NOT_INITIALIZED) {
>
> nit: update copyright year to 2021. Same goes for other sources.
right, thanks
> test/jdk/sun/security/pkcs11/Cipher/CancelMultipart.java line 2:
>
>> 1: /*
>> 2: * Copyright (c) 2020, Red Hat, Inc.
>
> 2021?
right, thanks
-
PR: https://git.openjdk.java.net/jdk/pull/1901
Re: RFR: JDK-8259786: initialize last parameter of getpwuid_r [v4]
> We have a couple of calls to getpwuid_r in the codebase, like > g= getpwuid_r(getuid(), , pwd_buf, sizeof(pwd_buf), ); > > Usually we NULL-check pwd after the call because we do not fully trust the > return code of the function (it is documented in the codebase why we do not > fully trust the return code) . However we miss to initialize pwd at some > places before the call, which might we a little problematic and should be > improved (at other places we already initialize it). > > This triggers also Sonar warnings like : > https://sonarcloud.io/project/issues?id=jdk=AXaE0dsA8L9hkQskGEbA=false=BUG > > > Aside from this issue , should we in other issue , unify the OS versions of > static char* get_user_name(uid_t uid)in posix code (currently we have it > for bsd, linux, aix but the functions look very similar ? Matthias Baesken has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits: - bring initialization to perfMemory_posix - Merge branch 'master' into JDK-8259786 - More revert - revert perfMemory changes - JDK-8259786 - Changes: https://git.openjdk.java.net/jdk/pull/2098/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2098=03 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/2098.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2098/head:pull/2098 PR: https://git.openjdk.java.net/jdk/pull/2098
Re: RFR: JDK-8259786: initialize last parameter of getpwuid_r [v3]
> We have a couple of calls to getpwuid_r in the codebase, like > g= getpwuid_r(getuid(), , pwd_buf, sizeof(pwd_buf), ); > > Usually we NULL-check pwd after the call because we do not fully trust the > return code of the function (it is documented in the codebase why we do not > fully trust the return code) . However we miss to initialize pwd at some > places before the call, which might we a little problematic and should be > improved (at other places we already initialize it). > > This triggers also Sonar warnings like : > https://sonarcloud.io/project/issues?id=jdk=AXaE0dsA8L9hkQskGEbA=false=BUG > > > Aside from this issue , should we in other issue , unify the OS versions of > static char* get_user_name(uid_t uid)in posix code (currently we have it > for bsd, linux, aix but the functions look very similar ? Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: More revert - Changes: - all: https://git.openjdk.java.net/jdk/pull/2098/files - new: https://git.openjdk.java.net/jdk/pull/2098/files/a98df02b..9fa25435 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2098=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2098=01-02 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2098.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2098/head:pull/2098 PR: https://git.openjdk.java.net/jdk/pull/2098
Re: RFR: JDK-8259786: initialize last parameter of getpwuid_r [v2]
> We have a couple of calls to getpwuid_r in the codebase, like > g= getpwuid_r(getuid(), , pwd_buf, sizeof(pwd_buf), ); > > Usually we NULL-check pwd after the call because we do not fully trust the > return code of the function (it is documented in the codebase why we do not > fully trust the return code) . However we miss to initialize pwd at some > places before the call, which might we a little problematic and should be > improved (at other places we already initialize it). > > This triggers also Sonar warnings like : > https://sonarcloud.io/project/issues?id=jdk=AXaE0dsA8L9hkQskGEbA=false=BUG > > > Aside from this issue , should we in other issue , unify the OS versions of > static char* get_user_name(uid_t uid)in posix code (currently we have it > for bsd, linux, aix but the functions look very similar ? Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: revert perfMemory changes - Changes: - all: https://git.openjdk.java.net/jdk/pull/2098/files - new: https://git.openjdk.java.net/jdk/pull/2098/files/15075d1e..a98df02b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2098=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2098=00-01 Stats: 11 lines in 3 files changed: 4 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/2098.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2098/head:pull/2098 PR: https://git.openjdk.java.net/jdk/pull/2098
