[9] RFR: 8168969: Merge SSLSocketSample and SSLSocketTemplate

John Jiang sha.jiang at oracle.com
Wed Nov 2 06:48:36 UTC 2016


Hi Artem,
Thanks for making the template to be used easily.

The tests in your patch extend class SSLSocketTemplate, but 
SSLSocketTemplate looks like an utility class, but not a parent class.
For example,
test/sun/security/ssl/ServerHandshaker/AnonCipherWithWantClientAuth.java
   67         new SSLSocketTemplate()
AnonCipherWithWantClientAuth still creates a SSLSocketTemplate instance, 
but not itself.
In fact, if replacing "print()" with "SSLSocketTemplate.print()", 
AnonCipherWithWantClientAuth can work fine without extending 
SSLSocketTemplate.

The original SSLSocketSample.java defines the core methods, such as 
doServerSide(), runServerApplication(), doClientSide() and 
runClientApplication(), as non-static. In my eyes, this style may be 
better. The child class can simply override the methods if necessary.

In addition, some tests have to configure SSLServerSocket. Why not 
provide one more extension point in doServerSide()? 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,
>
> 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 SSLTest, 
> and removed SSLSocketSample.
>
> SSL/TLS tests should use SSLSocketTemplate class. I updated test which 
> use SSLTest to use SSLSocketTemplate.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8168969
> Webrev: http://cr.openjdk.java.net/~asmotrak/8168969/webrev.00/
>
> Artem
>




More information about the security-dev mailing list