Re: RFR JDK-8049432: Need new tests for testing TLS property jdk.tls.client.protocols with various protocols and values

[Xuelei Fan]

Looks fine to me.  Only a few minor comments:

1.
 201         String[] expecteSupported_protos = new String[] {
 202                 "SSLv2Hello", "SSLv3", "TLSv1", "TLSv1.1", "TLSv1.2"
 203         };

This variable can be define as a static class variable:
     public class TLSClientPropertyTest {
         private String[] expecteSupportedProtos = new String[] {
             "SSLv2Hello", "SSLv3", "TLSv1", "TLSv1.1", "TLSv1.2"
         };
     }

Per Java coding conversions, better to use "theVariableName" style,
rather than link with "-" or "_" (expecteSupported_protos vs
expecteSupportedProtos).

2.
  60         String protocol;

I think, in the test, this is used as SSLContext protocol.  It would be
nice if the variable name is instinctive.  I may suggest to use
"contextProtocol".

The same for the "expectedProto" parameter name.

3.
  67     expectedDefaultProtos = new String[] {
  68         "TLSv1.1", "SSLv3", "TLSv1", "TLSv1.1", "TLSv1.2"
  69     };

Is it intend to have duplicated "TLSv1.1"?  It's OK to me, but better to
add a comment about why duplicated protocols are listed here.

4.
Do you want to test the "SSL" and "TLS" protocol of SSLContext ()?

Thanks,
Xuelei

On 12/3/2014 2:15 PM, zaiyao liu wrote:
> Hi Xuelei,
> 
> Can you help to review this code change?
> 
> Thanks and Regards.
> 
> Kevin
> 于 2014/9/10 10:29, zaiyao liu 写道:
>> Hi All,
>>
>> Please review the code change,the purpose of this fix is to address
>> following:
>> -Sets the property jdk.tls.client.protocols to one of this
>> protocols:SSLv3,TLSv1,TLSv1,TLSv1.1,TLSv1.2 and TLSV(invalid) or
>> removes this
>>  property (if any),then validates the default, supported and current
>> protocols in the SSLContext.
>>
>> Webrev: http://cr.openjdk.java.net/~tyan/kevin/JDK-8049432/webrev01/
>> <http://cr.openjdk.java.net/%7Etyan/kevin/JDK-8049432/webrev01/>
>> Bug:     https://bugs.openjdk.java.net/browse/JDK-8049432
>>
>> Thanks
>>
>> Kevin 
>