A small nit, but the debug field should be private (and final for good
measure). I know this is just a test and it doesn't expose a security
issue, but I think it is good practice to adhere to secure programming
conventions even when writing tests. This field is only used within the
test, so it should be private.
There is no need to post another review after making this change, please
go ahead and push the fix.
Thanks,
Sean
On 06/03/2016 03:52 AM, Tim Du wrote:
Thanks Sean and Xuelei
I filed new bug for adding debug option,see
https://bugs.openjdk.java.net/browse/JDK-8158620
The codes were also updated follow Sean's suggestion at here:
http://cr.openjdk.java.net/~tidu/8158620/webrev.00/
Please help to review again.Thanks.
Regards
Tim
On 2016/6/3 4:38, Sean Mullan wrote:
On 06/02/2016 06:16 AM, Tim Du wrote:
Hi,
Please help to review the path for JDK-8143305
TestEC.java test for ECC algorithms in SSL/TLS with different cipher
suites. It failed with javax.net.ssl.SSLException: Unsupported or
unrecognized SSL message intermittently.
The exception was threw at step of handshaking, I was not able to
reproduce this failures. Suggesting to enable the debug option,so that
it could print more information to logs. Hope get helpful information
and do future evaluation in next failures.
I think a new bug should be opened to add the debug option, since the
intermittent failure has not been resolved.
JBS: https://bugs.openjdk.java.net/browse/JDK-8143305
webrev: http://cr.openjdk.java.net/~tidu/8143305/webrev.01/
Can you add braces around the if statement on line 74-5? This is
preferred style to accidentally avoid bracing mistakes. See also
http://cr.openjdk.java.net/~alundblad/styleguide/index-v6.html#toc-braces
--Sean
