RFR: 8161016: Strange behavior of URLConnection with proxy

[Ramanand Patil]

Hi Aleksey,

Thank you for your review.
In the exception handler block: when last proxy fails, it was using a DIRECT connection, but in the fixed version it was just a re-try once with the last proxy before failing the connection.

Considering your points I think the alternate approach of not re-trying and just failing after the last proxy is good. I have updated the code slightly which does the same as you suggested.
And also updated the test case to use one more dummy proxy in MyProxySelector class.

Here is the updated Webrev: http://cr.openjdk.java.net/~rpatil/8161016/webrev.01/


Regards,
Ramanand.

-----Original Message-----
From: Aleksey Shipilev [mailto:aleksey.shipilev at gmail.com] 
Sent: Thursday, August 11, 2016 10:10 PM
To: Ramanand Patil; OpenJDK Network Dev list
Cc: core-libs-dev at openjdk.java.net
Subject: Re: RFR: 8161016: Strange behavior of URLConnection with proxy

On 08/11/2016 05:17 PM, Ramanand Patil wrote:
> Webrev: http://cr.openjdk.java.net/~rpatil/8161016/webrev.00/
> 
> Fix: Instead of falling back to direct connection when last proxy 
> fails to open connection, re-try once with the last proxy. An 
> alternate solution can also be- don't try to open any connection when 
> all set proxies fails to open a connection.

I wonder if the code should traverse the last proxy within the loop, not trying to special-case it in the exception handler -- otherwise we would miss logging, exceptions, and ProxySelector notifications coming from the last proxy?

E.g. instead of:

1116 } catch (IOException ioex) {
1117   if (p != Proxy.NO_PROXY) {
1118      sel.connectFailed(uri, p.address(), ioex);
1119      if (!it.hasNext()) {
1120         // re-try once with the last Proxy
1121         http = getNewHttpClient(url, p, connectTimeout, false);
1122         http.setReadTimeout(readTimeout);
1123         break;
1124      }
1125   } else {
1126      throw ioex;
1127   }
1128   continue;
1129 }

...do:

} catch (IOException ioex) {
  if (p == Proxy.NO_PROXY) {
    throw ioex;
  }
  sel.connectFailed(uri, p.address(), ioex);
  if (it.hasNext()) {
     continue;   // try the next Proxy
  } else {
     throw ioex; // that was the last Proxy, time to fail
  }
}

Thanks,
-Aleksey