Re: RFR: 8282723: Add constructors taking a cause to JSSE exceptions [v5]

2022-03-18 Thread Xue-Lei Andrew Fan
On Fri, 18 Mar 2022 21:08:56 GMT, Bradford Wetmore  wrote:

>> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   more test case udpate
>
> test/jdk/javax/net/ssl/SSLException/CheckSSLHandshakeException.java line 32:
> 
>> 30: import java.util.Objects;
>> 31: 
>> 32: public class CheckSSLHandshakeException {
> 
> My personal preference would have been to combine all of these into a single 
> testfile to minimize clutter in the logs and directories, but no strong 
> objection.

I understand. I would like to have them in separated test cases so that it is 
easier to monitor the regressions in the future.

-

PR: https://git.openjdk.java.net/jdk/pull/7722


Re: RFR: 8282723: Add constructors taking a cause to JSSE exceptions [v5]

2022-03-18 Thread Bradford Wetmore
On Fri, 18 Mar 2022 23:05:36 GMT, Iris Clark  wrote:

>> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   more test case udpate
>
> Marked as reviewed by iris (Reviewer).

Thanks, @irisclark!

-

PR: https://git.openjdk.java.net/jdk/pull/7722


Re: RFR: 8282723: Add constructors taking a cause to JSSE exceptions [v5]

2022-03-18 Thread Iris Clark
On Tue, 8 Mar 2022 08:02:16 GMT, Xue-Lei Andrew Fan  wrote:

>> Please review this small API enhancement to add the usual constructors 
>> taking a cause to javax.net.ssl exceptions.  The use of initCause in the 
>> JSSE implementation code is updated to use the new constructors accordingly.
>> 
>> Please review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282724
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   more test case udpate

Marked as reviewed by iris (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/7722


Re: RFR: 8282723: Add constructors taking a cause to JSSE exceptions [v5]

2022-03-18 Thread Iris Clark
On Fri, 18 Mar 2022 21:00:40 GMT, Bradford Wetmore  wrote:

>> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   more test case udpate
>
> test/jdk/javax/net/ssl/SSLException/CheckSSLHandshakeException.java line 2:
> 
>> 1: /*
>> 2:  * Copyright (C) 2022 THL A29 Limited, a Tencent company. All rights 
>> reserved.
> 
> I am unsure about the third-party copyrights on these files.  They are 
> probably ok, but you should get approval by someone more familiar.
> 
> Secondly, in the Oracle environment, we generally use a trailing , comma 
> after the date.  e.g.
> 
> `Copyright (C) 2022, `

The location of the copyright is fine.  No need to enforce Oracle date 
formatting.

-

PR: https://git.openjdk.java.net/jdk/pull/7722


Re: RFR: 8282723: Add constructors taking a cause to JSSE exceptions [v5]

2022-03-18 Thread Bradford Wetmore
On Tue, 8 Mar 2022 08:02:16 GMT, Xue-Lei Andrew Fan  wrote:

>> Please review this small API enhancement to add the usual constructors 
>> taking a cause to javax.net.ssl exceptions.  The use of initCause in the 
>> JSSE implementation code is updated to use the new constructors accordingly.
>> 
>> Please review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282724
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   more test case udpate

Please see latest comments and let's verify the copyright is correct, but 
otherwise looks good.

test/jdk/javax/net/ssl/SSLException/CheckSSLHandshakeException.java line 32:

> 30: import java.util.Objects;
> 31: 
> 32: public class CheckSSLHandshakeException {

My personal preference would have been to combine all of these into a single 
testfile to minimize clutter in the logs and directories, but no strong 
objection.

test/jdk/javax/net/ssl/templates/SSLSocketSSLEngineTemplate.java line 349:

> 347: }
> 348: }
> 349: } finally {

Copyright update

test/jdk/javax/net/ssl/templates/SSLSocketTemplate.java line 544:

> 542: /*
> 543:  * Check various exception conditions.
> 544:  */

Copyright update

-

Marked as reviewed by wetmore (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7722


Re: RFR: 8282723: Add constructors taking a cause to JSSE exceptions [v5]

2022-03-18 Thread Bradford Wetmore
On Tue, 8 Mar 2022 08:02:16 GMT, Xue-Lei Andrew Fan  wrote:

>> Please review this small API enhancement to add the usual constructors 
>> taking a cause to javax.net.ssl exceptions.  The use of initCause in the 
>> JSSE implementation code is updated to use the new constructors accordingly.
>> 
>> Please review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282724
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   more test case udpate

test/jdk/javax/net/ssl/ALPN/SSLServerSocketAlpnTest.java line 487:

> 485: if ((local != null) && (remote != null)) {
> 486: // If both failed, return the curthread's exception.
> 487: local.addSuppressed(remote);

Copyright 2016->2022.

test/jdk/javax/net/ssl/ALPN/SSLSocketAlpnTest.java line 483:

> 481: if ((local != null) && (remote != null)) {
> 482: // If both failed, return the curthread's exception.
> 483: local.addSuppressed(remote);

Copyright 2016->2022.

test/jdk/javax/net/ssl/SSLException/CheckSSLHandshakeException.java line 2:

> 1: /*
> 2:  * Copyright (C) 2022 THL A29 Limited, a Tencent company. All rights 
> reserved.

I am unsure about the copyrights on these files.  They are probably ok, but you 
should get approval by someone more familiar.

-

PR: https://git.openjdk.java.net/jdk/pull/7722


Re: RFR: 8282723: Add constructors taking a cause to JSSE exceptions [v5]

2022-03-18 Thread Xue-Lei Andrew Fan
On Tue, 8 Mar 2022 08:02:16 GMT, Xue-Lei Andrew Fan  wrote:

>> Please review this small API enhancement to add the usual constructors 
>> taking a cause to javax.net.ssl exceptions.  The use of initCause in the 
>> JSSE implementation code is updated to use the new constructors accordingly.
>> 
>> Please review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282724
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   more test case udpate

ping ...

-

PR: https://git.openjdk.java.net/jdk/pull/7722


Re: RFR: 8282723: Add constructors taking a cause to JSSE exceptions [v5]

2022-03-08 Thread Xue-Lei Andrew Fan
> Please review this small API enhancement to add the usual constructors taking 
> a cause to javax.net.ssl exceptions.  The use of initCause in the JSSE 
> implementation code is updated to use the new constructors accordingly.
> 
> Please review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282724

Xue-Lei Andrew Fan has updated the pull request incrementally with one 
additional commit since the last revision:

  more test case udpate

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7722/files
  - new: https://git.openjdk.java.net/jdk/pull/7722/files/37953310..61cc7934

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7722=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7722=03-04

  Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7722.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7722/head:pull/7722

PR: https://git.openjdk.java.net/jdk/pull/7722