[9] RFR: 8168969: Merge SSLSocketSample and SSLSocketTemplate

Artem Smotrakov artem.smotrakov at oracle.com
Thu Nov 3 00:48:12 UTC 2016


Hi John,

On 11/02/2016 05:36 PM, John Jiang wrote:
> Correct, AnonCipherWithWantClientAuth can work fine without extending 
> SSLSocketTemplate.
>>
>> I make those classes to extend SSLSocketTemplate to make lines 
>> shorter (we keep lines shorter than 80 symbols).
> In practice, you could divide one line to two lines if the line length 
> is greater than 80.
Correct. Extending SSLSocketTemplate allows not to mention class name 
when you call a static method which makes lines shorter.
>
>>>
>>> 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.
>> I think it may be better to keep them static which also means 
>> stateless here. It would be easier to re-use in my opinion.
>>>
>>> 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).
>> I see your point, and agree with that. I would prefer to add this 
>> extension when we update a test which really needs it. If you have 
>> such a bug, please feel free to do that.
> The test AnonCipherWithWantClientAuth.java in your patch looks need 
> this extension.
This update is about merging files in javax/net/ssl/template to avoid 
confusions Brad mentioned recently.

I would prefer to implement other enhancements separately. This may also 
eliminate some confusions.
>
>>
>> We have more than one hundred SSL/TLS tests, and they may need some 
>> other extensions. I am not sure that it's possible to provide a 
>> universal SSLSocketTemplate which fits all test's needs.
> I see. It's unnecessary to design too much before getting more 
> concrete requirements.
Agree. Please see above.

Artem
>
>
> Best regards,
> John Jiang
>
>>
>> Artem
>>> 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