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.shipi...@gmail.com]
Sent: Thursday, August 11, 2016 10:10 PM
To: Ramanand Patil; OpenJDK Network Dev list
Cc: core-libs-...@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
