[9] RFR:JDK-8143305:sun/security/ec/TestEC.java fails intermittently

Sean Mullan sean.mullan at oracle.com
Fri Jun 3 12:54:41 UTC 2016


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
>



More information about the security-dev mailing list