RFR (10): 8182589 TLS SNI in new Java 9 client is not available

[Daniel Fuchs]

On 27/06/2017 17:06, Michael McMahon wrote:
> Thanks Daniel. The serverName comes from the InetSocketAddress for the 
> destination.
> It can't have a null field. Though I am wondering now if in the case of 
> hardcoded IP address
> destinations, that we shouldn't set the SNI hostname.

In that case I wonder whether these two statements should be
conditional to serverName != null?

  140         SNIHostName sn = new SNIHostName(serverName);
  141         sslParameters.setServerNames(List.of(sn));

(IIRC there are 2 files)

And if serverName can't be null - maybe a comment or an
assert or an Objects.requireNonNull could make that clear.

best regards,

-- daniel

> 
> - Michael
> 
> On 27/06/2017, 15:44, Daniel Fuchs wrote:
>> Hi Michael,
>>
>> Looks good. I have imported your patch in my local
>> workspace and played with it a bit.
>>
>> AsyncSSLDelegate.java:
>>  128         this.serverName = sname;
>> and SSLDelegate.java
>>  63         this.serverName = sn;
>>  70         serverName = sn;
>>
>> Should this fail if sname/sn is null? Or is null a valid value?
>>
>> HttpRequestImpl.java
>>
>>  61         this.method = builder.method() == null? "GET" : 
>> builder.method();
>>
>> just a nit, but I think it might be better to use a local
>> variable (here and at line 81) to avoid calling method()
>> twice - and avoid having to wonder what will happens
>> if the second call returns a different value.
>>
>> best regards,
>>
>> -- daniel
>>
>> On 27/06/2017 12:17, Michael McMahon wrote:
>>> Just to clarify, there are two issues in 8181422. This fixes one of them
>>> but not the other. So, I will be leaving 8181422 open, and will just 
>>> edit out
>>> the part that is fixed (the NPE from not calling GET() )
>>>
>>> - Michael.
>>>
>>> On 27/06/2017, 10:26, Michael McMahon wrote:
>>>> Hi,
>>>>
>>>> Could I get the following change reviewed please?
>>>>
>>>> http://cr.openjdk.java.net/~michaelm/8182589/webrev.1/
>>>>
>>>> This also fixes 8181422.
>>>>
>>>> Thanks,
>>>> Michael
>>