Re: RFR: 8280494: (D)TLS signature schemes [v18]

2022-03-05 Thread Xue-Lei Andrew Fan
On Fri, 4 Mar 2022 16:29:04 GMT, Sean Mullan  wrote:

> Since this new API is also intended to be supported for DTLS, have you added 
> implementation support for that, and if so have you added a test for it?

Yes, the DTLS implementation is included.  I added a test case for DTLS.

-

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


Re: RFR: 8280494: (D)TLS signature schemes [v19]

2022-03-05 Thread Xue-Lei Andrew Fan
> This update is to support signature schemes customization for individual 
> (D)TLS connection.  Please review the CSR as well:
> CSR: https://bugs.openjdk.java.net/browse/JDK-8280495
> RFE: https://bugs.openjdk.java.net/browse/JDK-8280494
> Release-note: https://bugs.openjdk.java.net/browse/JDK-8281290

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

  add test for DTLS

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7252/files
  - new: https://git.openjdk.java.net/jdk/pull/7252/files/5a98c921..ca006ee0

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7252&range=18
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7252&range=17-18

  Stats: 134 lines in 1 file changed: 134 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7252.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7252/head:pull/7252

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


Re: RFR: JDK-8282686: Add constructors taking a cause to SocketException

2022-03-05 Thread Lance Andersen
On Fri, 4 Mar 2022 21:17:01 GMT, Joe Darcy  wrote:

> Please review this small API enhancement to add the usual constructors taking 
> a cause to SocketException and then update uses of initiCause on creating 
> SocketException to instead pass the cause via the constructor.
> 
> Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282688

Looks fine, would be worth including a couple of tests for coverage

-

Marked as reviewed by lancea (Reviewer).

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


Re: [Internet]Re: [External] : Re: Recent SSLSocket close() @apiNote Changes.

2022-03-05 Thread xueleifan(XueleiFan)
> http://cr.openjdk.java.net/~wetmore/8282529/webrev.00
I was wondering if the mixing of half-close and duplex-close could work as 
well, by adjusting the implementation code.  It could be easier for developers. 
 But your spec update looks good to me, even if we allow the three-lines 
closure.

Thanks,
Xuelei




On Mar 4, 2022, at 4:46 PM, Bradford Wetmore 
mailto:bradford.wetm...@oracle.com>> wrote:


On 3/2/2022 11:46 PM, xueleifan(XueleiFan) wrote:

I think you are right that this design is actually for TLSv1.3 half-close mode. 
 For TLS 1.3,  there is no duplex closure design.  The close() implementation 
in JDK is actually a workaround for compatibility.  Application can use either 
the half-close mode
  socket.shutdownOutput();
  socket.shutdownInput();
or duplex close mode for compatibilty:
  socket.close();
Unfortunately, in practice the half-close and duplex close are mixed with three 
lines:
  socket.shutdownOutput();
  socket.shutdownInput();
  socket.close();

Yeah, I've seen that in things like Apache HttpClient Core, which has 
subsequently been removed.

How about something like this:

http://cr.openjdk.java.net/~wetmore/8282529/webrev.00/

Thanks,

Brad



It was not something I expected when I designed the spec for half-close mode.  
It should be helpful if having another iteration for the @apiNote.
Thanks,
Xuelei
On Mar 2, 2022, at 5:14 PM, Bradford Wetmore  
wrote:


Hi Xuelei,

I am working on some close code including the recent PR[1] for:

   8282529: Fix API Note in javadoc for javax.net.ssl.SSLSocket

and ran into a change I hadn't noticed before.

* @apiNote
* When the connection is no longer needed, the client and server
* applications should each close both sides of their respective connection.
* For {@code SSLSocket} objects, for example, an application can call
* {@link Socket#shutdownOutput()} for output stream close and call
* {@link Socket#shutdownInput()} for input stream close.

It used to be that just a single SSLSocket.close() was sufficient to completely 
shutdown the SSLSocket, and under the hood it closed the output/input in the 
right order.

I believe this code still closes everything as before, but the updated @apiNote 
encourages the user to do a three-part shutdown instead.

  socket.shutdownOutput();
  socket.shutdownInput();
  socket.close();// mostly repeats of above.

This approach seems unnecessary unless the user is interested in the TLSv1.3 
half-close mode.

What is the rationale for recommending this way of doing closes in general?  Or 
does this @apiNote need another iteration?

Thanks,

Brad

[1] 
https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/7648__;!!ACWV5N9M2RV99hQ!cHw7i1wGs-eyCPUcrFXtAdFiUZL6aCPUGpGEQ9u56HHSuwew1j6YHapR8sSefEwr7TRXKQ$





Re: RFR: JDK-8282686: Add constructors taking a cause to SocketException

2022-03-05 Thread Xue-Lei Andrew Fan
On Fri, 4 Mar 2022 21:17:01 GMT, Joe Darcy  wrote:

> Please review this small API enhancement to add the usual constructors taking 
> a cause to SocketException and then update uses of initiCause on creating 
> SocketException to instead pass the cause via the constructor.
> 
> Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282688

Looks good to me.  I may file a new RFE and use this constructor in the JSSE 
implementation.

-

Marked as reviewed by xuelei (Reviewer).

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


Re: RFR: 8282632: Cleanup unnecessary calls to Throwable.initCause() in java.security.jgss [v2]

2022-03-05 Thread Sean Mullan
On Sat, 5 Mar 2022 06:49:43 GMT, Andrey Turbanov  wrote:

>> Pass cause exception as constructor parameter is shorter and easier to read.
>
> Andrey Turbanov has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8282632: Cleanup unnecessary calls to Throwable.initCause() in 
> java.security.jgss
>   typo

Marked as reviewed by mullan (Reviewer).

-

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


Re: RFR: JDK-8282686: Add constructors taking a cause to SocketException

2022-03-05 Thread Alan Bateman
On Fri, 4 Mar 2022 21:17:01 GMT, Joe Darcy  wrote:

> Please review this small API enhancement to add the usual constructors taking 
> a cause to SocketException and then update uses of initiCause on creating 
> SocketException to instead pass the cause via the constructor.
> 
> Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282688

This looks good, I just wonder if we should add a test for the new 
constructors. One of them will be tested by the usages in the JDK, the 
cause-only constructor may not be exercised by any code.

-

Marked as reviewed by alanb (Reviewer).

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