- Jon Nelson found a regression that turned out to be a flaw in how libcurl

  detects and uses proxies based on the environment variables. If the proxy
  was given as an explicit option it worked, but due to the setup order
  mistake proxies would not be used fine for a few protocols when picked up
  from '[protocol]_proxy'. Obviously this broke after 7.19.4. I now also added
  test case 1106 that verifies this functionality.

  (http://curl.haxx.se/bug/view.cgi?id=2913886)

5 files changed, 103 insertions, 32 deletions

diff --git a/CHANGES b/CHANGES
index 268a7444b..388df88f4 100644
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,16 @@
 
                                   Changelog
 
+Daniel Stenberg (15 Dec 2009)
+- Jon Nelson found a regression that turned out to be a flaw in how libcurl
+  detects and uses proxies based on the environment variables. If the proxy
+  was given as an explicit option it worked, but due to the setup order
+  mistake proxies would not be used fine for a few protocols when picked up
+  from '[protocol]_proxy'. Obviously this broke after 7.19.4. I now also added
+  test case 1106 that verifies this functionality.
+
+  (http://curl.haxx.se/bug/view.cgi?id=2913886)
+
 Daniel Stenberg (12 Dec 2009)
 - IMAP, POP3 and SMTP support and their TLS versions (including IMAPS, POP3S
   and SMTPS) are now supported. The current state may not yet be solid, but
diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index dd6eb8fd2..eeb2cb871 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -34,6 +34,7 @@ This release includes the following bugfixes:
  o Expect: 100-continue handling when set by the application
  o multi interface with OpenSSL read already freed memory when closing down
  o --retry didn't do right for FTP transient errors
+ o some *_proxy environment variables didn't function
 
 This release includes the following known bugs:
 
@@ -45,6 +46,7 @@ advice from friends like these:
  Yang Tse, Kamil Dudka, Christian Schmitz, Constantine Sapuntzakis,
  Marco Maggi, Camille Moncelier, Claes Jakobsson, Kevin Baughman,
  Marc Kleine-Budde, Jad Chamcham, Bjorn Augustsson, David Byron,
- Markus Koetter, Chad Monroe, Martin Storsjo, Siegfried Gyuricsko
+ Markus Koetter, Chad Monroe, Martin Storsjo, Siegfried Gyuricsko,
+ Jon Nelson,
 
         Thanks! (and sorry if I forgot to mention someone)
diff --git a/lib/url.c b/lib/url.c
index 95ca31632..907498db9 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -3242,6 +3242,7 @@ static struct connectdata *allocate_conn(void)
   conn->sock[FIRSTSOCKET] = CURL_SOCKET_BAD;     /* no file descriptor */
   conn->sock[SECONDARYSOCKET] = CURL_SOCKET_BAD; /* no file descriptor */
   conn->connectindex = -1;    /* no index */
+  conn->port = -1; /* unknown at this point */
 
   /* Default protocol-independent behavior doesn't support persistent
      connections, so we set this to force-close. Protocols that support
@@ -3498,7 +3499,8 @@ static CURLcode setup_range(struct SessionHandle *data)
 
 
 /***************************************************************
-* Setup connection internals specific to the requested protocol
+* Setup connection internals specific to the requested protocol.
+* This MUST get called after proxy magic has been figured out.
 ***************************************************************/
 static CURLcode setup_connection_internals(struct SessionHandle *data,
                                            struct connectdata *conn)
@@ -3537,7 +3539,10 @@ static CURLcode setup_connection_internals(struct SessionHandle *data,
         p = conn->handler;              /* May have changed. */
       }
 
-      conn->port = p->defport;
+      if(conn->port < 0)
+        /* we check for -1 here since if proxy was detected already, this
+           was very likely already set to the proxy port */
+        conn->port = p->defport;
       conn->remote_port = (unsigned short)p->defport;
       conn->protocol |= p->protocol;
       return CURLE_OK;
@@ -4314,7 +4319,7 @@ static CURLcode create_conn(struct SessionHandle *data,
   char *proxy = NULL;
 
   *async = FALSE;
-  
+
   /*************************************************************
    * Check input data
    *************************************************************/
@@ -4444,23 +4449,6 @@ static CURLcode create_conn(struct SessionHandle *data,
     conn->protocol &= ~PROT_MISSING; /* switch that one off again */
   }
 
-  /*************************************************************
-   * Setup internals depending on protocol
-   *************************************************************/
-  result = setup_connection_internals(data, conn);
-  if(result != CURLE_OK) {
-    Curl_safefree(proxy);
-    return result;
-  }
-
-  /*************************************************************
-   * Parse a user name and password in the URL and strip it out
-   * of the host name
-   *************************************************************/
-  result = parse_url_userpass(data, conn, user, passwd);
-  if(result != CURLE_OK)
-    return result;
-
 #ifndef CURL_DISABLE_PROXY
   /*************************************************************
    * Extract the user and password from the authentication string
@@ -4483,7 +4471,6 @@ static CURLcode create_conn(struct SessionHandle *data,
     }
   }
 
-
   if(data->set.str[STRING_NOPROXY] &&
      check_noproxy(conn->host.name, data->set.str[STRING_NOPROXY])) {
     if(proxy) {
@@ -4511,15 +4498,13 @@ static CURLcode create_conn(struct SessionHandle *data,
     conn->bits.proxy = TRUE;
   }
   else {
-      /* we aren't using the proxy after all... */
-      conn->bits.proxy = FALSE;
-      conn->bits.httpproxy = FALSE;
-      conn->bits.proxy_user_passwd = FALSE;
-      conn->bits.tunnel_proxy = FALSE;
+    /* we aren't using the proxy after all... */
+    conn->bits.proxy = FALSE;
+    conn->bits.httpproxy = FALSE;
+    conn->bits.proxy_user_passwd = FALSE;
+    conn->bits.tunnel_proxy = FALSE;
   }
-#endif /* CURL_DISABLE_PROXY */
 
-#ifndef CURL_DISABLE_PROXY
   /***********************************************************************
    * If this is supposed to use a proxy, we need to figure out the proxy
    * host name, so that we can re-use an existing connection
@@ -4534,6 +4519,24 @@ static CURLcode create_conn(struct SessionHandle *data,
   }
 #endif /* CURL_DISABLE_PROXY */
 
+  /*************************************************************
+   * Setup internals depending on protocol. Needs to be done after
+   * we figured out what/if proxy to use.
+   *************************************************************/
+  result = setup_connection_internals(data, conn);
+  if(result != CURLE_OK) {
+    Curl_safefree(proxy);
+    return result;
+  }
+
+  /*************************************************************
+   * Parse a user name and password in the URL and strip it out
+   * of the host name
+   *************************************************************/
+  result = parse_url_userpass(data, conn, user, passwd);
+  if(result != CURLE_OK)
+    return result;
+
   /***********************************************************************
    * file: is a special case in that it doesn't need a network connection
    ***********************************************************************/
@@ -4689,7 +4692,7 @@ static CURLcode create_conn(struct SessionHandle *data,
  * create_conn() is all done.
  *
  * setup_conn() also handles reused connections
- * 
+ *
  * conn->data MUST already have been setup fine (in create_conn)
  */
 
@@ -4845,7 +4848,7 @@ CURLcode Curl_async_resolved(struct connectdata *conn,
 
   if(conn->async.dns) {
     conn->dns_entry = conn->async.dns;
-    conn->async.dns = NULL;    
+    conn->async.dns = NULL;
   }
 
   code = setup_conn(conn, protocol_done);
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 7aa5f6dca..ac061a15f 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -63,7 +63,7 @@ EXTRA_DIST = test1 test108 test117 test127 test20 test27 test34 test46	   \
  test1089 test1090 test1091 test1092 test1093 test1094 test1095 test1096   \
  test1097 test560 test561 test1098 test1099 test562 test563 test1100       \
  test564 test1101 test1102 test1103 test1104 test299 test310 test311       \
- test312 test1105 test565 test800
+ test312 test1105 test565 test800 test1106
 
 filecheck:
 	@mkdir test-place; \
diff --git a/tests/data/test1106 b/tests/data/test1106
new file mode 100644
index 000000000..2ac14d145
--- /dev/null
+++ b/tests/data/test1106
@@ -0,0 +1,56 @@
+<testcase>
+<info>
+<keywords>
+FTP
+CURLOPT_PORT
+HTTP proxy
+</keywords>
+</info>
+
+# Server-side
+<reply>
+<data nocheck="yes">
+HTTP/1.1 200 OK swsclose

+Date: Thu, 09 Nov 2010 14:49:00 GMT

+Server: test-server/fake

+Accept-Ranges: bytes

+Content-Length: 6

+

+hello
+</data>
+</reply>
+
+# Client-side
+<client>
+<server>
+http
+</server>
+ <name>
+FTP URL and with ftp_proxy environment variable set
+ </name>
+
+<setenv>
+ftp_proxy=http://%HOSTIP:%HTTPPORT/
+</setenv>
+# note that we need quotes around the URL below to make sure the shell doesn't
+# treat the semicolon as a separator!
+ <command>
+"ftp://%HOSTIP:23456/1106"
+</command>
+
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+<strip>
+^User-Agent:.*
+</strip>
+<protocol>
+GET ftp://%HOSTIP:23456/1106 HTTP/1.1

+Host: %HOSTIP:23456

+Accept: */*

+Proxy-Connection: Keep-Alive

+

+</protocol>
+</verify>
+</testcase>