Re: [9] RFR 8165275: Replace the reflective call to the implUpdate method in HandshakeMessage::digestKey

2016-10-10 Thread Xuelei Fan
As this API is only used for legacy SSL 3.0, alternatively we may deprecate and remove the support of MessageDigestSpi2 in the future. I may add a note to MessageDigestSpi2, like: This is for SSL 3.0 implementation only, please don't use it for other purpose. Otherwise, looks fine to me

Re: 8165103: Update to "denyAfter constraint check" exception message

2016-10-10 Thread Anthony Scarpino
Ok.. I have included it into the first if statement.. http://cr.openjdk.java.net/~ascarpino/8165103/webrev.01/ Tony On 10/06/2016 05:06 PM, Valerie Peng wrote: I know it's intentional, however I doubt how much difference this makes. Code duplication is kind of error prone. Just my preference

RFR 8165274: SHA1 certpath constraint check fails with OCSP certificate

2016-10-10 Thread Anthony Scarpino
Hi, I need a review of a fix to JEP 288 were certpath algorithm checking wasn't checking OCSP certs against the jdkCA keyword. http://cr.openjdk.java.net/~ascarpino/8165274/webrev/ thanks Tony

RFR: JDK-8162723 : Array index overflow in Base64 utility class

2016-10-10 Thread Sean Mullan
Please review this change to fix a potential array index overflow in the XML security Base64 encoding class. This is a direct import of the corresponding patch that has already been made to the Apache code. webrev: http://cr.openjdk.java.net/~mullan/webrevs/8162723/webrev.00/ Thanks, Sean

Re: RFR: JDK-8162723 : Array index overflow in Base64 utility class

2016-10-10 Thread Anthony Scarpino
On 10/10/2016 12:29 PM, Sean Mullan wrote: Please review this change to fix a potential array index overflow in the XML security Base64 encoding class. This is a direct import of the corresponding patch that has already been made to the Apache code. webrev: http://cr.openjdk.java.net/~mullan/web

Re: [9] RFR 8165275: Replace the reflective call to the implUpdate method in HandshakeMessage::digestKey

2016-10-10 Thread Valerie Peng
Thanks for the review. I have updated the MessageDigestSpi2 class accordingly: http://cr.openjdk.java.net/~valeriep/8165275/webrev.01/src/java.base/share/classes/sun/security/util/MessageDigestSpi2.java.html Valerie On 10/10/2016 3:15 AM, Xuelei Fan wrote: As this API is only used for legacy

Re: [9] RFR 8165275: Replace the reflective call to the implUpdate method in HandshakeMessage::digestKey

2016-10-10 Thread Mandy Chung
On Oct 4, 2016, at 3:07 PM, Valerie Peng wrote: > > Hi Xuelei, > > Could you please help reviewing this JSSE-related code refactoring? > > Bug: https://bugs.openjdk.java.net/browse/JDK-8165275 > Webrev: http://cr.openjdk.java.net/~valeriep/8165275/webrev.00/ > Looks okay to me. Mandy

RFR 8167459: Add debug output for indicating if a chosen ciphersuite was legacy.

2016-10-10 Thread Bradford Wetmore
Hi Xuelei, We should provide more information about which ciphersuites were actually considered for a handshake and why they were ultimately chosen/not chosen, but for now we have been requested to add a debug message to indicate whether or not the selected ciphersuite was legacy. Examples:

Re: [9] RFR 8165275: Replace the reflective call to the implUpdate method in HandshakeMessage::digestKey

2016-10-10 Thread Xuelei Fan
Thanks for the update. Looks fine to me. Xuelei On 10/11/2016 3:49 AM, Valerie Peng wrote: Thanks for the review. I have updated the MessageDigestSpi2 class accordingly: http://cr.openjdk.java.net/~valeriep/8165275/webrev.01/src/java.base/share/classes/sun/security/util/MessageDigestSpi2.java

Re: RFR 8167459: Add debug output for indicating if a chosen ciphersuite was legacy.

2016-10-10 Thread Xuelei Fan
Nice update, and thanks! Xuelei On 10/11/2016 6:57 AM, Bradford Wetmore wrote: Hi Xuelei, We should provide more information about which ciphersuites were actually considered for a handshake and why they were ultimately chosen/not chosen, but for now we have been requested to add a debug mess

RFR: Release note for 8164705: Remove pathname canonicalization from FilePermission

2016-10-10 Thread Weijun Wang
https://bugs.openjdk.java.net/browse/JDK-8165836 I added more words, please take another look. Thanks Max On 9/13/2016 2:13, Brian Burkhalter wrote: Only picky comments: not sure but maybe change: 1) vice versa -> and vice versa 2) When it’s set to true -> When true 3) just like before -> as

Code Review Request, JDK-8167472, Chrome interop regression with JDK-8148516

2016-10-10 Thread Xuelei Fan
Hi, Please review this update: http://cr.openjdk.java.net/~xuelei/8167472/webrev.00/ The root cause is that in the update of JDK-8148516, we removed some EC curves from the default supported EC curves list. However, while select the preferable curves requested by the client side, the serv

Re: Code Review Request, JDK-8167472, Chrome interop regression with JDK-8148516

2016-10-10 Thread Weijun Wang
Code change looks fine. Or you can write if (isSupported && permits) Thanks Max On 10/11/2016 13:06, Xuelei Fan wrote: Hi, Please review this update: http://cr.openjdk.java.net/~xuelei/8167472/webrev.00/ The root cause is that in the update of JDK-8148516, we removed some EC curves f

Re: Code Review Request, JDK-8167472, Chrome interop regression with JDK-8148516

2016-10-10 Thread Xuelei Fan
On 10/11/2016 1:44 PM, Weijun Wang wrote: Code change looks fine. Or you can write if (isSupported && permits) Nice coding. Thanks! Xuelei Thanks Max On 10/11/2016 13:06, Xuelei Fan wrote: Hi, Please review this update: http://cr.openjdk.java.net/~xuelei/8167472/webrev.00/ Th