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
