Re: Code Review Request: 8028562

Hi Xuelei, I have updated, please review:http://cr.openjdk.java.net/~ewang/kevin/JDK-8028562/webrev.00/ Thanks Kevin On 2013/12/4 14:50, Xuelei Fan wrote: On 12/4/2013 2:36 PM, zaiyao liu wrote: Hi Xuelei, Thanks for you s

Re: Code Review Request: 8028562

On 12/4/2013 2:36 PM, zaiyao liu wrote: > Hi Xuelei, > > Thanks for you suggestion. please review again: > http://cr.openjdk.java.net/~ewang/kevin/JDK-8028562/webrev.00/ > Need a white space: - 224 //will try to read one more roud when read error + 224 // will try to read one more roud when re

Re: Code Review Request: 8028562

Hi Xuelei, Thanks for you suggestion. please review again: http://cr.openjdk.java.net/~ewang/kevin/JDK-8028562/webrev.00/ Kevin On 2013/12/4 12:06, Xuelei Fan wrote: On 12/4/2013 11:33 AM, zaiyao liu wrote: Hi Xuelei, Can yo

Re: Code Review Request: 8028562

On 12/4/2013 11:33 AM, zaiyao liu wrote: > Hi Xuelei, > > Can you help to review again. > http://cr.openjdk.java.net/~ewang/kevin/JDK-8028562/webrev.00/ > Thanks for the update. Please pay attentions to the code conversions. 300 if (serverIn.remaining() != clientMsg.length) { 301 if(retry)

Re: Code Review Request: 8028562

Hi Xuelei, Can you help to review again. http://cr.openjdk.java.net/~ewang/kevin/JDK-8028562/webrev.00/ Thanks Kevin On 2013/12/3 19:50, Xuelei Fan wrote: On 12/3/2013 6:59 PM, zaiyao liu wrote: Hi Xuelei, I can't reproduce

Code Review request 8027218 EC Test

Hi, I need a review of the below test bug fix: 8027218: TEST_BUG: sun/security/pkcs11/ec tests fail because of ever-changing key size restrictions http://cr.openjdk.java.net/~ascarpino/8027218/webrev/ Thanks Tony

Re: Code Review Request: 8028562

On 12/03/2013 05:59 AM, zaiyao liu wrote: Hi Xuelei, I can't reproduce this issue after run 900 times at windows and linux platform, for this fix just run one more round after get exception. please review: This is an OpenJDK mailing list. Please repost the webrev on cr.openjdk.java.net. -

Re: Code Review Request: 8028562

On 12/3/2013 6:59 PM, zaiyao liu wrote: > Hi Xuelei, > > I can't reproduce this issue after run 900 times at windows and linux > platform, It should be pretty hard to reproduce the issue in normal TCP/IP environment. > for this fix just run one more round after get exception. > > please review:

Code Review Request: 8028562

Hi Xuelei, I can't reproduce this issue after run 900 times at windows and linux platform, for this fix just run one more round after get exception. please review: http://sqeweb.us.oracle.com/net/sqenfs-1/export1/comp/jsn/users/kevin1/webrev/8028562/webrev/ Thanks Kevin

Re: Code Review Request: 8025763

The missing @since for overridden methods and missing @throws for unchecked exceptions is a known javadoc behaviour. Apparently there's some disagreement as to whether it is a bug but I encourage you to add the tags. Mike On Nov 27 2013, at 12:55 , Bradford Wetmore wrote: > Sean wrote: > >

Re: Code Review Request: 8021418

On 11/28/2013 5:38 AM, Rajan Halade wrote: > On 11/27/2013 13:07, Bradford Wetmore wrote: >> Where did this workaround suggestion come from? > Refer to JDK-6978415, it has possible workaround listed. The spec of ServerSocket.setReuseAddress() says, "When a ServerSocket is created the initial settin

Re: Code Review Request: 8021418

On 11/27/2013 13:07, Bradford Wetmore wrote: Where did this workaround suggestion come from? Refer to JDK-6978415, it has possible workaround listed. Please link these two issues (JDK-6978415) together. the two bugs are linked now. Have you talked to the network team about this? No, since

Re: Code Review Request: 8021418

Where did this workaround suggestion come from? Please link these two issues (JDK-6978415) together. Have you talked to the network team about this? brad On 11/26/2013 6:09 PM, Xuelei Fan wrote: This change looks fine. JSSE regression tests use a lot of code as "new ServerSocket(0)", we m

Re: Code Review Request: 8025763

Sean wrote: > That kind of seems like a javadoc bug to me. Shouldn't it add the > @since tag as part of inheriting the javadoc? On the chance that this is a real bug, I filed this yesterday: https://bugs.openjdk.java.net/browse/JDK-8029241 @throws/@since are both missing (others?), it seem

Re: Code Review Request: 8025763

> On Nov 27, 2013, at 10:12 AM, Sean Mullan wrote: > >> On 11/26/2013 08:20 PM, Bradford Wetmore wrote: >> Tony, >> >> I note the @since's are missing for the new methods, both in the >> generated output in the overridden methods (i.e. no javadoc), and the >> methods in which you've changed the

Re: Code Review Request: 8025763

On 11/26/2013 08:20 PM, Bradford Wetmore wrote: Tony, I note the @since's are missing for the new methods, both in the generated output in the overridden methods (i.e. no javadoc), and the methods in which you've changed the behavior (i.e. new javadoc). I'm not sure what you can do about the pr

Re: Code Review Request: 8021418

On 11/26/2013 18:09, Xuelei Fan wrote: This change looks fine. thanks! JSSE regression tests use a lot of code as "new ServerSocket(0)", we may want a cleanup for test stabilization. I will look at regression tests and file separate bug for it. - Rajan Xuelei On 11/27/2013 9:13 AM, Rajan

Re: Code Review Request: 8021418

This change looks fine. JSSE regression tests use a lot of code as "new ServerSocket(0)", we may want a cleanup for test stabilization. Xuelei On 11/27/2013 9:13 AM, Rajan Halade wrote: > May I request you to review this simple change - > > http://cr.openjdk.java.net/~juh/rajan/8021418/ >

Re: Code Review Request: 8025763

Tony, I note the @since's are missing for the new methods, both in the generated output in the overridden methods (i.e. no javadoc), and the methods in which you've changed the behavior (i.e. new javadoc). I'm not sure what you can do about the previous behavior (cc'ing Mike/Sowmya, maybe th

Code Review Request: 8021418

May I request you to review this simple change - http://cr.openjdk.java.net/~juh/rajan/8021418/ The test is modified to set SO_REUSEADDR on ServerSocket to false for stabilization. Thanks, Rajan

Re: Code Review Request for 7200306: SunPKCS11 provider delays the check of DSA key size for SHA1withDSA to sign() instead of init()

Thanks for the prompt review~ Valerie On 11/22/13 12:20, Sean Mullan wrote: On 11/22/2013 02:54 PM, Valerie (Yu-Ching) Peng wrote: Even if Solaris PKCS11 provider starts to support 2048-bit DSA keys, its SHA1withDSA signature impl should still only accept up-to-1024-bit DSA keys. The longer DS

Re: Code Review Request for 7200306: SunPKCS11 provider delays the check of DSA key size for SHA1withDSA to sign() instead of init()

On 11/22/2013 02:54 PM, Valerie (Yu-Ching) Peng wrote: Even if Solaris PKCS11 provider starts to support 2048-bit DSA keys, its SHA1withDSA signature impl should still only accept up-to-1024-bit DSA keys. The longer DSA keys need newer signature impls using SHA2-family digests. So, the regressio

Re: Code Review Request for 7200306: SunPKCS11 provider delays the check of DSA key size for SHA1withDSA to sign() instead of init()

Even if Solaris PKCS11 provider starts to support 2048-bit DSA keys, its SHA1withDSA signature impl should still only accept up-to-1024-bit DSA keys. The longer DSA keys need newer signature impls using SHA2-family digests. So, the regression test should still be valid. Thanks, Valerie On 11

Re: Code Review Request for 7200306: SunPKCS11 provider delays the check of DSA key size for SHA1withDSA to sign() instead of init()

The fix looks good. One comment on the test - it looks like the test would start failing if Solaris PKCS11 started to support 2048 bit DSA keys. Is there a way to workaround that by checking the max key length supported by the library? --Sean On 11/19/2013 08:37 PM, Valerie (Yu-Ching) Peng wr

Code Review Request for 7200306: SunPKCS11 provider delays the check of DSA key size for SHA1withDSA to sign() instead of init()

Can someone please help review my fixes for 7200306: SunPKCS11 provider delays the check of DSA key size for SHA1withDSA to sign() instead of init()? Native PKCS11 libraries don't seem to check the key during the initialization calls (triggered by initSign()/initVerify()). Rather, it errors

Re: Code Review Request for 8026943: SQE test jce/Global/Cipher/SameBuffer failed

This looks good to me. --Sean On 11/13/2013 04:43 PM, Valerie (Yu-Ching) Peng wrote: Can someone help review my fixes for 8026943 "SQE test jce/Global/Cipher/SameBuffer failed"? According to Cipher javadoc, both its update(...) and doFinal(...) methods should be copy-safe, meaning the |input|

Re: Code Review Request: 8014266

Looks fine to me. Xuelei On 11/14/2013 3:28 PM, Rajan Halade wrote: > May I request you to review this simple change. > http://cr.openjdk.java.net/~xuelei/8014266/webrev/ > > This is to update the test comment to assist in timeout analysis. The > test may timeout on heavy loaded system as it inv

Code Review Request: 8014266

May I request you to review this simple change. http://cr.openjdk.java.net/~xuelei/8014266/webrev/ This is to update the test comment to assist in timeout analysis. The test may timeout on heavy loaded system as it involves multiple tls transactions. -- Rajan Halade, C

Code Review Request for 8026943: SQE test jce/Global/Cipher/SameBuffer failed

Can someone help review my fixes for 8026943 "SQE test jce/Global/Cipher/SameBuffer failed"? According to Cipher javadoc, both its update(...) and doFinal(...) methods should be copy-safe, meaning the |input| and |output| buffers can reference the same byte array and no unprocessed input dat

Re: Code review request, 802314, Test DisabledShortRSAKeys.java intermittent failed

Looks ok to me. --Sean On 11/11/2013 08:44 AM, Xuelei Fan wrote: Please review a test stabilization fix. webrev: http://cr.openjdk.java.net/~xuelei/8023147/webrev.00/ Thanks, Xuelei

Code review request, 802314, Test DisabledShortRSAKeys.java intermittent failed

Please review a test stabilization fix. webrev: http://cr.openjdk.java.net/~xuelei/8023147/webrev.00/ Thanks, Xuelei

Re: Code Review Request: 8025763

On 10/18/2013 10:52 PM, Anthony Scarpino wrote: I've updated the webrev http://cr.openjdk.java.net/~ascarpino/8025763/webrev.01/ Update looks good. --Sean

Re: Code Review Request: 8025763

On 10/18/2013 12:44 PM, Sean Mullan wrote: Mostly looks good, just a few comments: - for completeness, please add @Override tags to all existing methods that are overridden - you need to also override the new forEach and getOrDefault methods to first check if the provider is initialized. See, f

Re: Code Review Request: 8025763

Mostly looks good, just a few comments: - for completeness, please add @Override tags to all existing methods that are overridden - you need to also override the new forEach and getOrDefault methods to first check if the provider is initialized. See, for example, the get method. - nit, say

Re: Code review request: 7025699: Policy Tool is not accessible by keyboard

+1 -- best regards, Anthony On 10/17/2013 09:11 PM, alexander potochkin wrote: Hello Leif Looks good to me I had a look at the PolicyTool code and indeed it is better to leave the IO processing as is at this moment. Thanks alexp Hi All, A new webrev is available at: http://cr.openjdk.

Code Review Request: 8025763

Hi, I need a code review for changes regarding 8025763 Provider does not override new Hashtable methods http://cr.openjdk.java.net/~ascarpino/8025763/ thanks Tony

Re: Code review request: 7025699: Policy Tool is not accessible by keyboard

Hello Leif Looks good to me I had a look at the PolicyTool code and indeed it is better to leave the IO processing as is at this moment. Thanks alexp Hi All, A new webrev is available at: http://cr.openjdk.java.net/~weijun/7025699/webrev.01/ The following new changes were made: 1. Adde

Re: Code review request: 7025699: Policy Tool is not accessible by keyboard

Hi All, A new webrev is available at: http://cr.openjdk.java.net/~weijun/7025699/webrev.01/ The following new changes were made: 1. Added call to SwingUtilities.invokeLater() in main() to run the GUI initialization on the Event Dispatch Thread. 2. Minor corrections to initial size and place

Re: Code review request: 7025699: Policy Tool is not accessible by keyboard

Hi Max, I don't have expertise in this code so I haven't reviewed the fix thoroughly. I'd like to point out one thing though: unlike AWT, Swing is a single-threaded GUI toolkit. While in AWT you can create components/windows and call APIs on any thread, in Swing everything GUI-related must be

Re: Code review request: 7025699: Policy Tool is not accessible by keyboard

Hello Well, performing I/O or other blocking operations on EDT can only freeze the app's GUI for the period of blocking. If developers/users are OK with that, this is fine with me too. I don't think an I/O blocking operation on EDT is acceptable. It can freeze the whole application and giv

Re: Code review request: 7025699: Policy Tool is not accessible by keyboard

Well, performing I/O or other blocking operations on EDT can only freeze the app's GUI for the period of blocking. If developers/users are OK with that, this is fine with me too. However, you should still call all Swing APIs (including creating your components/windows) on the EDT. And I don't

Re: Code review request: 7025699: Policy Tool is not accessible by keyboard

Thanks for the good feedback. We will make sure to move to move the GUI instantiation to the EDT and the file I/O to a worker thread. All other operations are event driven and therefore occur directly on the EDT. Leif On 2013-10-15 09:56, alexander potochkin wrote: Hello Well, performing I/

Re: Code review request: 7025699: Policy Tool is not accessible by keyboard

Policy Tool is a GUI editor for the plain text policy file. The only I/O is loading the policy file from disk and it should be quite small, so I think there won't be a problem here. Thanks Max On 10/15/13 8:08 PM, Anthony Petrov wrote: Hi Max, I don't have expertise in this code so I haven't

Re: Code review request 8026119 Regression test DHEKeySizing.java failing intermittently

9 is really huge. I will use 6 instead. Thanks for the code review. Xuelei On 10/14/2013 11:45 AM, Weijun Wang wrote: > On 10/14/13 11:19 AM, Xuelei Fan wrote: >> CC security-dev. >> >> On 10/14/2013 11:04 AM, Xuelei Fan wrote: >>> Normally, there are only leading zero of DH keys. >> Oops, typo

Re: Code review request 8026119 Regression test DHEKeySizing.java failing intermittently

On 10/14/13 11:19 AM, Xuelei Fan wrote: CC security-dev. On 10/14/2013 11:04 AM, Xuelei Fan wrote: Normally, there are only leading zero of DH keys. Oops, typo here: ... there are only one leading zero of DH keys. Xuelei By the fix, I suppose it should rally happen for 3 bytes leading zer

Re: Code review request 8026119 Regression test DHEKeySizing.java failing intermittently

CC security-dev. On 10/14/2013 11:04 AM, Xuelei Fan wrote: > Normally, there are only leading zero of DH keys. Oops, typo here: ... there are only one leading zero of DH keys. Xuelei > By the fix, I suppose > it should rally happen for 3 bytes leading zeros. The worst cases, > dh_p, dh_g and dh

Code review request: 7025699: Policy Tool is not accessible by keyboard

Hi All Please review the fix at http://cr.openjdk.java.net/~weijun/7025699/webrev.00/ The fix includes porting PolicyTool from AWT to Swing, defining mnemonics for menu items and buttons, and adding keyboard shortcuts for the File -> New/Open/Save items. Several tests are updated also. T

Re: Code review request: 8026235: keytool NSS test should use 64 bit lib on Solaris

Your changes look fine. Thanks. On 10/10/2013 01:56, Weijun Wang wrote: Hi Vinnie Please take a review at http://cr.openjdk.java.net/~weijun/8026235/webrev.00/ Thanks Max

Code review request: 8026235: keytool NSS test should use 64 bit lib on Solaris

Hi Vinnie Please take a review at http://cr.openjdk.java.net/~weijun/8026235/webrev.00/ Thanks Max

Re: Code Review Request for 8025967 "addition of -Werror broke the old build"

Thanks Vinnie for the review~ Forwarding this request to core-libs-dev per Sean's suggestion. Changes are for getting rid of compiler warnings in order for JCE provider build (still using the old build process) to complete successfully - either add the annotation to suppress rawtype warnings or

Re: Code Review Request for 8025967 "addition of -Werror broke the old build"

I would also send this to core-libs-dev for someone to look over the non-security component changes. --Sean On 10/04/2013 06:24 PM, Valerie (Yu-Ching) Peng wrote: Well, can someone please review the following trivial fix today or early Monday? 8025967: addition of -Werror broke the old build

Re: Code Review Request for 8025967 "addition of -Werror broke the old build"

Your changes look fine. On 04/10/2013 23:24, Valerie (Yu-Ching) Peng wrote: Well, can someone please review the following trivial fix today or early Monday? 8025967: addition of -Werror broke the old build JCE is still using the legacy build and as a result, I have to fix build warnings in oth

Code Review Request for 8025967 "addition of -Werror broke the old build"

Well, can someone please review the following trivial fix today or early Monday? 8025967: addition of -Werror broke the old build JCE is still using the legacy build and as a result, I have to fix build warnings in other components in order for the whole build to succeed. The changes are all

Re: Code review request: 8025123: SNI support in Kerberos cipher suites

Looks fine to me. Xuelei On 10/1/2013 5:45 PM, Artem wrote: > Hi Max, > > I updated the fix according to your comments: > > http://cr.openjdk.java.net/~kshefov/8025123/webrev.02/ > > > Artem > > On 09/30/2013 06:54 PM, Weijun Wang wro

Re: Code review request: 8025123: SNI support in Kerberos cipher suites

Hi Max, I updated the fix according to your comments: http://cr.openjdk.java.net/~kshefov/8025123/webrev.02/ Artem On 09/30/2013 06:54 PM, Weijun Wang wrote: Now that we only check "localhost" and "localhost.localdomain", the follow

Re: Code Review Request for 8014374: Cannot initialize "AES/GCM/NoPadding" on wrap/unseal on solaris with OracleUcrypto

The new tests in the closed repo will also be addressed under 8014374. So, you won't see them until this fix is integrated. Thanks, Valerie On 09/27/13 17:34, Bradford Wetmore wrote: On 9/27/2013 4:49 PM, Valerie (Yu-Ching) Peng wrote: Xuelei, Since the source for OracleUcrypto provider isn

Re: Code review request: 8025123: SNI support in Kerberos cipher suites

Looks fine to me. BTW, you may also want to consider Weijun's comments. Thanks, Xuelei On 9/30/2013 10:32 PM, Artem wrote: > Updated webrev: http://cr.openjdk.java.net/~kshefov/8025123/webrev.01/ > > > - broke lines exceed 80 characters

Re: Code review request: 8025123: SNI support in Kerberos cipher suites

Now that we only check "localhost" and "localhost.localdomain", the following hack in the SSL.java test is not necessary any more: 92 // Run this after KDC, so our own DNS service can be started 93 try { 94 server = InetAddress.getLocalHost().getHostName().toLo

Re: Code review request: 8025123: SNI support in Kerberos cipher suites

Updated webrev: http://cr.openjdk.java.net/~kshefov/8025123/webrev.01/ - broke lines exceed 80 characters - the code updated accrding to http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html#449 - upda

Re: Code review request: 8025123: SNI support in Kerberos cipher suites

It's a good catch and wonderful fix. Thanks, Artem! The fix looks fine to me, except three very minor coding style comments. 1. we used to have only 80 characters in each line. I think there might some lines exceed 80 characters, would you mind break these lines to keep the style consistent?

Code review request: 8025123: SNI support in Kerberos cipher suites

Hello, please review this fix for 8: http://cr.openjdk.java.net/~kshefov/8025123/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8025123 SNI APIs were introduced in JDK 8, but TLS Kerberos client implementation does not take into

Re: Code Review Request for 8014374: Cannot initialize "AES/GCM/NoPadding" on wrap/unseal on solaris with OracleUcrypto

Looks fine to me, too. Xuelei On 9/28/2013 8:34 AM, Bradford Wetmore wrote: > > > On 9/27/2013 4:49 PM, Valerie (Yu-Ching) Peng wrote: >> Xuelei, >> >> Since the source for OracleUcrypto provider isn't included in OpenJDK, >> it probably makes more sense to have its regression tests off OpenJDK

Re: Code Review Request for 8014374: Cannot initialize "AES/GCM/NoPadding" on wrap/unseal on solaris with OracleUcrypto

On 9/27/2013 4:49 PM, Valerie (Yu-Ching) Peng wrote: Xuelei, Since the source for OracleUcrypto provider isn't included in OpenJDK, it probably makes more sense to have its regression tests off OpenJDK as well. Please find the changes for the test relocation under 8014374: Webrev: http://cr.o

Code Review Request for 8014374: Cannot initialize "AES/GCM/NoPadding" on wrap/unseal on solaris with OracleUcrypto

Xuelei, Since the source for OracleUcrypto provider isn't included in OpenJDK, it probably makes more sense to have its regression tests off OpenJDK as well. Please find the changes for the test relocation under 8014374: Webrev: http://cr.openjdk.java.net/~valeriep/8014374/webrev.00/ Thanks,

Re: Code review request: 8024861: Incomplete token triggers GSS-API NullPointerException

On 09/26/2013 05:28 AM, Weijun Wang wrote: Hi All Please take a review at http://cr.openjdk.java.net/~weijun/8024861/webrev.00/ When the first NegTokenInit does not include the mechToken, Java throws an NPE. This code change checks it and throw a GSSException instead. Precisely, the mechTo

Code review request: 8024861: Incomplete token triggers GSS-API NullPointerException

Hi All Please take a review at http://cr.openjdk.java.net/~weijun/8024861/webrev.00/ When the first NegTokenInit does not include the mechToken, Java throws an NPE. This code change checks it and throw a GSSException instead. Precisely, the mechToken can be missing and the initiator will s

Re: Code review request: 8012615: backport of capaths fix to 7u60

Fixes look fine. Thanks, Valerie On 09/18/13 06:59, Weijun Wang wrote: Hi Valerie I've backported 8012615 to 7u60. Please take a review http://cr.openjdk.java.net/~weijun/8012615/7u-dev/webrev.00/ Changes include: 1. Direct copy of parseCapaths and parseHierarchy from jdk8, except for

Re: Code review request, JDK-6956398, make ephemeral DH key match the length of the certificate key

Thanks for the code review. ;-) There is a overloaded comment because this update is really complicated because of compatibilities although the update is simple. I hope the coder reader can understand the logic a little easier. Xuelei On 9/25/2013 4:44 PM, Weijun Wang wrote: > Please also updat

Re: Code review request, JDK-6956398, make ephemeral DH key match the length of the certificate key

Please also update the CCC. On 9/24/13 6:42 PM, Xuelei Fan wrote: new webrev: http://cr.openjdk.java.net/~xuelei/6956398/webrev.01/ ServerHandshaker.java: 1298: Should be "system property not defined". 1311: customize 1319: Read below Overall, I think the comment is too long. :) ... Why

Re: Code review request, JDK-6956398, make ephemeral DH key match the length of the certificate key

new webrev: http://cr.openjdk.java.net/~xuelei/6956398/webrev.01/ On 9/24/2013 5:08 PM, Weijun Wang wrote: > Hi Xuelei > > Comments below. > > On 9/22/13 11:15 AM, Xuelei Fan wrote: >> Hi Weijun, >> >> Are you available to review this update? >> >> webrev: http://cr.openjdk.java.net/~xuelei/6956

Re: Code review request, JDK-6956398, make ephemeral DH key match the length of the certificate key

Hi Xuelei Comments below. On 9/22/13 11:15 AM, Xuelei Fan wrote: Hi Weijun, Are you available to review this update? webrev: http://cr.openjdk.java.net/~xuelei/6956398/webrev.00/ This is an enhancement to support stronger ephemeral DH keys during TLS handshaking. A new system property is de

Re: Code review request, JDK-6956398, make ephemeral DH key match the length of the certificate key

Ping ... On 9/22/2013 11:15 AM, Xuelei Fan wrote: > Hi Weijun, > > Are you available to review this update? > > webrev: http://cr.openjdk.java.net/~xuelei/6956398/webrev.00/ > > This is an enhancement to support stronger ephemeral DH keys during TLS > handshaking. A new system property is defi

Code review request, JDK-6956398, make ephemeral DH key match the length of the certificate key

Hi Weijun, Are you available to review this update? webrev: http://cr.openjdk.java.net/~xuelei/6956398/webrev.00/ This is an enhancement to support stronger ephemeral DH keys during TLS handshaking. A new system property is defined "jdk.tls.ephemeralDHKeySize". By default, the value of this sy

Code review request: 8012615: backport of capaths fix to 7u60

Hi Valerie I've backported 8012615 to 7u60. Please take a review http://cr.openjdk.java.net/~weijun/8012615/7u-dev/webrev.00/ Changes include: 1. Direct copy of parseCapaths and parseHierarchy from jdk8, except for a) use old Config methods b) parseCapaths returns null instead of thro

Re: Code Review Request: 7122707 update Security Providers to JDK8

On 09/16/2013 09:27 PM, Xuelei Fan wrote: src/macosx/classes/apple/security/AppleProvider.java I'm not sure apple provider is using the JDK version as the provider version. -super("Apple", 1.1, info); +super("Apple", 1.8d, info)

Re: Code Review Request: 7122707 update Security Providers to JDK8

On 9/17/13 9:27 AM, Xuelei Fan wrote: src/share/classes/sun/security/jgss/wrapper/SunNativeProvider.java == As above, I'm not sure native JGSS provider is using JDK version as the provider version. -super(NAME, 1.0, INFO);

Re: Code Review Request: 7122707 update Security Providers to JDK8

On 9/17/2013 9:44 AM, Anthony Scarpino wrote: > On 09/16/2013 06:27 PM, Xuelei Fan wrote: >> src/macosx/classes/apple/security/AppleProvider.java >> >> I'm not sure apple provider is using the JDK version as the provider >> version. >> -s

Re: Code Review Request: 7122707 update Security Providers to JDK8

On 09/16/2013 06:27 PM, Xuelei Fan wrote: src/macosx/classes/apple/security/AppleProvider.java I'm not sure apple provider is using the JDK version as the provider version. -super("Apple", 1.1, info); +super("Apple", 1.8d, info)

Re: Code Review Request: 7122707 update Security Providers to JDK8

src/macosx/classes/apple/security/AppleProvider.java I'm not sure apple provider is using the JDK version as the provider version. -super("Apple", 1.1, info); +super("Apple", 1.8d, info); src/share/classes/sun/security/jgss/wrapp

Code Review Request: 7122707 update Security Providers to JDK8

Hi, I'd like a review of these quick changes to the security providers. 7122707 Security Providers need to have their version numbers updated for JDK8 http://cr.openjdk.java.net/~ascarpino/7122707/webrev.00/ thanks Tony

Re: Code review request: 8012615: Realm.getRealmsList returns realms list in wrong

The changes look fine - I don't care for supporting the chaining since it complicates up the parsing code very much. Oh-well. But at least the new code is way shorter than the older one. It's unfortunate that in order to support this chaining, we ended up wasting more cycles processing the who

Re: Code review request, 8024501, sun.security.mscapi.Key has no definition of serialVersionUID

Looks good. --Max On 9/11/13 10:30 AM, Xuelei Fan wrote: Hi, Please review a fix of a warning issue that sun.security.mscapi.Key has no definition of serialVersionUID. As -Werror specified, this warning cause jdk building error on windows platform. webrev: http://cr.openjdk.java.net/~xuelei/

Code review request, 8024501, sun.security.mscapi.Key has no definition of serialVersionUID

Hi, Please review a fix of a warning issue that sun.security.mscapi.Key has no definition of serialVersionUID. As -Werror specified, this warning cause jdk building error on windows platform. webrev: http://cr.openjdk.java.net/~xuelei/8024501/webrev.00/ Thanks, Xuelei

Re: Code review request: 8011402: Move blacklisting certificate logic from hard code to data

On 09/09/2013 03:15 AM, Weijun Wang wrote: UntrustedCertificates: [65] We should log the exception (could be a parsing error, so we would want to know that) You mean for -Djava.security.debug=certpath? Yes. BlacklistedCertsConverter: I'm a little concerned that this tool re-writes the bl

Re: Code review request, 8024444, Change to use othervm mode of tests in SSLEngineImpl

Looks fine to me. --Sean On 09/09/2013 01:03 AM, Xuelei Fan wrote: Hi, Please review this test update. webrev: http://cr.openjdk.java.net/~xuelei/802/webrev/ This bug had not published in the bugs.sun.com. Here is a short description of the issue: In Oracle JSSE provider, a few caches

Re: Code review request: 8011402: Move blacklisting certificate logic from hard code to data

On 9/7/13 1:26 AM, Sean Mullan wrote: 8011402: Move blacklisting certificate logic from hard code to data X509CertImpl: Might it not be better to store the fingerprints in UntrustedCertificates in a WeakHashMap (using the Certificate as a key)? This way we don't need to add public mutato

Code review request, 8024444, Change to use othervm mode of tests in SSLEngineImpl

Hi, Please review this test update. webrev: http://cr.openjdk.java.net/~xuelei/802/webrev/ This bug had not published in the bugs.sun.com. Here is a short description of the issue: In Oracle JSSE provider, a few caches are used to improve the performance. However, the caches cannot suppor

Re: Code review request: 8011402: Move blacklisting certificate logic from hard code to data

On 09/06/2013 09:30 AM, Weijun Wang wrote: Hi Sean Please review the code changes at 8011402: Move blacklisting certificate logic from hard code to data Hard coded blacklisted certificates are moved out of the class file and now inside a data file. Furthermore, only their fingerprints are r

Re: Code review request: 8011402: Move blacklisting certificate logic from hard code to data

On 2013-09-06 16:25, Weijun Wang wrote: On 9/6/13 10:07 PM, Erik Joelsson wrote: Hello Max, I couldn't find the link to the review but I'm guessing this is the one: http://cr.openjdk.java.net/~weijun/8011402/webrev.00/ Correct, sorry about that. 3. Most important: it only works if both $

Re: Code review request: 8011402: Move blacklisting certificate logic from hard code to data

Hello Max, I couldn't find the link to the review but I'm guessing this is the one: http://cr.openjdk.java.net/~weijun/8011402/webrev.00/ On 2013-09-06 15:30, Weijun Wang wrote: Hi Sean Please review the code changes at 8011402: Move blacklisting certificate logic from hard code to data Ha

Re: Code review request: 8011402: Move blacklisting certificate logic from hard code to data

On 9/6/13 10:07 PM, Erik Joelsson wrote: Hello Max, I couldn't find the link to the review but I'm guessing this is the one: http://cr.openjdk.java.net/~weijun/8011402/webrev.00/ Correct, sorry about that. 3. Most important: it only works if both $(BLACKLISTED_CERTS_SRC_OPEN) and $(BLACKLI

Code review request: 8011402: Move blacklisting certificate logic from hard code to data

Hi Sean Please review the code changes at 8011402: Move blacklisting certificate logic from hard code to data Hard coded blacklisted certificates are moved out of the class file and now inside a data file. Furthermore, only their fingerprints are released in the JRE. The makefile covers bla

Re: [JDK 8] Code review request 7188657, There should be a way to reorder the JSSE ciphers

If no objections, I will push the changeset. Xuelei On 9/4/2013 11:34 AM, Xuelei Fan wrote: > new webrev: http://cr.openjdk.java.net/~xuelei/7188657/webrev.01/ > > On 9/4/2013 10:35 AM, Weijun Wang wrote: >> Mostly good, only some word/style issues. >> >> SSLParameters.java: >> >> 83 * se

Code Review Request: 8004283 SecretKeysBasic.sh failing intermittently

Hi, I'd like a code review, really short, for 8004283 test/sun/security/pkcs11/KeyStore/SecretKeysBasic.sh failing intermittently. http://cr.openjdk.java.net/~ascarpino/8004283/webrev.00/ thanks Tony

Re: Code Review Request: 8004283 SecretKeysBasic.sh failing intermittently

Your fix looks fine to me. Thanks. On 5 Sep 2013, at 18:51, Anthony Scarpino wrote: > Hi, > > I'd like a code review, really short, for 8004283 > test/sun/security/pkcs11/KeyStore/SecretKeysBasic.sh failing intermittently. > > http://cr.openjdk.java.net/~ascarpino/8004283/webrev.00/ > > thank

Re: Code review request: 8012615: Realm.getRealmsList returns realms list in wrong

Hi Valerie Webrev updated: http://cr.openjdk.java.net/~weijun/8012615/webrev.02 Except for some comment, the only real source change is in Realm::parseCapaths: -if (!cfg.exists("capaths", cRealm)) { +if (!cfg.exists("capaths", cRealm, sRealm)) { throw new KrbEx

Re: [JDK 8] Code review request 7188657, There should be a way to reorder the JSSE ciphers

new webrev: http://cr.openjdk.java.net/~xuelei/7188657/webrev.01/ On 9/4/2013 10:35 AM, Weijun Wang wrote: > Mostly good, only some word/style issues. > > SSLParameters.java: > > 83 * server name matchers are set to null, cipher > suites > 84 * preference, wantClientAuth and needCl

Re: [JDK 8] Code review request 7188657, There should be a way to reorder the JSSE ciphers

Mostly good, only some word/style issues. SSLParameters.java: 83 * server name matchers are set to null, cipher suites 84 * preference, wantClientAuth and needClientAuth are set to 85 * false. Why not just use "preferLocalCipherSuites" instead of "cipher suites preferenc

Re: Code review request, JKD-8024068 sun/security/ssl/javax/net/ssl/ServerName/IllegalSNIName.java fails

Same. Brad On 9/1/2013 7:45 PM, Weijun Wang wrote: Code change looks fine. --Max On 9/2/13 10:41 AM, Xuelei Fan wrote: On 9/2/2013 10:38 AM, Xuelei Fan wrote: Hi Weijun, Please review this simple test fix. There is a typo in the test. The issue is exposed after the fix of JDK-8023881. w

Re: Code review request, JKD-8024068 sun/security/ssl/javax/net/ssl/ServerName/IllegalSNIName.java fails

Code change looks fine. --Max On 9/2/13 10:41 AM, Xuelei Fan wrote: On 9/2/2013 10:38 AM, Xuelei Fan wrote: Hi Weijun, Please review this simple test fix. There is a typo in the test. The issue is exposed after the fix of JDK-8023881. webrev: http://cr.openjdk.java.net/~xuelei/8024068/webre

<    4   5   6   7   8   9   10   11   12   13   >