Possible bug in SSLEngine / SSLSession implementation

2018-12-12 Thread Norman Maurer
Hi all, While working on some unit tests in netty I noticed that there may be a bug in the JDK implementation of SSLEngine / SSLSession. If its not a but it is at least surprising I would say. So it seems like before the handshake all values that are set on the SSLSession via putValue are sha

Re: RFR: 8211752: JNU_ThrowIOExceptionWithLastErrorAndPath - enhance some IOExceptions with path causing the issue

2018-12-12 Thread Alan Bateman
On 11/12/2018 20:00, Sean Mullan wrote: These exceptions are generated from a very low level part of the native JDK Windows or Unix FileSystem implementations. That is a concern. The previous usages of this property were more focused and confined to smaller parts of the code resulting in fewe

Re: Possible bug in SSLEngine / SSLSession implementation

2018-12-12 Thread Jamil Nimeh
Hi Norman, the new handshaker does return a new SSLSession object. Part of JDK-8212885 fixes the lack of propagation of session values across session objects, though that fix was largely in the context of TLS 1.3.  There is a backport set for it, but it is not yet complete as far as I'm aware. 

Re: RFR 8076190: Customizing the generation of a PKCS12 keystore

2018-12-12 Thread Weijun Wang
The latest webrev is at https://cr.openjdk.java.net/~weijun/8076190/webrev.07 The major change since webrev.05 is that the properties are no longer read into static fields, they are read on-demand. This means they will never be read if store() is not called. I've also added more test cases.

Re: Possible bug in SSLEngine / SSLSession implementation

2018-12-12 Thread Norman Maurer
Hi Jamil, This was just noticed during a test which uses TLS1.2. > On 12. Dec 2018, at 15:35, Jamil Nimeh wrote: > > Hi Norman, the new handshaker does return a new SSLSession object. Part of > JDK-8212885 fixes the lack of propagation of session values across session > objects, though that

Re: RFR 8076190: Customizing the generation of a PKCS12 keystore

2018-12-12 Thread Sean Mullan
Looks good. --Sean On 12/12/18 9:40 AM, Weijun Wang wrote: The latest webrev is at https://cr.openjdk.java.net/~weijun/8076190/webrev.07 The major change since webrev.05 is that the properties are no longer read into static fields, they are read on-demand. This means they will never be r

Re: RFR 8076190: Customizing the generation of a PKCS12 keystore

2018-12-12 Thread Weijun Wang
Thanks. Will you please also take a look at the release note at https://bugs.openjdk.java.net/browse/JDK-8215293? --Max > On Dec 12, 2018, at 11:01 PM, Sean Mullan wrote: > > Looks good. > > --Sean > > On 12/12/18 9:40 AM, Weijun Wang wrote: >> The latest webrev is at >>https://cr.openjd

RFR: 8215281: Use String.isEmpty() where applicable in java.base

2018-12-12 Thread Claes Redestad
Hi, please review this patch to use String.isEmpty when applicable: Webrev: http://cr.openjdk.java.net/~redestad/8215281/jdk.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8215281 Why? - It reads better :-) - Better startup/warmup due fewer method invocations - Better peak performance: Strin

Re: RFR: 8215281: Use String.isEmpty() where applicable in java.base

2018-12-12 Thread Daniel Fuchs
Hi Claes, It might read even better if things like: +resultString = !specarg.isEmpty() ? specarg.intern(): opt; were changed into: +resultString = specarg.isEmpty() ? opt : specarg.intern(); best regards, -- daniel On 12/12/2018 16:53, Claes Redestad wrote: Hi, please revi

Re: RFR: 8215281: Use String.isEmpty() where applicable in java.base

2018-12-12 Thread Alan Bateman
On 12/12/2018 16:53, Claes Redestad wrote: Hi, please review this patch to use String.isEmpty when applicable: Webrev: http://cr.openjdk.java.net/~redestad/8215281/jdk.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8215281 Why? - It reads better :-) - Better startup/warmup due fewer method

Re: RFR: 8215281: Use String.isEmpty() where applicable in java.base

2018-12-12 Thread Claes Redestad
On 2018-12-12 17:56, Alan Bateman wrote: In Checks.java, the parameter change from CharSequence to String means that "cs" needs to be renamed. Changed to 'str' /Claes

Re: RFR: 8215281: Use String.isEmpty() where applicable in java.base

2018-12-12 Thread Claes Redestad
On 2018-12-12 17:54, Daniel Fuchs wrote: Hi Claes, It might read even better if things like: +    resultString = !specarg.isEmpty() ? specarg.intern(): opt; were changed into: +    resultString = specarg.isEmpty() ? opt : specarg.intern(); best regards, I only found this pattern

Re: RFR[12] JDK-8214520: [TEST_BUG] sun/security/mscapi/nonUniqueAliases/NonUniqueAliases.java failed with incorrect jtreg tags order

2018-12-12 Thread Xue-Lei Fan
looks fine to me. thanks, xuelei On 12/11/2018 11:35 PM, sha.ji...@oracle.com wrote: Hi, When run this test on Windows, jtreg complains that '@library' must appear before first action tag. So, just adjust the tag positions. Issue: https://bugs.openjdk.java.net/browse/JDK-8214520 diff -r a61

Re: RFR JDK-7092821 "java.security.Provider.getService() is synchronized and became scalability bottleneck"

2018-12-12 Thread Valerie Peng
If anyone is still reviewing this and need more time, please let me know soon. Otherwise, I will proceed with integration this afternoon. Thanks, Valerie On 12/6/2018 7:27 PM, Valerie Peng wrote: Hi Sean, Thanks for your review, I have removed the stale comment from the VerificationProvider

Re: RFR 6722928: Support SSPI as a native GSS-API provider

2018-12-12 Thread Valerie Peng
Hi Max, - the DER related code is very hard to read... Would be nice to use constants/enum for commonly used tag or use some method to construct them. - line 449, I think you mean to use "c" instead of "cred_handle" - gss_unwrap: add "const" to the 2nd and 3rd arguments? Isn't variable nam

Re: RFR 8213010: [cng] Supporting keys created with certmgr.exe

2018-12-12 Thread Weijun Wang
Hi Valerie, The updated webrev is at https://cr.openjdk.java.net/~weijun/8213010/webrev.01/ I haven't merged importPublicKey and importECPublicKey because the content is so different and we expect someday to remove the CAPI method at all. The interdiff.patch has some problem since I haven't

Re: RFR 8213010: [cng] Supporting keys created with certmgr.exe

2018-12-12 Thread Valerie Peng
The updated webrev looks fine. Would prefer to have importPublicKey and importECPublicKey methods merged as other methods do. But not a deal breaker as it does not affect functionality. Also, would be nice to use a http URL for the EC public key blob format. Thanks, Valerie On 12/12/2018

Re: RFR 8213010: [cng] Supporting keys created with certmgr.exe

2018-12-12 Thread Weijun Wang
> On Dec 13, 2018, at 10:07 AM, Valerie Peng wrote: > > > The updated webrev looks fine. > > Would prefer to have importPublicKey and importECPublicKey methods merged as > other methods do. But not a deal breaker as it does not affect functionality. > > Also, would be nice to use a http UR

Re: RFR 8214568: Use {@systemProperty} for definitions of system properties

2018-12-12 Thread Weijun Wang
Ping again. This is an RFE so needs to be fixed today unless we request for an approval. Thanks, Max > On Dec 7, 2018, at 11:21 PM, Weijun Wang wrote: > > Please take a review at > > https://cr.openjdk.java.net/~weijun/8214568/webrev.00 > > Thanks > Max

RFR[12] JDK-8214937: sun/security/tools/jarsigner/warnings/NoTimestampTest.java failed due to unexpected expiration date

2018-12-12 Thread sha . jiang
Hi, This test should determine the cert expiration date from the cert itself, but not try to calculate that date. Issue: https://bugs.openjdk.java.net/browse/JDK-8214937 Webrev: http://cr.openjdk.java.net/~jjiang/8214937/webrev.00/ Best regards, John Jiang

Re: RFR 8214568: Use {@systemProperty} for definitions of system properties

2018-12-12 Thread Xuelei Fan
Looks fine to me. Xuelei > On Dec 12, 2018, at 6:25 PM, Weijun Wang wrote: > > Ping again. > > This is an RFE so needs to be fixed today unless we request for an approval. > > Thanks, > Max > >> On Dec 7, 2018, at 11:21 PM, Weijun Wang wrote: >> >> Please take a review at >> >> https://

Re: RFR[12] JDK-8214937: sun/security/tools/jarsigner/warnings/NoTimestampTest.java failed due to unexpected expiration date

2018-12-12 Thread Xuelei Fan
Nice catch and looks good to me. Xuelei > On Dec 12, 2018, at 7:14 PM, sha.ji...@oracle.com wrote: > > Hi, > This test should determine the cert expiration date from the cert itself, but > not try to calculate that date. > > Issue: https://bugs.openjdk.java.net/browse/JDK-8214937 > Webrev: htt

8215534: [testbug] some jfr test don't check @requires vm.hasJFR

2018-12-12 Thread Lindenmaier, Goetz
Hi, These tests lack @requires vm.hasJFR, thus they are failing on AIX. http://cr.openjdk.java.net/~goetz/wr18/8215334-JFR_requires/01/ Please review. I will push this to jdk12 as it is a testbug if I miss the RDP deadline. Best regards, Goetz.