Commit a4cece3d authored by Daniel Stenberg's avatar Daniel Stenberg
Browse files

CONNECT: Revert Curl_proxyCONNECT back to 7.29.0 design

This reverts commit cb3e6dfa 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
parent d242839a
Loading
Loading
Loading
Loading
+30 −17
Original line number Diff line number Diff line
@@ -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,8 +563,15 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
    /* to back to init state */
    conn->tunnel_state[sockindex] = TUNNEL_INIT;

    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;

+10 −6
Original line number Diff line number Diff line
@@ -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;