timeout handling: auto-detect effective timeout

When checking a transfer for being expired via `Curl_timeleft_ms()`,
eleminate the `bool connecting` parameter and have the function check
the `mstate` of the transfer instead.

Advantages:
* eleminate the caller needing awareness if the transfer is
  connecting or in a later state
* fix pingpong timeout handling to check the correct timeout
  during "proto_connect" phases
* avoid using "connecting" timeouts during establishing a secondary
  connection (e.g. FTP) since this would use the timestamp from
  the original, primary connect and thus be wrong

Reported-by: Wyuer on github
Fixes #20347
Closes #20354

lib/asyn-ares.c

@@ -392,7 +392,7 @@ CURLcode Curl_async_await(struct Curl_easy *data,
   DEBUGASSERT(entry);
   *entry = NULL; /* clear on entry */
 
-  timeout_ms = Curl_timeleft_ms(data, TRUE);
+  timeout_ms = Curl_timeleft_ms(data);
   if(timeout_ms < 0) {
     /* already expired! */
     connclose(data->conn, "Timed out before name resolve started");

lib/cf-h1-proxy.c

@@ -571,7 +571,7 @@ static CURLcode H1_CONNECT(struct Curl_cfilter *cf,
 
   do {
 
-    if(Curl_timeleft_ms(data, TRUE) < 0) {
+    if(Curl_timeleft_ms(data) < 0) {
       failf(data, "Proxy CONNECT aborted due to timeout");
       result = CURLE_OPERATION_TIMEDOUT;
       goto out;

lib/cf-h2-proxy.c

@@ -1058,7 +1058,7 @@ static CURLcode cf_h2_proxy_connect(struct Curl_cfilter *cf,
   }
   DEBUGASSERT(ts->authority);
 
-  if(Curl_timeleft_ms(data, TRUE) < 0) {
+  if(Curl_timeleft_ms(data) < 0) {
     failf(data, "Proxy CONNECT aborted due to timeout");
     result = CURLE_OPERATION_TIMEDOUT;
     goto out;

lib/cf-ip-happy.c

@@ -496,8 +496,8 @@ out:
     bool more_possible;
 
     /* when do we need to be called again? */
-    next_expire_ms = Curl_timeleft_ms(data, TRUE);
-    if(next_expire_ms <= 0) {
+    next_expire_ms = Curl_timeleft_ms(data);
+    if(next_expire_ms < 0) {
       failf(data, "Connection timeout after %" FMT_OFF_T " ms",
         curlx_ptimediff_ms(Curl_pgrs_now(data),
                            &data->progress.t_startsingle));
@@ -699,7 +699,7 @@ static CURLcode start_connect(struct Curl_cfilter *cf,
   if(!dns)
     return CURLE_FAILED_INIT;
 
-  if(Curl_timeleft_ms(data, TRUE) < 0) {
+  if(Curl_timeleft_ms(data) < 0) {
     /* a precaution, no need to continue if time already is up */
     failf(data, "Connection time-out");
     return CURLE_OPERATION_TIMEDOUT;

lib/cf-socket.c

@@ -1955,7 +1955,7 @@ static timediff_t cf_tcp_accept_timeleft(struct Curl_cfilter *cf,
 #endif
 
   /* check if the generic timeout possibly is set shorter */
-  other_ms = Curl_timeleft_ms(data, FALSE);
+  other_ms = Curl_timeleft_ms(data);
   if(other_ms && (other_ms < timeout_ms))
     /* note that this also works fine for when other_ms happens to be negative
        due to it already having elapsed */

lib/cfilters.c

@@ -177,8 +177,6 @@ CURLcode Curl_conn_shutdown(struct Curl_easy *data, int sockindex, bool *done)
 
   *done = FALSE;
   if(!Curl_shutdown_started(data, sockindex)) {
-    CURL_TRC_M(data, "shutdown start on%s connection",
-               sockindex ? " secondary" : "");
     Curl_shutdown_start(data, sockindex, 0);
   }
   else {
@@ -552,7 +550,7 @@ CURLcode Curl_conn_connect(struct Curl_easy *data,
       goto out;
     else {
       /* check allowed time left */
-      const timediff_t timeout_ms = Curl_timeleft_ms(data, TRUE);
+      const timediff_t timeout_ms = Curl_timeleft_ms(data);
       curl_socket_t sockfd = Curl_conn_cf_get_socket(cf, data);
       int rc;
 

lib/connect.c

@@ -100,25 +100,27 @@ enum alpnid Curl_str2alpnid(const struct Curl_str *cstr)
  * transfer/connection. If the value is 0, there is no timeout (ie there is
  * infinite time left). If the value is negative, the timeout time has already
  * elapsed.
- * @param data the transfer to check on
- * @param duringconnect TRUE iff connect timeout is also taken into account.
  * @unittest: 1303
  */
 timediff_t Curl_timeleft_now_ms(struct Curl_easy *data,
-                                const struct curltime *pnow,
-                                bool duringconnect)
+                                const struct curltime *pnow)
 {
   timediff_t timeleft_ms = 0;
   timediff_t ctimeleft_ms = 0;
-  timediff_t ctimeout_ms;
 
-  /* The duration of a connect and the total transfer are calculated from two
-     different time-stamps. It can end up with the total timeout being reached
-     before the connect timeout expires and we must acknowledge whichever
-     timeout that is reached first. The total timeout is set per entire
-     operation, while the connect timeout is set per connect. */
-  if((!data->set.timeout || data->set.connect_only) && !duringconnect)
+  if(Curl_shutdown_started(data, FIRSTSOCKET))
+    return Curl_shutdown_timeleft(data, data->conn, FIRSTSOCKET);
+  else if(Curl_is_connecting(data)) {
+    timediff_t ctimeout_ms = (data->set.connecttimeout > 0) ?
+      data->set.connecttimeout : DEFAULT_CONNECT_TIMEOUT;
+    ctimeleft_ms = ctimeout_ms -
+      curlx_ptimediff_ms(pnow, &data->progress.t_startsingle);
+    if(!ctimeleft_ms)
+      ctimeleft_ms = -1; /* 0 is "no limit", fake 1 ms expiry */
+  }
+  else if(!data->set.timeout || data->set.connect_only) {
     return 0; /* no timeout in place or checked, return "no limit" */
+  }
 
   if(data->set.timeout) {
     timeleft_ms = data->set.timeout -
@@ -127,25 +129,16 @@ timediff_t Curl_timeleft_now_ms(struct Curl_easy *data,
       timeleft_ms = -1; /* 0 is "no limit", fake 1 ms expiry */
   }
 
-  if(!duringconnect)
-    return timeleft_ms; /* no connect check, this is it */
-  ctimeout_ms = (data->set.connecttimeout > 0) ?
-    data->set.connecttimeout : DEFAULT_CONNECT_TIMEOUT;
-  ctimeleft_ms = ctimeout_ms -
-    curlx_ptimediff_ms(pnow, &data->progress.t_startsingle);
   if(!ctimeleft_ms)
-    ctimeleft_ms = -1; /* 0 is "no limit", fake 1 ms expiry */
-  if(!timeleft_ms)
-    return ctimeleft_ms; /* no general timeout, this is it */
-
-  /* return minimal time left or max amount already expired */
-  return (ctimeleft_ms < timeleft_ms) ? ctimeleft_ms : timeleft_ms;
+    return timeleft_ms;
+  else if(!timeleft_ms)
+    return ctimeleft_ms;
+  return CURLMIN(ctimeleft_ms, timeleft_ms);
 }
 
-timediff_t Curl_timeleft_ms(struct Curl_easy *data,
-                            bool duringconnect)
+timediff_t Curl_timeleft_ms(struct Curl_easy *data)
 {
-  return Curl_timeleft_now_ms(data, Curl_pgrs_now(data), duringconnect);
+  return Curl_timeleft_now_ms(data, Curl_pgrs_now(data));
 }
 
 void Curl_shutdown_start(struct Curl_easy *data, int sockindex,
@@ -162,6 +155,8 @@ void Curl_shutdown_start(struct Curl_easy *data, int sockindex,
   /* Set a timer, unless we operate on the admin handle */
   if(data->mid)
     Curl_expire_ex(data, conn->shutdown.timeout_ms, EXPIRE_SHUTDOWN);
+  CURL_TRC_M(data, "shutdown start on%s connection",
+             sockindex ? " secondary" : "");
 }
 
 timediff_t Curl_shutdown_timeleft(struct Curl_easy *data,
@@ -204,8 +199,11 @@ void Curl_shutdown_clear(struct Curl_easy *data, int sockindex)
 
 bool Curl_shutdown_started(struct Curl_easy *data, int sockindex)
 {
-  struct curltime *pt = &data->conn->shutdown.start[sockindex];
-  return (pt->tv_sec > 0) || (pt->tv_usec > 0);
+  if(data->conn) {
+    struct curltime *pt = &data->conn->shutdown.start[sockindex];
+    return (pt->tv_sec > 0) || (pt->tv_usec > 0);
+  }
+  return FALSE;
 }
 
 /* retrieves ip address and port from a sockaddr structure. note it calls

lib/connect.h

@@ -36,11 +36,9 @@ enum alpnid Curl_str2alpnid(const struct Curl_str *str);
 
 /* generic function that returns how much time there is left to run, according
    to the timeouts set */
-timediff_t Curl_timeleft_ms(struct Curl_easy *data,
-                            bool duringconnect);
+timediff_t Curl_timeleft_ms(struct Curl_easy *data);
 timediff_t Curl_timeleft_now_ms(struct Curl_easy *data,
-                                const struct curltime *pnow,
-                                bool duringconnect);
+                                const struct curltime *pnow);
 
 #define DEFAULT_CONNECT_TIMEOUT 300000 /* milliseconds == five minutes */
 

lib/cshutdn.c

@@ -76,6 +76,10 @@ static void cshutdn_run_once(struct Curl_easy *data,
   /* We expect to be attached when called */
   DEBUGASSERT(data->conn == conn);
 
+  if(!Curl_shutdown_started(data, FIRSTSOCKET)) {
+    Curl_shutdown_start(data, FIRSTSOCKET, 0);
+  }
+
   cshutdn_run_conn_handler(data, conn);
 
   if(conn->bits.shutdown_filters) {

lib/doh.c

@@ -304,8 +304,8 @@ static CURLcode doh_probe_run(struct Curl_easy *data,
     goto error;
   }
 
-  timeout_ms = Curl_timeleft_ms(data, TRUE);
-  if(timeout_ms <= 0) {
+  timeout_ms = Curl_timeleft_ms(data);
+  if(timeout_ms < 0) {
     result = CURLE_OPERATION_TIMEDOUT;
     goto error;
   }

lib/ftp.c

@@ -686,7 +686,7 @@ static CURLcode getftpresponse(struct Curl_easy *data,
 
   while(!*ftpcodep && !result) {
     /* check and reset timeout value every lap */
-    timediff_t timeout = Curl_pp_state_timeout(data, pp, FALSE);
+    timediff_t timeout = Curl_pp_state_timeout(data, pp);
     timediff_t interval_ms;
 
     if(timeout <= 0) {

lib/gopher.c

@@ -125,7 +125,7 @@ static CURLcode gopher_do(struct Curl_easy *data, bool *done)
     else
       break;
 
-    timeout_ms = Curl_timeleft_ms(data, FALSE);
+    timeout_ms = Curl_timeleft_ms(data);
     if(timeout_ms < 0) {
       result = CURLE_OPERATION_TIMEDOUT;
       break;

lib/multi.c

@@ -351,6 +351,11 @@ static void multi_warn_debug(struct Curl_multi *multi, struct Curl_easy *data)
 #define multi_warn_debug(x, y) Curl_nop_stmt
 #endif
 
+bool Curl_is_connecting(struct Curl_easy *data)
+{
+  return data->mstate < MSTATE_DO;
+}
+
 static CURLMcode multi_xfers_add(struct Curl_multi *multi,
                                  struct Curl_easy *data)
 {
@@ -1720,14 +1725,13 @@ static bool multi_handle_timeout(struct Curl_easy *data,
                                  bool *stream_error,
                                  CURLcode *result)
 {
-  bool connect_timeout = data->mstate < MSTATE_DO;
   timediff_t timeout_ms;
 
-  timeout_ms = Curl_timeleft_ms(data, connect_timeout);
+  timeout_ms = Curl_timeleft_ms(data);
   if(timeout_ms < 0) {
     /* Handle timed out */
     struct curltime since;
-    if(connect_timeout)
+    if(Curl_is_connecting(data))
       since = data->progress.t_startsingle;
     else
       since = data->progress.t_startop;

lib/multiif.h

@@ -40,6 +40,7 @@ bool Curl_multiplex_wanted(const struct Curl_multi *multi);
 void Curl_set_in_callback(struct Curl_easy *data, bool value);
 bool Curl_is_in_callback(struct Curl_easy *data);
 CURLcode Curl_preconnect(struct Curl_easy *data);
+bool Curl_is_connecting(struct Curl_easy *data);
 
 void Curl_multi_connchanged(struct Curl_multi *multi);
 

lib/pingpong.c

@@ -29,6 +29,7 @@
 #include "urldata.h"
 #include "cfilters.h"
 #include "connect.h"
+#include "multiif.h"
 #include "sendf.h"
 #include "curl_trc.h"
 #include "select.h"
@@ -40,9 +41,9 @@
 /* Returns timeout in ms. 0 or negative number means the timeout has already
    triggered */
 timediff_t Curl_pp_state_timeout(struct Curl_easy *data,
-                                 struct pingpong *pp, bool disconnecting)
+                                 struct pingpong *pp)
 {
-  timediff_t timeout_ms; /* in milliseconds */
+  timediff_t timeout_ms, xfer_timeout_ms;
   timediff_t response_time = data->set.server_response_timeout ?
     data->set.server_response_timeout : RESP_TIMEOUT;
 
@@ -55,19 +56,10 @@ timediff_t Curl_pp_state_timeout(struct Curl_easy *data,
      full response to arrive before we bail out */
   timeout_ms = response_time -
                curlx_ptimediff_ms(Curl_pgrs_now(data), &pp->response);
-
-  if(data->set.timeout && !disconnecting) {
-    /* if timeout is requested, find out how much overall remains */
-    timediff_t timeout2_ms = Curl_timeleft_ms(data, FALSE);
-    /* pick the lowest number */
-    timeout_ms = CURLMIN(timeout_ms, timeout2_ms);
-  }
-
-  if(disconnecting) {
-    timediff_t total_left_ms = Curl_timeleft_ms(data, FALSE);
-    timeout_ms = CURLMIN(timeout_ms, CURLMAX(total_left_ms, 0));
-  }
-
+  /* transfer timeout can be 0, which means no timeout applies */
+  xfer_timeout_ms = Curl_timeleft_ms(data);
+  if(xfer_timeout_ms && (xfer_timeout_ms < timeout_ms))
+    return xfer_timeout_ms;
   return timeout_ms;
 }
 
@@ -82,7 +74,7 @@ CURLcode Curl_pp_statemach(struct Curl_easy *data,
   curl_socket_t sock = conn->sock[FIRSTSOCKET];
   int rc;
   timediff_t interval_ms;
-  timediff_t timeout_ms = Curl_pp_state_timeout(data, pp, disconnecting);
+  timediff_t timeout_ms = Curl_pp_state_timeout(data, pp);
   CURLcode result = CURLE_OK;
 
   if(timeout_ms <= 0) {

lib/pingpong.h

@@ -91,7 +91,7 @@ void Curl_pp_init(struct pingpong *pp, const struct curltime *pnow);
 /* Returns timeout in ms. 0 or negative number means the timeout has already
    triggered */
 timediff_t Curl_pp_state_timeout(struct Curl_easy *data,
-                                 struct pingpong *pp, bool disconnecting);
+                                 struct pingpong *pp);
 
 /***********************************************************************
  *

lib/socks.c

@@ -131,7 +131,7 @@ CURLcode Curl_blockread_all(struct Curl_cfilter *cf,
 
   *pnread = 0;
   for(;;) {
-    timediff_t timeout_ms = Curl_timeleft_ms(data, TRUE);
+    timediff_t timeout_ms = Curl_timeleft_ms(data);
     if(timeout_ms < 0) {
       /* we already got the timeout */
       return CURLE_OPERATION_TIMEDOUT;

lib/tftp.c

@@ -156,10 +156,9 @@ static CURLcode tftp_set_timeouts(struct tftp_conn *state)
 {
   time_t timeout;
   timediff_t timeout_ms;
-  bool start = (state->state == TFTP_STATE_START);
 
   /* Compute drop-dead time */
-  timeout_ms = Curl_timeleft_ms(state->data, start);
+  timeout_ms = Curl_timeleft_ms(state->data);
 
   if(timeout_ms < 0) {
     /* time-out, bail out, go home */
@@ -1142,8 +1141,7 @@ static timediff_t tftp_state_timeout(struct tftp_conn *state,
   if(event)
     *event = TFTP_EVENT_NONE;
 
-  timeout_ms = Curl_timeleft_ms(state->data,
-                                (state->state == TFTP_STATE_START));
+  timeout_ms = Curl_timeleft_ms(state->data);
   if(timeout_ms < 0) {
     state->error = TFTP_ERR_TIMEOUT;
     state->state = TFTP_STATE_FIN;

lib/transfer.c

@@ -386,7 +386,7 @@ CURLcode Curl_sendrecv(struct Curl_easy *data)
     goto out;
 
   if(k->keepon) {
-    if(Curl_timeleft_ms(data, FALSE) < 0) {
+    if(Curl_timeleft_ms(data) < 0) {
       if(k->size != -1) {
         failf(data, "Operation timed out after %" FMT_TIMEDIFF_T
               " milliseconds with %" FMT_OFF_T " out of %"

lib/url.c

@@ -3039,7 +3039,7 @@ static CURLcode resolve_server(struct Curl_easy *data,
 {
   struct hostname *ehost;
   int eport;
-  timediff_t timeout_ms = Curl_timeleft_ms(data, TRUE);
+  timediff_t timeout_ms = Curl_timeleft_ms(data);
   const char *peertype = "host";
   CURLcode result;
 

lib/vssh/libssh.c

@@ -2420,7 +2420,7 @@ static CURLcode myssh_block_statemach(struct Curl_easy *data,
       if(result)
         break;
 
-      left_ms = Curl_timeleft_ms(data, FALSE);
+      left_ms = Curl_timeleft_ms(data);
       if(left_ms < 0) {
         failf(data, "Operation timed out");
         return CURLE_OPERATION_TIMEDOUT;

lib/vssh/libssh2.c

@@ -3205,7 +3205,7 @@ static CURLcode ssh_block_statemach(struct Curl_easy *data,
       if(result)
         break;
 
-      left_ms = Curl_timeleft_ms(data, FALSE);
+      left_ms = Curl_timeleft_ms(data);
       if(left_ms < 0) {
         failf(data, "Operation timed out");
         return CURLE_OPERATION_TIMEDOUT;

lib/vtls/schannel.c

@@ -1797,7 +1797,7 @@ schannel_recv_renegotiate(struct Curl_cfilter *cf, struct Curl_easy *data,
       remaining = MAX_RENEG_BLOCK_TIME - elapsed;
 
       if(blocking) {
-        timeout_ms = Curl_timeleft_ms(data, FALSE);
+        timeout_ms = Curl_timeleft_ms(data);
 
         if(timeout_ms < 0) {
           result = CURLE_OPERATION_TIMEDOUT;
@@ -1950,7 +1950,7 @@ static CURLcode schannel_send(struct Curl_cfilter *cf, struct Curl_easy *data,
     while(len > *pnwritten) {
       size_t this_write = 0;
       int what;
-      timediff_t timeout_ms = Curl_timeleft_ms(data, FALSE);
+      timediff_t timeout_ms = Curl_timeleft_ms(data);
       if(timeout_ms < 0) {
         /* we already got the timeout */
         failf(data, "schannel: timed out sending data "

lib/ws.c

@@ -1699,7 +1699,7 @@ static CURLcode ws_send_raw_blocking(struct Curl_easy *data,
 
       CURL_TRC_WS(data, "ws_send_raw_blocking() partial, %zu left to send",
                   buflen);
-      left_ms = Curl_timeleft_ms(data, FALSE);
+      left_ms = Curl_timeleft_ms(data);
       if(left_ms < 0) {
         failf(data, "[WS] Timeout waiting for socket becoming writable");
         return CURLE_SEND_ERROR;

tests/unit/unit1303.c

@@ -148,7 +148,8 @@ static CURLcode test_unit1303(const char *arg)
     NOW(run[i].now_s, run[i].now_us);
     TIMEOUTS(run[i].timeout_ms, run[i].connecttimeout_ms);
     easy->progress.now = now;
-    timeout = Curl_timeleft_now_ms(easy, &now, run[i].connecting);
+    easy->mstate = run[i].connecting ? MSTATE_INIT : MSTATE_DO;
+    timeout = Curl_timeleft_now_ms(easy, &now);
     if(timeout != run[i].result)
       fail(run[i].comment);
   }