Hi all,

Recently I encountered an issue about curl library. Sometimes it enters infinite loop if it got connection timeout.
The following is the env/curl version I used:
OS: Wiindows 7 (32bit)
cURL: 7.21.4

Here's the finding:
The following is the code snippet inside curl (@curl-7.21.4/lib/url.c).
It uses a loop to handle connections and there's a retry mechanism for authentication information exchanging inside the loop.

static CURLcode setup_conn(struct connectdata *conn,
                                       bool *protocol_done)
{
       ...
       ...
      for(;;) {
            /* loop for CURL_SERVER_CLOSED_CONNECTION */
            ...
            if(CURL_SOCKET_BAD == conn->sock[FIRSTSOCKET]) {
                  /* Try to connect only if not already connected */
                  bool connected = FALSE;

                  result = ConnectPlease(data, conn, &connected);

                  if(connected) {
                        result = Curl_protocol_connect(conn, protocol_done);
                        if(CURLE_OK == result)
                              conn->bits.tcpconnect = TRUE;
                  }
                  else
                        conn->bits.tcpconnect = FALSE;

                  /* if the connection was closed by the server while exchanging
                  authentication informations, retry with the new set
                  authentication information */
                  if(conn->bits.proxy_connect_closed) {
                        /* reset the error buffer */
                        if(data->set.errorbuffer)
                              data->set.errorbuffer[0] = '\0';
                        data->state.errorbuf = FALSE;
                        continue;
                  }

                  if(CURLE_OK != result)
                        return result;
            }
            else {
                  Curl_pgrsTime(data, TIMER_CONNECT); /* we're connected already */
                  Curl_pgrsTime(data, TIMER_APPCONNECT); /* we're connected already */
                  conn->bits.tcpconnect = TRUE;
                  *protocol_done = TRUE;
                  Curl_verboseconnect(conn);
                  Curl_updateconninfo(conn, conn->sock[FIRSTSOCKET]);
            }
            /* Stop the loop now */
            break;
      }
      ...
      ...
}

If it tries to retry with the new set authentication info, it continues the loop and invokes ConnectPlease().
Inside the  function ConnectPlease(), it invokes Curl_connecthost() @curl-7.21.4/lib/connect.c
It first checks if the operation time exceeds timeout. If it is, it returns the timeout error immediately.
However, in this state when the program returns to setup_conn() @ curl-7.21.4/lib/url.c, it enters an infinite loop since it doesn't check the returned result.

CURLcode Curl_connecthost(struct connectdata *conn,  /* context */
                                      const struct Curl_dns_entry *remotehost,
                                      curl_socket_t *sockconn,   /* the connected socket */
                                      Curl_addrinfo **addr,      /* the one we used */
                                      bool *connected)           /* really connected? */
{
      struct SessionHandle *data = conn->data;
      curl_socket_t sockfd = CURL_SOCKET_BAD;
      int aliasindex;
      Curl_addrinfo *ai;
      Curl_addrinfo *curr_addr;

      struct timeval after;
      struct timeval before = Curl_tvnow();

      /*************************************************************
      * Figure out what maximum time we have left
      *************************************************************/
      long timeout_ms;

      DEBUGASSERT(sockconn);
      *connected = FALSE; /* default to not connected */

      /* get the timeout left */
      timeout_ms = Curl_timeleft(data, &before, TRUE);

      if(timeout_ms < 0) {
            /* a precaution, no need to continue if time already is up */
            failf(data, "Connection time-out");
            return CURLE_OPERATION_TIMEDOUT;
      }
      ...
      ...
}

In my thinking if the connection is timeout, there's no need to continue the operation.
It might be better to break the loop in setup_conn() in such kind of scenario (like the following).

// current code
if(conn->bits.proxy_connect_closed) {
      /* reset the error buffer */
      if(data->set.errorbuffer)
            data->set.errorbuffer[0] = '\0';
      data->state.errorbuf = FALSE;
      continue;
}

// modified code
if(conn->bits.proxy_connect_closed && CURLE_OPERATION_TIMEDOUT != result) {
      /* reset the error buffer */
      if(data->set.errorbuffer)
            data->set.errorbuffer[0] = '\0';
      data->state.errorbuf = FALSE;
      continue;
}

Regards,
Jerry

TREND MICRO EMAIL NOTICE
The information contained in this email and any attachments is confidential and may be subject to copyright or other intellectual property protection. If you are not the intended recipient, you are not authorized to use or disclose this information, and we request that you notify us by reply mail or telephone and delete the original message from your mail system.

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2011-08-31