On Wed, 25 Feb 2015, laser peter via curl-library wrote:

I see the bug#1479 timeout issue over proxy still open

Okay, lets do this.

Here's a shot at fixing this problem. Please have a go at this and tell me how it behaves for you. If you have a good way to reproduce problems after this patch is applied I'm very interested!

--

 / daniel.haxx.se
From 8c0289a6b416174153bad066704b9b06f08c7f38 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <[email protected]>
Date: Wed, 11 Feb 2015 23:18:32 +0100
Subject: [PATCH] multi: fix *getsock() with CONNECT

The code used some happy eyeballs logic even _after_ CONNECT has been
sent to a proxy, while the happy eyeball phase is already (should be)
over by then.

This is solved by splitting the multi state into two separate states
introducing the new SENDPROTOCONNECT state.

Bug: http://curl.haxx.se/mail/lib-2015-01/0170.html
Reported-by: Peter Laser
---
 lib/http_proxy.c  |  3 +-
 lib/multi.c       | 90 +++++++++++++++++++++++++++----------------------------
 lib/multihandle.h | 31 +++++++++----------
 3 files changed, 62 insertions(+), 62 deletions(-)

diff --git a/lib/http_proxy.c b/lib/http_proxy.c
index 72123ed..d304b84 100644
--- a/lib/http_proxy.c
+++ b/lib/http_proxy.c
@@ -3,11 +3,11 @@
  *  Project                     ___| | | |  _ \| |
  *                             / __| | | | |_) | |
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2014, Daniel Stenberg, <[email protected]>, et al.
+ * Copyright (C) 1998 - 2015, Daniel Stenberg, <[email protected]>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
  * are also available at http://curl.haxx.se/docs/copyright.html.
  *
@@ -73,10 +73,11 @@ CURLcode Curl_proxy_connect(struct connectdata *conn)
     result = Curl_proxyCONNECT(conn, FIRSTSOCKET,
                                conn->host.name, conn->remote_port);
     conn->data->req.protop = prot_save;
     if(CURLE_OK != result)
       return result;
+    Curl_safefree(conn->allocptr.proxyuserpwd);
 #else
     return CURLE_NOT_BUILT_IN;
 #endif
   }
   /* no HTTP tunnel proxy, just return */
diff --git a/lib/multi.c b/lib/multi.c
index 97c9e65..730fc75 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -87,10 +87,11 @@ static const char * const statename[]={
   "CONNECT_PEND",
   "CONNECT",
   "WAITRESOLVE",
   "WAITCONNECT",
   "WAITPROXYCONNECT",
+  "SENDPROTOCONNECT",
   "PROTOCONNECT",
   "WAITDO",
   "DO",
   "DOING",
   "DO_MORE",
@@ -647,18 +648,28 @@ static int waitconnect_getsock(struct connectdata *conn,
       sock[s] = conn->tempsock[i];
       rc |= GETSOCK_WRITESOCK(s++);
     }
   }
 
+  return rc;
+}
+
+static int waitproxyconnect_getsock(struct connectdata *conn,
+                                    curl_socket_t *sock,
+                                    int numsocks)
+{
+  if(!numsocks)
+    return GETSOCK_BLANK;
+
+  sock[0] = conn->sock[FIRSTSOCKET];
+
   /* when we've sent a CONNECT to a proxy, we should rather wait for the
      socket to become readable to be able to get the response headers */
-  if(conn->tunnel_state[FIRSTSOCKET] == TUNNEL_CONNECT) {
-    sock[0] = conn->sock[FIRSTSOCKET];
-    rc = GETSOCK_READSOCK(0);
-  }
+  if(conn->tunnel_state[FIRSTSOCKET] == TUNNEL_CONNECT)
+    return GETSOCK_READSOCK(0);
 
-  return rc;
+  return GETSOCK_WRITESOCK(0);
 }
 
 static int domore_getsock(struct connectdata *conn,
                           curl_socket_t *socks,
                           int numsocks)
@@ -707,17 +718,20 @@ static int multi_getsock(struct SessionHandle *data,
 
   case CURLM_STATE_WAITRESOLVE:
     return Curl_resolver_getsock(data->easy_conn, socks, numsocks);
 
   case CURLM_STATE_PROTOCONNECT:
+  case CURLM_STATE_SENDPROTOCONNECT:
     return Curl_protocol_getsock(data->easy_conn, socks, numsocks);
 
   case CURLM_STATE_DO:
   case CURLM_STATE_DOING:
     return Curl_doing_getsock(data->easy_conn, socks, numsocks);
 
   case CURLM_STATE_WAITPROXYCONNECT:
+    return waitproxyconnect_getsock(data->easy_conn, socks, numsocks);
+
   case CURLM_STATE_WAITCONNECT:
     return waitconnect_getsock(data->easy_conn, socks, numsocks);
 
   case CURLM_STATE_DO_MORE:
     return domore_getsock(data->easy_conn, socks, numsocks);
@@ -1164,74 +1178,58 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
 #ifndef CURL_DISABLE_HTTP
     case CURLM_STATE_WAITPROXYCONNECT:
       /* this is HTTP-specific, but sending CONNECT to a proxy is HTTP... */
       result = Curl_http_connect(data->easy_conn, &protocol_connect);
 
+      rc = CURLM_CALL_MULTI_PERFORM;
       if(data->easy_conn->bits.proxy_connect_closed) {
         /* connect back to proxy again */
         result = CURLE_OK;
-        rc = CURLM_CALL_MULTI_PERFORM;
         multistate(data, CURLM_STATE_CONNECT);
       }
       else if(!result) {
         if(data->easy_conn->tunnel_state[FIRSTSOCKET] == TUNNEL_COMPLETE)
-          multistate(data, CURLM_STATE_WAITCONNECT);
+          /* initiate protocol connect phase */
+          multistate(data, CURLM_STATE_SENDPROTOCONNECT);
       }
       break;
 #endif
 
     case CURLM_STATE_WAITCONNECT:
-      /* awaiting a completion of an asynch connect */
-      result = Curl_is_connected(data->easy_conn,
-                                 FIRSTSOCKET,
-                                 &connected);
-      if(connected) {
-
-        if(!result)
-          /* if everything is still fine we do the protocol-specific connect
-             setup */
-          result = Curl_protocol_connect(data->easy_conn,
-                                         &protocol_connect);
-      }
-
-      if(data->easy_conn->bits.proxy_connect_closed) {
-        /* connect back to proxy again since it was closed in a proxy CONNECT
-           setup */
-        result = CURLE_OK;
+      /* awaiting a completion of an asynch TCP connect */
+      result = Curl_is_connected(data->easy_conn, FIRSTSOCKET, &connected);
+      if(connected && !result) {
         rc = CURLM_CALL_MULTI_PERFORM;
-        multistate(data, CURLM_STATE_CONNECT);
-        break;
+        multistate(data, data->easy_conn->bits.tunnel_proxy?
+                   CURLM_STATE_WAITPROXYCONNECT:
+                   CURLM_STATE_SENDPROTOCONNECT);
       }
       else if(result) {
         /* failure detected */
         /* Just break, the cleaning up is handled all in one place */
         disconnect_conn = TRUE;
         break;
       }
+      break;
 
-      if(connected) {
-        if(!protocol_connect) {
-          /* We have a TCP connection, but 'protocol_connect' may be false
-             and then we continue to 'STATE_PROTOCONNECT'. If protocol
-             connect is TRUE, we move on to STATE_DO.
-             BUT if we are using a proxy we must change to WAITPROXYCONNECT
-          */
-#ifndef CURL_DISABLE_HTTP
-          if(data->easy_conn->tunnel_state[FIRSTSOCKET] == TUNNEL_CONNECT)
-            multistate(data, CURLM_STATE_WAITPROXYCONNECT);
-          else
-#endif
-            multistate(data, CURLM_STATE_PROTOCONNECT);
-
-        }
-        else
-          /* after the connect has completed, go WAITDO or DO */
-          multistate(data, multi->pipelining_enabled?
-                     CURLM_STATE_WAITDO:CURLM_STATE_DO);
-
+    case CURLM_STATE_SENDPROTOCONNECT:
+      result = Curl_protocol_connect(data->easy_conn, &protocol_connect);
+      if(!protocol_connect)
+        /* switch to waiting state */
+        multistate(data, CURLM_STATE_PROTOCONNECT);
+      else if(!result) {
+        /* protocol connect has completed, go WAITDO or DO */
+        multistate(data, multi->pipelining_enabled?
+                   CURLM_STATE_WAITDO:CURLM_STATE_DO);
         rc = CURLM_CALL_MULTI_PERFORM;
       }
+      else if(result) {
+        /* failure detected */
+        Curl_posttransfer(data);
+        Curl_done(&data->easy_conn, result, TRUE);
+        disconnect_conn = TRUE;
+      }
       break;
 
     case CURLM_STATE_PROTOCONNECT:
       /* protocol-specific connect phase */
       result = Curl_protocol_connecting(data->easy_conn, &protocol_connect);
diff --git a/lib/multihandle.h b/lib/multihandle.h
index 1a4b1d9..d8b9d88 100644
--- a/lib/multihandle.h
+++ b/lib/multihandle.h
@@ -5,11 +5,11 @@
  *  Project                     ___| | | |  _ \| |
  *                             / __| | | | |_) | |
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2014, Daniel Stenberg, <[email protected]>, et al.
+ * Copyright (C) 1998 - 2015, Daniel Stenberg, <[email protected]>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
  * are also available at http://curl.haxx.se/docs/copyright.html.
  *
@@ -33,26 +33,27 @@ struct Curl_message {
 typedef enum {
   CURLM_STATE_INIT,         /* 0 - start in this state */
   CURLM_STATE_CONNECT_PEND, /* 1 - no connections, waiting for one */
   CURLM_STATE_CONNECT,      /* 2 - resolve/connect has been sent off */
   CURLM_STATE_WAITRESOLVE,  /* 3 - awaiting the resolve to finalize */
-  CURLM_STATE_WAITCONNECT,  /* 4 - awaiting the connect to finalize */
+  CURLM_STATE_WAITCONNECT,  /* 4 - awaiting the TCP connect to finalize */
   CURLM_STATE_WAITPROXYCONNECT, /* 5 - awaiting proxy CONNECT to finalize */
-  CURLM_STATE_PROTOCONNECT, /* 6 - completing the protocol-specific connect
+  CURLM_STATE_SENDPROTOCONNECT, /* 6 - initiate protocol connect procedure */
+  CURLM_STATE_PROTOCONNECT, /* 7 - completing the protocol-specific connect
                                    phase */
-  CURLM_STATE_WAITDO,       /* 7 - wait for our turn to send the request */
-  CURLM_STATE_DO,           /* 8 - start send off the request (part 1) */
-  CURLM_STATE_DOING,        /* 9 - sending off the request (part 1) */
-  CURLM_STATE_DO_MORE,      /* 10 - send off the request (part 2) */
-  CURLM_STATE_DO_DONE,      /* 11 - done sending off request */
-  CURLM_STATE_WAITPERFORM,  /* 12 - wait for our turn to read the response */
-  CURLM_STATE_PERFORM,      /* 13 - transfer data */
-  CURLM_STATE_TOOFAST,      /* 14 - wait because limit-rate exceeded */
-  CURLM_STATE_DONE,         /* 15 - post data transfer operation */
-  CURLM_STATE_COMPLETED,    /* 16 - operation complete */
-  CURLM_STATE_MSGSENT,      /* 17 - the operation complete message is sent */
-  CURLM_STATE_LAST          /* 18 - not a true state, never use this */
+  CURLM_STATE_WAITDO,       /* 8 - wait for our turn to send the request */
+  CURLM_STATE_DO,           /* 9 - start send off the request (part 1) */
+  CURLM_STATE_DOING,        /* 10 - sending off the request (part 1) */
+  CURLM_STATE_DO_MORE,      /* 11 - send off the request (part 2) */
+  CURLM_STATE_DO_DONE,      /* 12 - done sending off request */
+  CURLM_STATE_WAITPERFORM,  /* 13 - wait for our turn to read the response */
+  CURLM_STATE_PERFORM,      /* 14 - transfer data */
+  CURLM_STATE_TOOFAST,      /* 15 - wait because limit-rate exceeded */
+  CURLM_STATE_DONE,         /* 16 - post data transfer operation */
+  CURLM_STATE_COMPLETED,    /* 17 - operation complete */
+  CURLM_STATE_MSGSENT,      /* 18 - the operation complete message is sent */
+  CURLM_STATE_LAST          /* 19 - not a true state, never use this */
 } CURLMstate;
 
 /* we support N sockets per easy handle. Set the corresponding bit to what
    action we should wait for */
 #define MAX_SOCKSPEREASYHANDLE 5
-- 
2.1.4

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html

Reply via email to