RFR[15] JDK-8183369 : RFC unconformity of HttpURLConnection with proxy
Hello All, Could you please review this patch. This patch fixes the RFC unconformity of HttpURLConnection with proxy issue. 1.Change in HttpURLConnection.java is to make sure we do retry with proxy as mentioned in RFC 2.Change is test case HttpURLConWithProxy.java is to make sure we don't throw an error "Can't use direct connection" even when the connection is made through proxy. Webrev: http://cr.openjdk.java.net/~coffeys/webrev.8183369/index.html Issue: https://bugs.openjdk.java.net/browse/JDK-8183369 Thanks, Ravi
RE: RFR[15] JDK-8183369 : RFC unconformity of HttpURLConnection with proxy
Hi, Please do review the below webrev. Thanks, Ravi From: Ravi Reddy Sent: Wednesday, February 5, 2020 6:13 PM To: net-dev@openjdk.java.net Subject: RFR[15] JDK-8183369 : RFC unconformity of HttpURLConnection with proxy Hello All, Could you please review this patch. This patch fixes the RFC unconformity of HttpURLConnection with proxy issue. 1.Change in HttpURLConnection.java is to make sure we do retry with proxy as mentioned in RFC 2.Change is test case HttpURLConWithProxy.java is to make sure we don't throw an error "Can't use direct connection" even when the connection is made through proxy. Webrev: http://cr.openjdk.java.net/~coffeys/webrev.8183369/index.html Issue: https://bugs.openjdk.java.net/browse/JDK-8183369 Thanks, Ravi
RE: RFR[15] JDK-8183369 : RFC unconformity of HttpURLConnection with proxy
Hi Chris, From: Chris Hegarty Sent: Thursday, February 13, 2020 3:07 PM To: Ravi Reddy Cc: net-dev@openjdk.java.net Subject: Re: RFR[15] JDK-8183369 : RFC unconformity of HttpURLConnection with proxy Hi Ravi, On 5 Feb 2020, at 12:43, Ravi Reddy mailto:ravi.k.re...@oracle.com"ravi.k.re...@oracle.com> wrote: Hello All, Could you please review this patch. This patch fixes the RFC unconformity of HttpURLConnection with proxy issue. 1.Change in HttpURLConnection.java is to make sure we do retry with proxy as mentioned in RFC 2.Change is test case HttpURLConWithProxy.java is to make sure we don’t throw an error “Can’t use direct connection” even when the connection is made through proxy. Webrev: http://cr.openjdk.java.net/~coffeys/webrev.8183369/index.html Issue: https://bugs.openjdk.java.net/browse/JDK-8183369 To better understand the issue and the behavior of the HTTP protocol handler I had to dig into the history. Back in JDK 9, the changes for 8161016 [1] altered the behavior of the failed-proxy-connect case to not retry with a direct connection, but to instead throw the IOException that occurred during the failed connection attempt. What is being proposed now is to reinstate the retry, but instead of retrying with a direct connection, retry with the same proxy as previously just failed. Right? Yes Chris , we are now going to do a retry with the proxy , to make sure that we are following RFC which states “idempotent methods do retry”. This effectively amounts to the following changes ( over that of what is in JDK 8 ): --- http = getNewHttpClient(url, null, connectTimeout, false); +++ http = getNewHttpClient(url, p, connectTimeout, false); I notice that the `break` from the original code has not been reintroduced. I don't think that it is strictly needed, but did you give it any consideration? Chris , in original code since we were doing retry with direct connection and not proceeding further with proxies by adding ‘break;’ , whereas now we are retrying with the proxy and if the connection fails again , we have to make sure we do try with other proxies , hence I haven’t reintroduced ‘break’ . [1] 8161016: Strange behavior of URLConnection with proxy https://hg.openjdk.java.net/jdk/jdk/rev/9b5eee5d7a26 Ravi
RE: RFR[15] JDK-8183369 : RFC unconformity of HttpURLConnection with proxy
Hi Daniel/Vyom, As mentioned in the review comments , Change in test case HttpURLConWithProxy.java is to make sure we don’t throw an error “Can’t use direct connection” even when the connection is made through proxy. i.e to make sure the existing test case won't fail because of the fix. Daniel , As you suggested I will try and come up with a test case to make sure there is a retry happening after connection fails once through proxy. Thanks, Ravi -Original Message- From: Daniel Fuchs Sent: Thursday, February 13, 2020 8:59 PM To: Vyom Tiwari Cc: Ravi Reddy ; Chris Hegarty ; net-dev Subject: Re: RFR[15] JDK-8183369 : RFC unconformity of HttpURLConnection with proxy Thanks Vyom. I was suspecting as much. Ravi, could you come up with a test that fails without the fix and pass with it? best regards, -- daniel On 13/02/2020 14:26, Vyom Tiwari wrote: > Hi Ravi/Daniel, > > At my local env(REL 7) test is passing without fix as well. Although > my local repo contain some additional code changes but it is not > related with the current fix. > > Test1 Passed with: Connect timed out > Test2 Passed with: Connect timed out > ## > > Please change copyright year(2020) as well. > > Thanks, > Vyom
RE: RFR[15] JDK-8183369 : RFC unconformity of HttpURLConnection with proxy
Hi Daniel, As suggested by you I have added a test case for retry connection with proxy case . Please review the latest webrev and let me know your comments. With the latest changes , the HttpURLConWithProxy test fails without the fix. Webrev: http://cr.openjdk.java.net/~pkoppula/8183369/webrev.00/ Issue: https://bugs.openjdk.java.net/browse/JDK-8183369 Thanks, Ravi -Original Message- From: Ravi Reddy Sent: Thursday, February 13, 2020 10:10 PM To: Daniel Fuchs ; Vyom Tiwari Cc: net-dev Subject: RE: RFR[15] JDK-8183369 : RFC unconformity of HttpURLConnection with proxy Hi Daniel/Vyom, As mentioned in the review comments , Change in test case HttpURLConWithProxy.java is to make sure we don’t throw an error “Can’t use direct connection” even when the connection is made through proxy. i.e to make sure the existing test case won't fail because of the fix. Daniel , As you suggested I will try and come up with a test case to make sure there is a retry happening after connection fails once through proxy. Thanks, Ravi -Original Message- From: Daniel Fuchs Sent: Thursday, February 13, 2020 8:59 PM To: Vyom Tiwari Cc: Ravi Reddy ; Chris Hegarty ; net-dev Subject: Re: RFR[15] JDK-8183369 : RFC unconformity of HttpURLConnection with proxy Thanks Vyom. I was suspecting as much. Ravi, could you come up with a test that fails without the fix and pass with it? best regards, -- daniel On 13/02/2020 14:26, Vyom Tiwari wrote: > Hi Ravi/Daniel, > > At my local env(REL 7) test is passing without fix as well. Although > my local repo contain some additional code changes but it is not > related with the current fix. > > Test1 Passed with: Connect timed out > Test2 Passed with: Connect timed out > ## > > Please change copyright year(2020) as well. > > Thanks, > Vyom
