Re: RFR: 8259662: Don't wrap SocketExceptions into SSLExceptions in SSLSocketImpl [v6]
On Thu, 28 Jan 2021 19:04:29 GMT, Xue-Lei Andrew Fan wrote:
>> Clive Verghese has updated the pull request with a new target base due to a
>> merge or a rebase.
>
> src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java line 1702:
>
>> 1700: if (!conContext.isInboundClosed()) {
>> 1701: try {
>> 1702: decode(null);
>
> One decode may be not sufficient if there are multiple pending handshake
> messages record, or the connection has been established and application data
> transportation is in progress. I'm not sure if an additional decode() here
> really solve the problem as described in JDK-8259516.
>
> I did not follow the issue of JDK-8259516. Why an SSLException is desired
> but not SocketException, as if the socket has been closed?
I have removed issues for JDK-8259516.
-
PR: https://git.openjdk.java.net/jdk/pull/2057
Re: RFR: 8259662: Don't wrap SocketExceptions into SSLExceptions in SSLSocketImpl [v6]
On Tue, 26 Jan 2021 18:56:57 GMT, Clive Verghese wrote:
>> Redo for 8237578: JDK-8214339 (SSLSocketImpl wraps SocketException) appears
>> to not be fully fixed
>>
>> This also fixes JDK-8259516: Alerts sent by peer may not be received
>> correctly during TLS handshake
>
> Clive Verghese has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains four commits:
>
> - Update copyright year
> - Add error handling guidelines
> - Fix bugids and use server port passed as a parameter
> - 8259662: Don't wrap SocketExceptions into SSLExceptions in SSLSocketImpl
src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java line 1425:
> 1423: } catch (SocketException se) {
> 1424: // don't change exception in case of SocketException
> 1425: throw se;
I may add the SocketException catch line together within line 1420.
src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java line 1491:
> 1489: } catch (SocketException se) {
> 1490: // don't change exception in case of SocketException
> 1491: throw se;
I may add the SocketException catch line together within line 1489.
src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java line 1702:
> 1700: if (!conContext.isInboundClosed()) {
> 1701: try {
> 1702: decode(null);
One decode may be not sufficient if there are multiple pending handshake
messages record, or the connection has been established and application data
transportation is in progress. I'm not sure if an additional decode() here
really solve the problem as described in JDK-8259516.
I did not follow the issue of JDK-8259516. Why an SSLException is desired but
not SocketException, as if the socket has been closed?
src/java.base/share/classes/sun/security/ssl/SSLTransport.java line 146:
> 144: } catch (SocketException se) {
> 145: // don't change exception in case of SocketException
> 146: throw se;
I may add the SocketException catch line together within line 141.
-
PR: https://git.openjdk.java.net/jdk/pull/2057
Re: RFR: 8259662: Don't wrap SocketExceptions into SSLExceptions in SSLSocketImpl [v6]
On Wed, 27 Jan 2021 17:46:03 GMT, Xue-Lei Andrew Fan wrote: >> Changes requested by rhalade (Reviewer). > > For the CSR request, I updated the component to security-libs/javax.net.ssl, > add 17 as one of the fix versions, and added myself as reviewer. Thank you. I will change the status of the CSR to proposed. - PR: https://git.openjdk.java.net/jdk/pull/2057
Re: RFR: 8259662: Don't wrap SocketExceptions into SSLExceptions in SSLSocketImpl [v6]
On Wed, 13 Jan 2021 18:46:10 GMT, Rajan Halade wrote: >> Clive Verghese has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains four commits: >> >> - Update copyright year >> - Add error handling guidelines >> - Fix bugids and use server port passed as a parameter >> - 8259662: Don't wrap SocketExceptions into SSLExceptions in SSLSocketImpl > > Changes requested by rhalade (Reviewer). For the CSR request, I updated the component to security-libs/javax.net.ssl, add 17 as one of the fix versions, and added myself as reviewer. - PR: https://git.openjdk.java.net/jdk/pull/2057
Re: RFR: 8259662: Don't wrap SocketExceptions into SSLExceptions in SSLSocketImpl [v6]
> Redo for 8237578: JDK-8214339 (SSLSocketImpl wraps SocketException) appears > to not be fully fixed > > This also fixes JDK-8259516: Alerts sent by peer may not be received > correctly during TLS handshake Clive Verghese has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits: - Update copyright year - Add error handling guidelines - Fix bugids and use server port passed as a parameter - 8259662: Don't wrap SocketExceptions into SSLExceptions in SSLSocketImpl - Changes: https://git.openjdk.java.net/jdk/pull/2057/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2057=05 Stats: 501 lines in 7 files changed: 490 ins; 0 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/2057.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2057/head:pull/2057 PR: https://git.openjdk.java.net/jdk/pull/2057
