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

[Michael McMahon]

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.

- 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
>