RFR: 8237578: JDK-8214339 (SSLSocketImpl wraps SocketException) appears to not be fully fixed [v4]

Clive Verghese cverghese at openjdk.java.net
Sun Jan 10 06:56:19 UTC 2021


On Sat, 9 Jan 2021 05:25:40 GMT, Xue-Lei Andrew Fan <xuelei at openjdk.org> wrote:

>> Clive Verghese has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR.
>
> test/jdk/sun/security/ssl/SSLSocketImpl/SSLSocketShouldThrowSocketException.java line 59:
> 
>> 57:     static String keyStoreFile = "keystore";
>> 58:     static String trustStoreFile = "truststore";
>> 59:     static String passwd = "passphrase";
> 
> In JSSE testing, we are trying to avoid the dependency on the binary key store files for a while.  Would you like to check out the new template, test/jdk/javax/net/ssl/templates/SSLSocketTemplate.java?  You could refer to test/jdk/sun/security/ssl/ServerHandshaker/AnonCipherWithWantClientAuth.java, or search for "extends SSLSocketTemplate" about how to use the new template.

I have updated the test to use SSLSocketTemplate.

> src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java line 1703:
> 
>> 1701:         }
>> 1702: 
>> 1703:         throw conContext.fatal(alert, cause);
> 
> It might be not necessary to change the TransportContext by adding a new teardownTransport() method. It would be good to keep the fatal() behavior as if a fatal alter will be sent.  Maybe, the exception thrown by fatal() could be replaced with the socket exception, like:
> 
> if (cause instanceof SocketException) {
>      try {
>         conContext.fatal(alert, cause);
>      } catch (Exception) {
>         // Just delivering the fatal alert, re-throw the socket exception instead.
>      } finally {
>         throw (SocketException)cause;
>      }
> } else {
>       throw conContext.fatal(alert, cause);
> }

Thank you for the feedback, I have updated as recommended. I could not add a throw in the finally block as this generates a warning. Instead i have done this

            try {
                conContext.fatal(alert, cause);
            } catch (Exception e) {
                // Just delivering the fatal alert, re-throw the socket exception instead.
            }

            throw (SocketException)cause;

-------------

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



More information about the security-dev mailing list