CONNECT: Revert Curl_proxyCONNECT back to 7.29.0 design

This reverts commit cb3e6dfa3511 and instead fixes the problem
differently.

The reverted commit addressed a test failure in test 1021 by simplifying
and generalizing the code flow in a way that damaged the
performance. Now we modify the flow so that Curl_proxyCONNECT() again
does as much as possible in one go, yet still do test 1021 with and
without valgrind. It failed due to mistakes in the multi state machine.

Bug: http://curl.haxx.se/bug/view.cgi?id=1397
Reported-by: Paul Saab

diff --git a/lib/http_proxy.c b/lib/http_proxy.c
index a98c68c1cbeca6c28d220f509a79b7503d500293..17f1c00a1d704267828f7fbbf876d09a8fc15fe7 100644 (file)
--- a/lib/http_proxy.c
+++ b/lib/http_proxy.c
@@ -98,8 +98,6 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
   struct SessionHandle *data=conn->data;
   struct SingleRequest *k = &data->req;
   CURLcode result;
-  long timeout =
-    data->set.timeout?data->set.timeout:PROXY_TIMEOUT; /* in milliseconds */
   curl_socket_t tunnelsocket = conn->sock[sockindex];
   curl_off_t cl=0;
   bool closeConnection = FALSE;
@@ -223,14 +221,25 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
         return result;
 
       conn->tunnel_state[sockindex] = TUNNEL_CONNECT;
+    } /* END CONNECT PHASE */
+
+    check = Curl_timeleft(data, NULL, TRUE);
+    if(check <= 0) {
+      failf(data, "Proxy CONNECT aborted due to timeout");
+      return CURLE_RECV_ERROR;
+    }
 
-      /* now we've issued the CONNECT and we're waiting to hear back, return
-         and get called again polling-style */
+    if(0 == Curl_socket_ready(tunnelsocket, CURL_SOCKET_BAD, 0))
+      /* return so we'll be called again polling-style */
       return CURLE_OK;
+    else {
+      DEBUGF(infof(data,
+                   "Read response immediately from proxy CONNECT\n"));
+    }
 
-    } /* END CONNECT PHASE */
+    /* at this point, the tunnel_connecting phase is over. */
 
-    { /* BEGIN NEGOTIATION PHASE */
+    { /* READING RESPONSE PHASE */
       size_t nread;   /* total size read */
       int perline; /* count bytes per line */
       int keepon=TRUE;
@@ -247,9 +256,7 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
 
       while((nread<BUFSIZE) && (keepon && !error)) {
 
-        /* if timeout is requested, find out how much remaining time we have */
-        check = timeout - /* timeout time */
-          Curl_tvdiff(Curl_tvnow(), conn->now); /* spent time */
+        check = Curl_timeleft(data, NULL, TRUE);
         if(check <= 0) {
           failf(data, "Proxy CONNECT aborted due to timeout");
           error = SELECT_TIMEOUT; /* already too little time */
@@ -279,6 +286,7 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
               /* proxy auth was requested and there was proxy auth available,
                  then deem this as "mere" proxy disconnect */
               conn->bits.proxy_connect_closed = TRUE;
+              infof(data, "Proxy CONNECT connection closed");
             }
             else {
               error = SELECT_ERROR;
@@ -527,7 +535,7 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
         conn->sock[sockindex] = CURL_SOCKET_BAD;
         break;
       }
-    } /* END NEGOTIATION PHASE */
+    } /* END READING RESPONSE PHASE */
 
     /* If we are supposed to continue and request a new URL, which basically
      * means the HTTP authentication is still going on so if the tunnel
@@ -542,13 +550,11 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
   } while(data->req.newurl);
 
   if(200 != data->req.httpcode) {
-    failf(data, "Received HTTP code %d from proxy after CONNECT",
-          data->req.httpcode);
-
-    if(closeConnection && data->req.newurl)
+    if(closeConnection && data->req.newurl) {
       conn->bits.proxy_connect_closed = TRUE;
-
-    if(data->req.newurl) {
+      infof(data, "Connect me again please\n");
+    }
+    else if(data->req.newurl) {
       /* this won't be used anymore for the CONNECT so free it now */
       free(data->req.newurl);
       data->req.newurl = NULL;
@@ -557,7 +563,14 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
     /* to back to init state */
     conn->tunnel_state[sockindex] = TUNNEL_INIT;
 
-    return CURLE_RECV_ERROR;
+    if(conn->bits.proxy_connect_closed)
+      /* this is not an error, just part of the connection negotiation */
+      return CURLE_OK;
+    else {
+      failf(data, "Received HTTP code %d from proxy after CONNECT",
+            data->req.httpcode);
+      return CURLE_RECV_ERROR;
+    }
   }
 
   conn->tunnel_state[sockindex] = TUNNEL_COMPLETE;

diff --git a/lib/multi.c b/lib/multi.c
index ca975a05652a83ec7ef24a8235dd8d02d7690806..1e5b3c8df5b51a1d22646e35d650049fbe58583a 100644 (file)
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -1137,11 +1137,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
       data->result = Curl_http_connect(data->easy_conn, &protocol_connect);
 
       if(data->easy_conn->bits.proxy_connect_closed) {
-        /* reset the error buffer */
-        if(data->set.errorbuffer)
-          data->set.errorbuffer[0] = '\0';
-        data->state.errorbuf = FALSE;
-
+        /* connect back to proxy again */
         data->result = CURLE_OK;
         result = CURLM_CALL_MULTI_PERFORM;
         multistate(data, CURLM_STATE_CONNECT);
@@ -1167,7 +1163,15 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
                                                &protocol_connect);
       }
 
-      if(CURLE_OK != data->result) {
+      if(data->easy_conn->bits.proxy_connect_closed) {
+        /* connect back to proxy again since it was closed in a proxy CONNECT
+           setup */
+        data->result = CURLE_OK;
+        result = CURLM_CALL_MULTI_PERFORM;
+        multistate(data, CURLM_STATE_CONNECT);
+        break;
+      }
+      else if(CURLE_OK != data->result) {
         /* failure detected */
         /* Just break, the cleaning up is handled all in one place */
         disconnect_conn = TRUE;