RFR: 8161016: Strange behavior of URLConnection with proxy

[Chris Hegarty]

On 12/08/16 20:56, Ramanand Patil wrote:
> 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/

This looks good to me.

-Chris.


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