Re: RFR 8144008: Setting NO_PROXY on an URLConnection is not complied with

[Vyom Tewari]

Hi Chris,

I am fine with suggested change( return new 
java.net.Socket(Proxy.NO_PROXY)); )as this will improve the code 
readability.

Thanks,
Vyom

On Tuesday 21 June 2016 01:38 PM, Chris Hegarty wrote:
Yom,

Looking at this again, with fresh eyes, before pushing it for you. I think the
change would be clearer if it explicitly used Proxy.NO_PROXY, rather than
relying on the fact that the `proxy` field is initialized to NO_PROXY. Since
the intent here is to ALWAYS make an non-proxied connection.

@@ -192,11 +192,11 @@

       * The following method, createSocket, is provided to allow the
       * https client to override it so that it may use its socket factory
       * to create the socket.
       */
      protected Socket createSocket() throws IOException {

-        return new java.net.Socket();
+        return new java.net.Socket(Proxy.NO_PROXY));

      }
  
      protected InetAddress getLocalAddress() throws IOException {
          if (serverSocket == null)
              throw new IOException("not connected”);

-Chris.

On 16 Jun 2016, at 21:09, Chris Hegarty <chris.hega...@oracle.com> wrote:


On 16 Jun 2016, at 18:55, Roger Riggs <roger.ri...@oracle.com> wrote:

Hi Vyom,

Looks ok,
+1.

I attempted to add a test for https, and just realised that the 
javax.net.SocketFactory
does not support Socket(Proxy).  It may be worth capturing this issue 
somewhere, but
it is way beyond the scope of this bug, and less of an issue with the new HTTP
Client alternative.

-Chris.

Roger

On 6/16/2016 10:35 AM, Vyom Tewari wrote:
Hi All,

Please find the latest 
webrev(http://cr.openjdk.java.net/~vtewari/8144008/webrev0.1/index.html 
<http://cr.openjdk.java.net/%7Evtewari/8144008/webrev0.1/index.html>), i got 
some off line comments from Chris.

Thanks,
Vyom

On Tuesday 14 June 2016 12:11 PM, Vyom Tewari wrote:
Hi All,

Please review the below fix.
Bug           : JDK-8144008 Setting NO_PROXY on an URLConnection is not 
complied with
Webrev     : http://cr.openjdk.java.net/~vtewari/8144008/webrev0.0/index.html 
<http://cr.openjdk.java.net/%7Evtewari/8144008/webrev0.0/index.html>

Thanks,
Vyom