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

[Daniel Fuchs]

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