Re: RFR 8180570: Refactor sun/security/mscapi shell tests to plain java tests

2018-03-27 Thread Artem Smotrakov
Hi Max, KeytoolChangeAlias.java: - maybe better to run ks.deleteEntry("13579") in a finally block - should it delete "246810" as well? Otherwise looks good to me :) Artem 2018-03-27 9:51 GMT+02:00 Weijun Wang : > Ping again. > > > On Mar 8, 2018, at 8:37 PM, Weijun

Re: RFR[10] JDK-8190335: Backout changeset for JDK-8176354 due to JDK-8190333

2017-10-30 Thread Artem Smotrakov
Looks good to me. Please make sure that the tests pass after baking out 8176354. Artem On 10/30/2017 12:12 PM, sha.ji...@oracle.com wrote: Hi, It has to backout the changeset for JDK-8176354, because it changed a keystore and caused a lot of DTLS test failures. For more details, please see

Re: RFR 8186884: Test native KDC, Java krb5 lib, and native krb5 lib in one test

2017-09-11 Thread Artem Smotrakov
ccache file. Thanks Max On Sep 8, 2017, at 8:38 PM, Artem Smotrakov <artem.smotra...@oracle.com> wrote: Hi Max, Looks good to me. Below are a couple of minor comments you may want to address. No need a new webrev. Thanks! 1. Proc.java, better to use braces http://www.oracle.com/technetwor

Re: RFR 8186884: Test native KDC, Java krb5 lib, and native krb5 lib in one test

2017-09-08 Thread Artem Smotrakov
=,n=,m=lib1.so,h=lib2.so BasicProc.java More comments inline below. On Sep 7, 2017, at 3:29 PM, Artem Smotrakov <artem.smotra...@oracle.com> wrote: Hi Max, In general, looks fine to me. Below are a couple of comments you might want to address. 1. BasicProc.java, it might be better

Re: RFR[10] 8186057: TLS interoperability testing between different Java versions

2017-09-07 Thread Artem Smotrakov
/SSLParameters.html#setServerNames-java.util.List- Artem On 09/07/2017 11:24 AM, sha.ji...@oracle.com wrote: Hi Artem, On 07/09/2017 16:07, Artem Smotrakov wrote: - Please use try-with-resources if possible (files, sockets, etc) The test uses only JDK 6-supported language features, but try

Re: RFR[10] 8186057: TLS interoperability testing between different Java versions

2017-09-07 Thread Artem Smotrakov
Hi John, Please see inline. On 09/07/2017 10:52 AM, sha.ji...@oracle.com wrote: Then you can build a Cartesian product of all parameters. Client and sever should take parameters, and say if they support all of the or not (for example, JDK 6 might not support all features that JDK 9 does).

Re: RFR 8186884: Test native KDC, Java krb5 lib, and native krb5 lib in one test

2017-09-07 Thread Artem Smotrakov
Hi Max, In general, looks fine to me. Below are a couple of comments you might want to address. 1. BasicProc.java, it might be better to use named constants for parameters for once() method. That would make it easier to understand what each particular onse() call does +

Re: RFR[10] 8186057: TLS interoperability testing between different Java versions

2017-09-07 Thread Artem Smotrakov
Hi John, Thanks for starting working on this. I believe this tool is going to be very helpful. Let me skip some coding comments for a while, I have a couple of comments about the design. The main idea is that it should cover as many cases as it can. Even if it might look a bit redundant, I

[10] RFR: 8182388: Backout 8182143

2017-06-16 Thread Artem Smotrakov
This patch backs out 8182143 because of possible issues on Windows even if we don't have a test to reproduce it. Checking if SunMSCAPI provider is enabled looks like a hack. I filed https://bugs.openjdk.java.net/browse/JDK-8182386 Bug: https://bugs.openjdk.java.net/browse/JDK-8182388 Webrev:

Re: [10] RFR: 8182143: SHA224-based signature algorithms are not enabled for TLSv12 on Windows

2017-06-16 Thread Artem Smotrakov
is changed and the signatures are supported now by MSCapi? Gruss Bernd -- http://bernd.eckenfels.net *From:* security-dev <security-dev-boun...@openjdk.java.net> on behalf of Artem Smotrakov <artem.smotra...@oracle.com> *Se

Re: [10] RFR: 8182143: SHA224-based signature algorithms are not enabled for TLSv12 on Windows

2017-06-15 Thread Artem Smotrakov
now by MSCapi? Gruss Bernd -- http://bernd.eckenfels.net *From:* security-dev <security-dev-boun...@openjdk.java.net> on behalf of Artem Smotrakov <artem.smotra...@oracle.com> *Sent:* Thursday, June 15, 2017 10:57:00

[10] RFR: 8182143: SHA224-based signature algorithms are not enabled for TLSv12 on Windows

2017-06-15 Thread Artem Smotrakov
Hi Xuelei, Could you please take a look at this patch? It enables SHA224-based signature algorithms on Windows since they should be provided not only by SunMSCAPI provider. Please see details in the bug description. The test works fine on all supported platforms. Bug:

Re: RFR(XS) : 8180895 : java/security/AccessController/DoPrivAccompliceTest.java has to be improved

2017-05-26 Thread Artem Smotrakov
@summary doesn't seem to be correct, the test uses user.name system property. Otherwise, looks good for me. Artem On 05/23/2017 10:17 PM, Igor Ignatyev wrote: http://cr.openjdk.java.net/~iignatyev//8180895/webrev.00/index.html 81 lines changed: 37 ins; 23 del; 21 mod; Hi all, could you

Re: [9] RFR: 8176183: sun/security/mscapi/SignedObjectChain.java fails on Windows

2017-03-08 Thread Artem Smotrakov
/classes/java/security/Signature.java#l1197 Artem On 03/08/2017 05:57 PM, Weijun Wang wrote: On 03/09/2017 09:55 AM, Artem Smotrakov wrote: Hi Max, I am not sure if SunMSCAPI has higher priority than SunJCE, SUN, etc on Windows. But this test is for SunMSCAPI provider, so we explicitly set

Re: [9] RFR: 8176183: sun/security/mscapi/SignedObjectChain.java fails on Windows

2017-03-08 Thread Artem Smotrakov
to the correct provider when its init(key) is called. Does this mean you don't need to care about Signature.getInstance(alg,provider)? Thanks Max On 03/09/2017 05:52 AM, Artem Smotrakov wrote: Hello, The test fails with "Key type not supported" error on Windows only with SunMSCAP

[9] RFR: 8176183: sun/security/mscapi/SignedObjectChain.java fails on Windows

2017-03-08 Thread Artem Smotrakov
Hello, The test fails with "Key type not supported" error on Windows only with SunMSCAPI provider. It happens because the test passes an incompatible key object to Signature instance. Please see more details in

Re: RFR 8172975: SecurityTools.keytool() needs to accept user input

2017-01-20 Thread Artem Smotrakov
Wang wrote: Updated http://cr.openjdk.java.net/~weijun/8172975/root/webrev.01/ http://cr.openjdk.java.net/~weijun/8172975/webrev.01/ I'll need this in my other work. Thanks Max On 01/20/2017 07:23 PM, Artem Smotrakov wrote: It's up to you. You can change it now if you have time, or we can do

Re: RFR 8172975: SecurityTools.keytool() needs to accept user input

2017-01-20 Thread Artem Smotrakov
It's up to you. You can change it now if you have time, or we can do it once we need to update jarsigner tests. Artem On 01/20/2017 12:23 PM, Weijun Wang wrote: Also, I am feeling that the jarsigner-related calls are quite complicated. I suggest we use the same method for signing and

Re: RFR 8172975: SecurityTools.keytool() needs to accept user input

2017-01-20 Thread Artem Smotrakov
Hi Max, Please see inline. On 01/19/2017 05:15 PM, Weijun Wang wrote: On 01/19/2017 09:40 PM, Artem Smotrakov wrote: Hi Max, In general, looks okay. Would it be better if it called redirectInput() only if the response file exists? keytool() method might also delete the response file

Re: RFR 8172975: SecurityTools.keytool() needs to accept user input

2017-01-19 Thread Artem Smotrakov
Hi Max, In general, looks okay. Would it be better if it called redirectInput() only if the response file exists? keytool() method might also delete the response file after reading it. These two measures may prevent situations when the response file is unnecessary used. What do you think?

Re: RFR 8171353: New home for SecurityTools.java test utility

2016-12-16 Thread Artem Smotrakov
Hi Max, I am fine with it. Artem On 12/16/2016 12:07 AM, Wang Weijun wrote: Hi Artem I hope you are OK with this change: diff --git a/test/lib/security/SecurityTools.java b/test/lib/testlibrary/jdk/testlibrary/SecurityTools.java rename from test/lib/security/SecurityTools.java rename to

Re: [9] RFR: 8166531: sun/security/ssl/SocketCreation/SocketCreation.java fails intermittently

2016-12-15 Thread Artem Smotrakov
PM, Xuelei Fan wrote: I'm not sure this update would work. As this is a special test, it might be possible to copy/past and update the template code (or old sample code) here as we did to use the old template. Xuelei On 12/15/2016 11:23 AM, Artem Smotrakov wrote: Hello, Please review

[9] RFR: 8166531: sun/security/ssl/SocketCreation/SocketCreation.java fails intermittently

2016-12-15 Thread Artem Smotrakov
Hello, Please review the patch below for sun/security/ssl/SocketCreation/SocketCreation.java test. The test has been observed to fail intermittently, but I couldn't reproduce it by running the test in a loop. The test creates sockets for TLS connection in different ways. Even if the test is

Re: RFR 8075618: Create tests to check jarsigner work with multi-version jar

2016-12-12 Thread Artem Smotrakov
You can use http://hg.openjdk.java.net/jdk9/dev/jdk/file/d4d7f1f0d688/test/lib/security/SecurityTools.java which would simplify the code. This lib was added to be used in such tests. Note that SecurityTools addresses a couple of known issues with running security tools on machines with

Re: [9] RFR JDK-8157529 : Remove intermittent key from javax/net/ssl/DTLS/CipherSuite.java

2016-12-05 Thread Artem Smotrakov
Looks good to me. Artem On 11/30/2016 10:29 PM, Tim Du wrote: Hi , Would you help to review the path for "8157529:Remove intermittent key from javax/net/ssl/DTLS/CipherSuite.java" , the intermittently failed issue was fixed by JDK-8167680 , '@key intermittent ' can be removed.Thanks.

Re: Code Review Request JDK-8170329 New SSLSocket testing template

2016-12-05 Thread Artem Smotrakov
Hi Xuelei, Please see inline. On 12/02/2016 09:48 PM, Xue-Lei Fan wrote: - Exceptions are printed out in startServer/startClient methods, it doesn't look necessary to use suppressed exceptions and initCause() method. What was wrong with the code in runTest() method? The code in runTest()

Re: Code Review Request JDK-8170329 New SSLSocket testing template

2016-12-02 Thread Artem Smotrakov
Please see inline On 12/02/2016 05:25 PM, Xue-Lei Fan wrote: - Why did you remove Peer and Application interfaces? I think those interfaces make SSLSocketTemplate more flexible since it allows override doServerSide/doClientSide logic if necessary - it doesn't seem to be worse. If there is

Re: Code Review Request JDK-8170329 New SSLSocket testing template

2016-12-02 Thread Artem Smotrakov
Hi Xuelei, Please see inline. On 12/02/2016 03:53 PM, Xue-Lei Fan wrote: Let's whether Sean or Weijun can have free cycle for the review of this part. Yeah, that would be great. - Why did you remove Peer and Application interfaces? I think those interfaces make SSLSocketTemplate more

Re: Code Review Request JDK-8170329 New SSLSocket testing template

2016-12-02 Thread Artem Smotrakov
Hi Xuelei, I am not sure how the updates in SimpleValidator relate to the template for JSSE tests. It might be better to separate those changes if I am not missing something. This update in SimpleValidator looks okay to me, but taking into account Sean's comments below, I'll let someone who

Re: [9] RFR: 8168969: Merge SSLSocketSample and SSLSocketTemplate

2016-11-15 Thread Artem Smotrakov
I don't like it, but no lambdas, no nothing in SSLSocketTemplate.java http://cr.openjdk.java.net/~asmotrak/8168969/webrev.01/ Xuelei, Could you please take a look? Artem On 11/02/2016 09:14 AM, Artem Smotrakov wrote: Hi Xuelei, This totally makes sense. But in my opinion we should use new

Re: RFR 8164881: Add more tests for JDK-8139565.

2016-11-15 Thread Artem Smotrakov
Hi Mallikarjuna, I have a couple of comments. 1. I see you extract DSA key size from "jdk.certpath.disabledAlgorithms" security property. I think I would be better not to rely on it, but expect that keys less than 1024 bits are not allowed by default. You can pass a boolean parameter to the

Re: [9] RFR: 8168882: keytool doesn't print certificate info if disabled algorithm was used for signing a jar

2016-11-07 Thread Artem Smotrakov
6 05:47 PM, Artem Smotrakov wrote: Thank you for review Sean. I'll remove the warning then. And I'll update it to reset the security property only if a jar file has been specified. Let me also check how "-printcert -file ..." and "-printcert -sslserver" work. Artem On 11

Re: [9] RFR: 8168882: keytool doesn't print certificate info if disabled algorithm was used for signing a jar

2016-11-03 Thread Artem Smotrakov
Thank you for review Sean. I'll remove the warning then. And I'll update it to reset the security property only if a jar file has been specified. Let me also check how "-printcert -file ..." and "-printcert -sslserver" work. Artem On 11/03/2016 07:27 AM, Wang Weijun wrote: I agree with

Re: [9] RFR: 8168882: keytool doesn't print certificate info if disabled algorithm was used for signing a jar

2016-11-02 Thread Artem Smotrakov
My bad, I missed that. http://cr.openjdk.java.net/~asmotrak/8168882/webrev.02/ Artem On 11/02/2016 06:30 PM, Wang Weijun wrote: On 11/01/2016 11:59 PM, Wang Weijun wrote: >>Main.java: >> >>The warning (and the subsequent empty line) should be printed into System.err. This one?

Re: [9] RFR: 8168969: Merge SSLSocketSample and SSLSocketTemplate

2016-11-02 Thread Artem Smotrakov
before getting more concrete requirements. Agree. Please see above. Artem Best regards, John Jiang Artem The code talks more clearly. Please take a look at my example: http://cr.openjdk.java.net/~jjiang/8168969/example/ Best regards, John Jiang On 2016/11/2 8:54, Artem Smotrakov wrote

Re: [9] RFR: 8168882: keytool doesn't print certificate info if disabled algorithm was used for signing a jar

2016-11-02 Thread Artem Smotrakov
ine. Thanks Max On Nov 2, 2016, at 7:35 AM, Artem Smotrakov <artem.smotra...@oracle.com> wrote: Hello, Please review this small update for keytool. "keytool -printcert -jarfile" doesn't work with jars which were signed with algorithms listed in "jdk.jar.d

Re: [9] RFR: 8168969: Merge SSLSocketSample and SSLSocketTemplate

2016-11-02 Thread Artem Smotrakov
Side()? Then, it unnecessary to re-write the whole doServerSide() (or, set a new server peer). The code talks more clearly. Please take a look at my example: http://cr.openjdk.java.net/~jjiang/8168969/example/ Best regards, John Jiang On 2016/11/2 8:54, Artem Smotrakov wrote: Hello, Pleas

[9] RFR: 8168969: Merge SSLSocketSample and SSLSocketTemplate

2016-11-01 Thread Artem Smotrakov
Hello, Please review the following patch which merges a couple of classes in javax/net/ssl/templates. SSLTest class contains re-usable parts of SSLSocketSample. SSLSocketTemplate class is buggy (tests which follows it may fail intermittently). I basically replaced SSLSocketTemplate with

[9] RFR: 8168882: keytool doesn't print certificate info if disabled algorithm was used for signing a jar

2016-11-01 Thread Artem Smotrakov
Hello, Please review this small update for keytool. "keytool -printcert -jarfile" doesn't work with jars which were signed with algorithms listed in "jdk.jar.disabledAlgorithms" security property. The patch below resets "jdk.jar.disabledAlgorithms" security property before reading a jar

Re: Code Review Request, JDK-8167680, DTLS implementation bugs

2016-10-28 Thread Artem Smotrakov
Hi Xuelei, Looks good to me. Artem On 10/27/2016 05:50 AM, Xuelei Fan wrote: Updated to handle handle optional CertificateVerify message: http://cr.openjdk.java.net/~xuelei/8167680/webrev.02/ Thanks, Xuelei On 10/21/2016 11:31 AM, Xuelei Fan wrote: Updated webrev per Jamil's

Re: Code Review Request JDK-8161106 Improve SSLSocket test template

2016-10-26 Thread Artem Smotrakov
There is SSLTest.java which follows SSLSocketSample.java and can be used by other tests. Artem On 10/26/2016 09:45 AM, Xuelei Fan wrote: The new test case is just a test in order to make sure this approach works in the testing environment. I plan to remove both of the sample and template,

Re: RFR[9] JDK-8168064: sun/security/ssl/ServerHandshaker/AnonCipherWithWantClientAuth.java failed with "Received fatal alert: handshake_failure"

2016-10-26 Thread Artem Smotrakov
Hi John, Looks good to me, thank you for the update. Artem On 10/26/2016 04:45 AM, John Jiang wrote: Hi Artem, Please take a look at this version: http://cr.openjdk.java.net/~jjiang/8168064/webrev.02/ It set a new Server peer. Best regards, John Jiang On 2016/10/25 1:33, Artem Smotrakov

Re: RFR[9] JDK-8168064: sun/security/ssl/ServerHandshaker/AnonCipherWithWantClientAuth.java failed with "Received fatal alert: handshake_failure"

2016-10-24 Thread Artem Smotrakov
the fixing. Best regards, John Jiang On 2016/10/22 1:50, Artem Smotrakov wrote: Hi John, It may be better to use SSLTest.java to avoid duplicate code. The class basically contains parts of SSLSocketSample.java http://hg.openjdk.java.net/jdk9/dev/jdk/file/0fb9ba19a63a/test/javax/net/ssl

Re: RFR[9] JDK-8168064: sun/security/ssl/ServerHandshaker/AnonCipherWithWantClientAuth.java failed with "Received fatal alert: handshake_failure"

2016-10-21 Thread Artem Smotrakov
Hi John, It may be better to use SSLTest.java to avoid duplicate code. The class basically contains parts of SSLSocketSample.java http://hg.openjdk.java.net/jdk9/dev/jdk/file/0fb9ba19a63a/test/javax/net/ssl/templates/SSLTest.java Here is an example

[9] RFR: 8166530: sun/net/www/protocol/https/HttpsClient/ProxyAuthTest.java fails intermittently

2016-10-07 Thread Artem Smotrakov
Hello, Please review the patch below for sun/net/www/protocol/https/HttpsClient/ProxyAuthTest.java test. The test has been observed to fail intermittently, but the failure is not reproducible standalone. The patch updates the test to follow the approach from SSLSocketSample.java

Re: [9] RFR: JDK-8164322: sun/security/pkcs11/PKCS11Test.java shall be updated to run on ARM platforms

2016-09-28 Thread Artem Smotrakov
Hi Xuelei, I understand your concerns. But I'd prefer to be aware of situations when a test reports that it passed when it actually did nothing. How about using @requires key? We can try to specify all expected platforms. If I understand correctly, jtreg won't run tests if specified

Re: [9] RFR: JDK-8164322: sun/security/pkcs11/PKCS11Test.java shall be updated to run on ARM platforms

2016-09-27 Thread Artem Smotrakov
(cc'ing Denis who reported the bug) I think making PKCS11 tests fail on unexpected platform would be helpful for people who port JDK on new platforms and run tests on them. Currently the tests silently quit which looks like they pass. This makes people think that everything went smoothly, but

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

2016-09-20 Thread Artem Smotrakov
Smotrakov wrote: Hi Xuelei, Chris, Thank you for looking into it. Please see inline. On 09/15/2016 12:53 AM, Chris Hegarty wrote: On 15 Sep 2016, at 02:55, Xuelei Fan <xuelei@oracle.com> wrote: On 9/15/2016 9:45 AM, Artem Smotrakov wrote: Well, in this particular case it's not

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

2016-09-15 Thread Artem Smotrakov
Hi Xuelei, Chris, Thank you for looking into it. Please see inline. On 09/15/2016 12:53 AM, Chris Hegarty wrote: On 15 Sep 2016, at 02:55, Xuelei Fan <xuelei@oracle.com> wrote: On 9/15/2016 9:45 AM, Artem Smotrakov wrote: Well, in this particular case it's not clear that it has th

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

2016-09-14 Thread Artem Smotrakov
, so I would like to keep debug output on. Artem On 09/14/2016 06:39 PM, Xuelei Fan wrote: On 9/15/2016 9:23 AM, Artem Smotrakov wrote: Hi Xuelei, For this one, I am not sure that it would help here since the test failed after it already had started handshaking. It has the same issue

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

2016-09-14 Thread Artem Smotrakov
the SSLSocketSample.java template as the comment in JDK-8163924 review thread. Thanks, Xuelei On 9/15/2016 9:13 AM, Artem Smotrakov wrote: Not urgent, but I would appreciate if someone can get a chance to look at this. Artem On 09/07/2016 03:17 PM, Artem Smotrakov wrote: Sending to net

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

2016-09-14 Thread Artem Smotrakov
Not urgent, but I would appreciate if someone can get a chance to look at this. Artem On 09/07/2016 03:17 PM, Artem Smotrakov wrote: Sending to net-...@openjdk.java.net as well. Artem On 09/07/2016 12:28 PM, Artem Smotrakov wrote: Hello, Please review the following patch for sun/net

Re: [9] RFR: 8163924: SSLEngineBadBufferArrayAccess.java fails intermittently with Unrecognized SSL message

2016-09-14 Thread Artem Smotrakov
/templates? Thanks, Xuelei On 9/14/2016 9:31 AM, Artem Smotrakov wrote: Hello, Not urgent, but I would appreciate if somebody can take a look at the webrev below and webvev for 8164591. Thank you! Artem On 09/07/2016 02:58 PM, Artem Smotrakov wrote: Hello, Please review the following patch

Re: [9] RFR: 8163924: SSLEngineBadBufferArrayAccess.java fails intermittently with Unrecognized SSL message

2016-09-13 Thread Artem Smotrakov
Hello, Not urgent, but I would appreciate if somebody can take a look at the webrev below and webvev for 8164591. Thank you! Artem On 09/07/2016 02:58 PM, Artem Smotrakov wrote: Hello, Please review the following patch for SSLEngineBadBufferArrayAccess.java test. The test has been

Re: [9] RFR: 8156054: Test Task: Develop new tests for JEP C155: Remove FilePermission Pathname Canonicalization

2016-09-13 Thread Artem Smotrakov
Hi Siba, I see that the test expects only true or false. You can just pass these boolean values, and check() can make sure that everything returns the expected boolean value without building a string. That would be clearer to me. Not an issue, it's up to you to change it or not. Artem On

Re: RFR[9] JDK-8077138: Some PKCS11 tests fail because NSS library is not initialized

2016-09-13 Thread Artem Smotrakov
I am wondering if we really need to put nss-3.16-with-nspr-4.10.4.tar.gz in to the repository. Would it be better just to provide a link to this archive? Artem On 09/13/2016 02:43 AM, John Jiang wrote: Hi, Please review this patch for fixing JDK-8077138. The solution is re-building NSS

Re: [9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

2016-09-07 Thread Artem Smotrakov
Sending to net-...@openjdk.java.net as well. Artem On 09/07/2016 12:28 PM, Artem Smotrakov wrote: Hello, Please review the following patch for sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java The test has been observed to fail a couple of times, but it's still not clear why

[9] RFR: 8163924: SSLEngineBadBufferArrayAccess.java fails intermittently with Unrecognized SSL message

2016-09-07 Thread Artem Smotrakov
Hello, Please review the following patch for SSLEngineBadBufferArrayAccess.java test. The test has been observed to fail intermittently with "Unrecognized SSL message" error. I couldn't reproduce it, and it's currently not clear what caused the test to fail. One guess is that something else

[9] RFR: 8164591: sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java failed with SSLHandshakeException

2016-09-07 Thread Artem Smotrakov
Hello, Please review the following patch for sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java The test has been observed to fail a couple of times, but it's still not clear why it failed because there is not much info in logs. The patch updates the test to enable additional

Re: [jdk9] (S) RFR: 8165463: Native implementation of sunmscapi should use operator new (nothrow) for allocations

2016-09-06 Thread Artem Smotrakov
text, 1411 CERT_NAME_FRIENDLY_DISPLAY_TYPE, 0, NULL, pszNameString, 1412 cchNameString) == 1) { 1413 1414 continue; // not found 1415 } ... Thanks! Artem On 09/06/2016 01:07 PM, Ivan Gerasimov wrote: Hi Artem! On 06.09.2016 20:45, Artem Smotr

Re: [jdk9] (S) RFR: 8165463: Native implementation of sunmscapi should use operator new (nothrow) for allocations

2016-09-06 Thread Artem Smotrakov
Hi Ivan, It's not really about your changes, but I am wondering if "delete [] pszNameString" should be called before "continue" in line 1414? Looks like a potential memory leak. Artem On 09/06/2016 02:54 AM, Ivan Gerasimov wrote: Thank you Christoph for looking into this! On 05.09.2016

Re: RFR 8157404: Unable to read certain PKCS12 keystores from SequenceInputStream

2016-08-29 Thread Artem Smotrakov
01/ <http://cr.openjdk.java.net/%7Esnikandrova/8157404/webrev.01/> Thanks, Svetlana On 26.08.2016 23:48, Artem Smotrakov wrote: Hi Svetlana, DerValue class may be implicitly used in different areas (x509, SSL/TLS, keystores, maybe krb5, etc). Please make sure that tests from jdk_secur

Re: RFR 8157404: Unable to read certain PKCS12 keystores from SequenceInputStream

2016-08-26 Thread Artem Smotrakov
Hi Svetlana, DerValue class may be implicitly used in different areas (x509, SSL/TLS, keystores, maybe krb5, etc). Please make sure that tests from jdk_security pass. I'll leave the main review to someone who is more knowledgeable in this area, here are a couple of comments: - Please

Re: RFR 8164533: [TEST_BUG] sun/security/ssl/SSLSocketImpl/CloseSocket.java failed with "Error while cleaning up threads after test"

2016-08-25 Thread Artem Smotrakov
in a separate JVM. I believe most of test are fine now. I rely on your expertise, and I'll let you decide about it. Artem On 08/25/2016 10:45 AM, Xuelei Fan wrote: On Aug 26, 2016, at 1:33 AM, Artem Smotrakov <artem.smotra...@oracle.com> wrote: Hi Xuelei, On 08/25/2016 10:26 AM, Xuel

Re: RFR 8164533: [TEST_BUG] sun/security/ssl/SSLSocketImpl/CloseSocket.java failed with "Error while cleaning up threads after test"

2016-08-25 Thread Artem Smotrakov
Hi Xuelei, On 08/25/2016 10:26 AM, Xuelei Fan wrote: On Aug 26, 2016, at 12:43 AM, Artem Smotrakov <artem.smotra...@oracle.com> wrote: Hi Xuelei, I am not sure why you think it's too strong to follow to it. To me it's pretty easy to see if a test calls System.setPr

Re: RFR 8164533: [TEST_BUG] sun/security/ssl/SSLSocketImpl/CloseSocket.java failed with "Error while cleaning up threads after test"

2016-08-25 Thread Artem Smotrakov
no such modifications, can share the *same* VM using agentvm. It's too strong to follow. And it is not easy to know there is no static fields update in a test case. Xuelei On 8/25/2016 13:31, Xuelei Fan wrote: On 8/25/2016 12:31 PM, Artem Smotrakov wrote: Those tests which modify default JSSE parameters

Re: RFR 8164533: [TEST_BUG] sun/security/ssl/SSLSocketImpl/CloseSocket.java failed with "Error while cleaning up threads after test"

2016-08-24 Thread Artem Smotrakov
t; On 8/25/2016 11:17 AM, Artem Smotrakov wrote: > Hi Xuelei, > > Yes, I know that JSSE provider initializes only once. But I suppose that > tests which use default JSSE configuration (like this test) can be safely run > in agent VM. Am I missing something? > The d

Re: RFR 8164533: [TEST_BUG] sun/security/ssl/SSLSocketImpl/CloseSocket.java failed with "Error while cleaning up threads after test"

2016-08-24 Thread Artem Smotrakov
/25/2016 9:27 AM, Artem Smotrakov wrote: >> BTW, please run the test in othervm mode. > Why do you think it's necessary here? I don't see the test modifies > anything that may affect other tests running in the same JVM (for > example, system properties). Am I missing something? It'

Re: RFR 8164533: [TEST_BUG] sun/security/ssl/SSLSocketImpl/CloseSocket.java failed with "Error while cleaning up threads after test"

2016-08-24 Thread Artem Smotrakov
Hi Xuelei, Please see inline. On 08/24/2016 06:10 PM, Xuelei Fan wrote: On 8/25/2016 3:55 AM, Artem Smotrakov wrote: Hi Svetlana, Thank you for cleaning up this test. I have a couple of comments (mostly about the original test). 1. I see that the test tries to connect to a server three

Re: RFR 8164533: [TEST_BUG] sun/security/ssl/SSLSocketImpl/CloseSocket.java failed with "Error while cleaning up threads after test"

2016-08-24 Thread Artem Smotrakov
Hi Svetlana, Thank you for cleaning up this test. I have a couple of comments (mostly about the original test). 1. I see that the test tries to connect to a server three times, but the server accept only first connection, and then it stops. So test cases #2-3 fail just because the

Re: RFR: (XS) 8162916:Test sun/security/krb5/auto/UnboundSSL.java fails

2016-08-18 Thread Artem Smotrakov
Hi Sean, The patch below looks fine to me, but I am not an official reviewer. Artem On 08/18/2016 08:28 AM, Seán Coffey wrote: Thanks for the tip Artem, Max. No need to modify the policy file then. Below is the new suggested patch for jdk8u-dev. JPRT results are good. diff --git

[9] RFR: 8164100: com/sun/crypto/provider/KeyFactory/TestProviderLeak.java fails with java.util.concurrent.TimeoutException

2016-08-17 Thread Artem Smotrakov
Hello, Please review the following patch for com/sun/crypto/provider/KeyFactory/TestProviderLeak.java test. This is a request to make the test take into account a test timeout factor. Timeout factor can be specified with "-timeout" jtreg's command line option. This option is used in some

Re: RFR: (XS) 8162916:Test sun/security/krb5/auto/UnboundSSL.java fails

2016-08-17 Thread Artem Smotrakov
jdk8u-dev. The bug is marked 9-na. The provider loading changes made in this area for 9 mean that it's not affected. Regards, Sean. On 17/08/16 18:10, Artem Smotrakov wrote: Hi Sean, If I remember correctly, there is no ext directory in JDK 9 any more. I don't see in jtr file that &quo

Re: RFR: (XS) 8162916:Test sun/security/krb5/auto/UnboundSSL.java fails

2016-08-17 Thread Artem Smotrakov
Hi Sean, If I remember correctly, there is no ext directory in JDK 9 any more. I don't see in jtr file that "java.ext.dirs" system property is passed to the test. If I understand correctly, "file:${{java.ext.dirs}}/*" becomes "file:/*" which seems to grand all permissions to all the code. It

Re: [9] RFR: 8162484: javax/net/ssl/Stapling/SSLSocketWithStapling.java test fails intermittently with "Address already in use" error

2016-08-15 Thread Artem Smotrakov
to bootup again, is the delay still needed? Otherwise, looks fine to me. Thanks, Xuelei On 8/13/2016 6:25 AM, Artem Smotrakov wrote: Thank you for review Jamil. Xuelei, Could you please take a look? Artem On 08/12/2016 02:38 PM, Jamil Nimeh wrote: Thank you Artem. The fix looks good. You

Re: [9] RFR: 8162484: javax/net/ssl/Stapling/SSLSocketWithStapling.java test fails intermittently with "Address already in use" error

2016-08-12 Thread Artem Smotrakov
Thank you for review Jamil. Xuelei, Could you please take a look? Artem On 08/12/2016 02:38 PM, Jamil Nimeh wrote: Thank you Artem. The fix looks good. You just need a +1 from an official reviewer. --Jamil Original message From: Artem Smotrakov <artem.smo

Re: [9] RFR: 8162484: javax/net/ssl/Stapling/SSLSocketWithStapling.java test fails intermittently with "Address already in use" error

2016-08-12 Thread Artem Smotrakov
that we can say for all intended uses that we'll *never* need to restart it. That's why I'd like to keep the unbound socket/set sockopt/bind/listen behavior. I don't think ServerSocket(0) achieves that. --Jamil On 8/12/2016 11:30 AM, Artem Smotrakov wrote: Hi Jamil, There was no any specific

Re: [9] RFR: 8162484: javax/net/ssl/Stapling/SSLSocketWithStapling.java test fails intermittently with "Address already in use" error

2016-08-12 Thread Artem Smotrakov
, Artem Smotrakov wrote: Hi Jamil, Thank you for review. Please see inline. On 08/10/2016 04:16 PM, Jamil Nimeh wrote: Hi Artem, I'm not an official reviewer but the solution for making the servers reject connections rather than stop and start looks pretty fair to me and seems like a nice way

Re: [9] RFR: 8162484: javax/net/ssl/Stapling/SSLSocketWithStapling.java test fails intermittently with "Address already in use" error

2016-08-11 Thread Artem Smotrakov
k.java.net/~asmotrak/8162484/webrev.01/ Artem --Jamil On 08/10/2016 03:44 PM, Artem Smotrakov wrote: Hello, Please review this update for OCSP stapling tests. The tests use test/java/security/testlibrary/SimpleOCSPServer.java which try to re-use a server port if the server restarted.

[9] RFR: 8162484: javax/net/ssl/Stapling/SSLSocketWithStapling.java test fails intermittently with "Address already in use" error

2016-08-10 Thread Artem Smotrakov
Hello, Please review this update for OCSP stapling tests. The tests use test/java/security/testlibrary/SimpleOCSPServer.java which try to re-use a server port if the server restarted. Looks like sometimes it may cause "Address already in use" error. The patch updates OCSP stapling tests

Re: RFR 8133910: Some sun/security/tools tests failed.

2016-08-09 Thread Artem Smotrakov
Hi Max, The update looks good to me. Artem On 08/09/2016 08:39 AM, Weijun Wang wrote: I was wrong. The test were written by Artem. --Max On 8/9/2016 15:37, Wang Weijun wrote: Please review the fix at http://cr.openjdk.java.net/~weijun/8133910/webrev.00/ Basically,

Re: RFR 8163489: Avoid using Utils.getFreePort() in TsacertOptionTest.java test

2016-08-09 Thread Artem Smotrakov
+1 Minor: no need a semicolon in "try" block, I originally added it by mistake. You also may want to update a copyright year. Artem On 08/09/2016 08:46 AM, Chris Hegarty wrote: On 9 Aug 2016, at 16:37, Weijun Wang wrote:

Re: [9] RFR: 8159416: javax/net/ssl/DTLS/CipherSuite.java failed on timeout

2016-08-02 Thread Artem Smotrakov
Please see an updated webrev http://cr.openjdk.java.net/~asmotrak/8159416/webrev.04/ Artem On 08/02/2016 02:42 PM, Artem Smotrakov wrote: Hi Xuelei, Thank you for review. Please see inline. I'll send an updated webrev soon. On 08/01/2016 09:35 PM, Xuelei Fan wrote: DTLSOutputRecord

Re: [9] RFR: 8159416: javax/net/ssl/DTLS/CipherSuite.java failed on timeout

2016-08-02 Thread Artem Smotrakov
problematic test. Or alternatively, we can ask the JTREG to consider the behavior more about the debug length restrictions. Thanks, Xuelei On 8/2/2016 9:25 AM, Artem Smotrakov wrote: Here is an updated webrev which includes a fix for 8161086 (thanks Xuelei for help): - No updates to the problem l

Re: [9] RFR: 8159416: javax/net/ssl/DTLS/CipherSuite.java failed on timeout

2016-08-01 Thread Artem Smotrakov
is specified - Updated InvalidRecords.java tests not to drop packets by default because handshaking unexpectedly succeeds if an invalid packet was dropped http://cr.openjdk.java.net/~asmotrak/8159416/webrev.03/ Please take a look. Artem On 07/28/2016 04:36 PM, Artem Smotrakov wrote: Hi Xuelei, I

Re: [9] RFR: 8159416: javax/net/ssl/DTLS/CipherSuite.java failed on timeout

2016-07-28 Thread Artem Smotrakov
conditions. Could you please take a look at updated webrev? http://cr.openjdk.java.net/~asmotrak/8159416/webrev.02/ Artem On 06/27/2016 06:25 PM, Xuelei Fan wrote: On 6/28/2016 9:12 AM, Artem Smotrakov wrote: Hello, Please review this patch for javax/net/ssl/DTLS tests. A couple of DTLS

Re: RFR 8054536: sun.security.x509.Extension object may throw NPE for hashCode and equals method

2016-07-15 Thread Artem Smotrakov
licate code. As for unnecessary try-catch in test I'd prefer to have it to emphasis that we are checking for NPE. Okay. I'm not sure about "iff" but let it be, it seems like a right place to use it. Sure. Artem Thank you, Svetlana On 14.07.2016 19:35, Artem Smotrakov wrote: Hi

Re: RFR 8054536: sun.security.x509.Extension object may throw NPE for hashCode and equals method

2016-07-14 Thread Artem Smotrakov
Hi Svetlana, I'll leave the main review to an official reviewer, but I have a couple of comments. There are a couple of other places in Extension.java where NPE may occur: - line 255: I see that "extensionId" is checked for null in other methods, but not in getId() - line 200: I see that

Re: Code Review Request JDK-8161106 Improve SSLSocket test template

2016-07-13 Thread Artem Smotrakov
Hi Xuelei, The webrev looks good to me. Please see inline. On 07/12/2016 10:36 PM, Xuelei Fan wrote: Thanks for the feedback, Artem. Here is the updated webrev per your suggestions: http://cr.openjdk.java.net/~xuelei/8161106/webrev.02/ On 7/13/2016 1:03 AM, Artem Smotrakov wrote: Hi

Re: Code Review Request JDK-8161106 Improve SSLSocket test template

2016-07-12 Thread Artem Smotrakov
Hi Xuelei, I am not an official reviewer, but I have a couple of comments. 1. line 149: would it be better to check this condition in a loop? 2. Using try-with-resources blocks might simplify doServerSide() a little bit (no need to call close() on sockets, and a couple of "try" blocks might

Re: [9] RFR: 8159416: javax/net/ssl/DTLS/CipherSuite.java failed on timeout

2016-06-28 Thread Artem Smotrakov
UDP sockets. If we want to test real connections, then it should be different tests which take into account #1 and #2 above. Artem On 06/27/2016 06:25 PM, Xuelei Fan wrote: On 6/28/2016 9:12 AM, Artem Smotrakov wrote: Hello, Please review this patch for javax/net/ssl/DTLS tests. A couple

[9] RFR: 8159416: javax/net/ssl/DTLS/CipherSuite.java failed on timeout

2016-06-27 Thread Artem Smotrakov
Hello, Please review this patch for javax/net/ssl/DTLS tests. A couple of DTLS tests fail intermittently on Mac with timeout or "Too many handshake loops ..." error. The tests use UDP to transfer DTLS records. It happens sometimes that server cannot receive UDP packets with

Re: [9] RFR: 8152745: javax/net/ssl/TLS/TestJSSE.java fails intermittently: Unsupported or unrecognized SSL message

2016-06-21 Thread Artem Smotrakov
dex-v6.html It has a recommendation about line lengths, but it allows lines more than 80 characters if it makes it more readable. Anyway, I avoided lines more than 80 characters (mostly I renamed variables). Please take a look at updated webrev: http://cr.openjdk.java.net/~asmotrak/8152745/w

Re: [9] RFR: 8074580: sun/security/pkcs11/rsa/TestKeyPairGenerator.java fails due to PKCS11Exception: CKR_FUNCTION_FAILED

2016-06-21 Thread Artem Smotrakov
I forgot to include PKCS11.java to webrev, here is an updated webrev http://cr.openjdk.java.net/~asmotrak/8074580/webrev.01/ Artem On 06/20/2016 11:12 AM, Artem Smotrakov wrote: Hello, Please review the following patch below for 9. TestKeyPairGenerator.java test intermittently fails

[9] RFR: 8152745: javax/net/ssl/TLS/TestJSSE.java fails intermittently: Unsupported or unrecognized SSL message

2016-06-21 Thread Artem Smotrakov
Hello, Please review the patch below for javax/net/ssl/TLS/TestJSSE.java test. The test has been observed to fail intermittently with "Unsupported or unrecognized SSL" error. But I couldn't reproduce it manually while running the test in a loop for a couple of days on Linux x64. For now

[9] RFR: 8049314: javax/net/ssl/templates/SSLSocketSSLEngineTemplate.java fails intermittently with "Unexpected EOF" message

2016-06-20 Thread Artem Smotrakov
Hello, Please review this patch for SSLSocketSSLEngineTemplate.java test. The test fails intermittently with "Unexpected EOF" error. Please see Brad's comment which explains the issue

[9] RFR: 8074580: sun/security/pkcs11/rsa/TestKeyPairGenerator.java fails due to PKCS11Exception: CKR_FUNCTION_FAILED

2016-06-20 Thread Artem Smotrakov
Hello, Please review the following patch below for 9. TestKeyPairGenerator.java test intermittently fails with CKR_FUNCTION_FAILED error when NSS crypto libs is used via SunPKCS11 provider. Looks like the root cause is a bug 1012786 in NSS

[9] RFR: 8159501: ShortRSAKey512.java intermittently times out

2016-06-17 Thread Artem Smotrakov
Hello, Please review the patch below for javax/net/ssl/TLSv12/ShortRSAKey512.java test. The test has been seen to fail intermittently with a time out. I was not able to reproduce this failure. The test fails with "Client died ..." message which occurs in case of exception on client side.

Re: [9] RFR: 8129389: javax/net/ssl/DTLS tests fail intermittently

2016-05-19 Thread Artem Smotrakov
I added more output for debugging intermittent failures in https://bugs.openjdk.java.net/browse/JDK-8132320 I also updated produceHandshakePackets() to handle NEED_UNWRAP_AGAIN. http://cr.openjdk.java.net/~asmotrak/8129389/webrev.01/ Artem On 05/18/2016 08:28 PM, Artem Smotrakov wrote

[9] RFR: 8157344: Multiple test timeouts after push for JDK-8141039

2016-05-19 Thread Artem Smotrakov
Hello, Please review this patch for SecureRandom tests which may fail with timeout because SeedGenerator. generateSeed() may block on /dev/random. The tests now use /dev/urandom instead. They also run in othervm mode since "java.security.egd" system property seems to be read once while

  1   2   >