url: fix connection lifetime checks

The checks for a connection being "too long idle" or "too old" where
rounding down the elapsed time to seconds before comparing to the
configured max values. This caused connections to be reused for up to
999ms longer than intended.

Change the compares to scale the configured seconds up to ms, so
connection will properly be "too old" 1 ms after the coonfigured values.

Fixes sporadic failures of test1542 on platforms where "sleep(2)"
returnes before 2 full seconds on the internal clock where passed.

Reported-by: Christian Weisgerber
URL: https://curl.se/mail/lib-2025-06/0004.html
Closes #17571

diff --git a/lib/cf-https-connect.c b/lib/cf-https-connect.c
index cd0d226efdad980ec4048f227c06b33095677165..00c26d47c8ded0a895bafbd8639b9ffc099045a8 100644 (file)
--- a/lib/cf-https-connect.c
+++ b/lib/cf-https-connect.c
@@ -117,8 +117,8 @@ struct cf_hc_ctx {
   CURLcode result;          /* overall result */
   struct cf_hc_baller ballers[2];
   size_t baller_count;
-  unsigned int soft_eyeballs_timeout_ms;
-  unsigned int hard_eyeballs_timeout_ms;
+  timediff_t soft_eyeballs_timeout_ms;
+  timediff_t hard_eyeballs_timeout_ms;
 };
 
 static void cf_hc_baller_assign(struct cf_hc_baller *b,
@@ -268,15 +268,16 @@ static bool time_to_start_next(struct Curl_cfilter *cf,
   }
   elapsed_ms = curlx_timediff(now, ctx->started);
   if(elapsed_ms >= ctx->hard_eyeballs_timeout_ms) {
-    CURL_TRC_CF(data, cf, "hard timeout of %dms reached, starting %s",
+    CURL_TRC_CF(data, cf, "hard timeout of %" FMT_TIMEDIFF_T "ms reached, "
+                "starting %s",
                 ctx->hard_eyeballs_timeout_ms, ctx->ballers[idx].name);
     return TRUE;
   }
 
   if((idx > 0) && (elapsed_ms >= ctx->soft_eyeballs_timeout_ms)) {
     if(cf_hc_baller_reply_ms(&ctx->ballers[idx - 1], data) < 0) {
-      CURL_TRC_CF(data, cf, "soft timeout of %dms reached, %s has not "
-                  "seen any data, starting %s",
+      CURL_TRC_CF(data, cf, "soft timeout of %" FMT_TIMEDIFF_T "ms reached, "
+                  "%s has not seen any data, starting %s",
                   ctx->soft_eyeballs_timeout_ms,
                   ctx->ballers[idx - 1].name, ctx->ballers[idx].name);
       return TRUE;
@@ -314,8 +315,8 @@ static CURLcode cf_hc_connect(struct Curl_cfilter *cf,
     cf_hc_baller_init(&ctx->ballers[0], cf, data, cf->conn->transport);
     if(ctx->baller_count > 1) {
       Curl_expire(data, ctx->soft_eyeballs_timeout_ms, EXPIRE_ALPN_EYEBALLS);
-      CURL_TRC_CF(data, cf, "set next attempt to start in %ums",
-                  ctx->soft_eyeballs_timeout_ms);
+      CURL_TRC_CF(data, cf, "set next attempt to start in %" FMT_TIMEDIFF_T
+                  "ms", ctx->soft_eyeballs_timeout_ms);
     }
     ctx->state = CF_HC_CONNECT;
     FALLTHROUGH();

diff --git a/lib/connect.c b/lib/connect.c
index abf30a641f423ec3b94f77b0bb898a0e03d00da2..41435235cefb8830cbd4a7061bbb5bd2acc21d09 100644 (file)
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -172,11 +172,11 @@ void Curl_shutdown_start(struct Curl_easy *data, int sockindex,
   }
   data->conn->shutdown.start[sockindex] = *nowp;
   data->conn->shutdown.timeout_ms = (timeout_ms > 0) ?
-    (unsigned int)timeout_ms :
+    (timediff_t)timeout_ms :
     ((data->set.shutdowntimeout > 0) ?
      data->set.shutdowntimeout : DEFAULT_SHUTDOWN_TIMEOUT_MS);
   /* Set a timer, unless we operate on the admin handle */
-  if(data->mid && data->conn->shutdown.timeout_ms)
+  if(data->mid && (data->conn->shutdown.timeout_ms > 0))
     Curl_expire_ex(data, nowp, data->conn->shutdown.timeout_ms,
                    EXPIRE_SHUTDOWN);
 }
@@ -187,7 +187,8 @@ timediff_t Curl_shutdown_timeleft(struct connectdata *conn, int sockindex,
   struct curltime now;
   timediff_t left_ms;
 
-  if(!conn->shutdown.start[sockindex].tv_sec || !conn->shutdown.timeout_ms)
+  if(!conn->shutdown.start[sockindex].tv_sec ||
+     (conn->shutdown.timeout_ms <= 0))
     return 0; /* not started or no limits */
 
   if(!nowp) {

diff --git a/lib/ftp.c b/lib/ftp.c
index 576100be46e5bfab1197c35d49980e88aad2629e..6246b8d3aca7e80090e61ccd6db20b89bbbfad94 100644 (file)
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -1245,7 +1245,7 @@ out:
     }
     data->conn->bits.do_more = FALSE;
     Curl_pgrsTime(data, TIMER_STARTACCEPT);
-    Curl_expire(data, data->set.accepttimeout ?
+    Curl_expire(data, (data->set.accepttimeout > 0) ?
                 data->set.accepttimeout: DEFAULT_ACCEPT_TIMEOUT,
                 EXPIRE_FTP_ACCEPT);
   }

diff --git a/lib/pingpong.c b/lib/pingpong.c
index c5513f6050283e37821e33a474f46daab9a58d5a..7ef57e946b2432e4224ab8fcd71dc3d49d1bba28 100644 (file)
--- a/lib/pingpong.c
+++ b/lib/pingpong.c
@@ -52,7 +52,7 @@ timediff_t Curl_pp_state_timeout(struct Curl_easy *data,
                                  struct pingpong *pp, bool disconnecting)
 {
   timediff_t timeout_ms; /* in milliseconds */
-  timediff_t response_time = (data->set.server_response_timeout) ?
+  timediff_t response_time = (data->set.server_response_timeout > 0) ?
     data->set.server_response_timeout : pp->response_time;
   struct curltime now = curlx_now();
 
@@ -65,7 +65,7 @@ timediff_t Curl_pp_state_timeout(struct Curl_easy *data,
      full response to arrive before we bail out */
   timeout_ms = response_time - curlx_timediff(now, pp->response);
 
-  if(data->set.timeout && !disconnecting) {
+  if((data->set.timeout > 0) && !disconnecting) {
     /* if timeout is requested, find out how much overall remains */
     timediff_t timeout2_ms = Curl_timeleft(data, &now, FALSE);
     /* pick the lowest number */

diff --git a/lib/setopt.c b/lib/setopt.c
index d5ddf6d0c26dac993a482ab0c7b812cbebe968a1..b32b9a9d42c4cf34a750dda764fe8253b7417a5d 100644 (file)
--- a/lib/setopt.c
+++ b/lib/setopt.c
@@ -60,6 +60,35 @@
 #include "curl_memory.h"
 #include "memdebug.h"
 
+
+static CURLcode setopt_set_timeout_sec(timediff_t *ptimeout_ms, long secs)
+{
+  if(secs < 0)
+    return CURLE_BAD_FUNCTION_ARGUMENT;
+#if LONG_MAX > (TIMEDIFF_T_MAX/1000)
+  if(secs > (TIMEDIFF_T_MAX/1000)) {
+    *ptimeout_ms = TIMEDIFF_T_MAX;
+    return CURLE_OK;
+  }
+#endif
+  *ptimeout_ms = (timediff_t)secs * 1000;
+  return CURLE_OK;
+}
+
+static CURLcode setopt_set_timeout_ms(timediff_t *ptimeout_ms, long ms)
+{
+  if(ms < 0)
+    return CURLE_BAD_FUNCTION_ARGUMENT;
+#if LONG_MAX > TIMEDIFF_T_MAX
+  if(ms > TIMEDIFF_T_MAX) {
+    *ptimeout_ms = TIMEDIFF_T_MAX;
+    return CURLE_OK;
+  }
+#endif
+  *ptimeout_ms = (timediff_t)ms;
+  return CURLE_OK;
+}
+
 CURLcode Curl_setstropt(char **charp, const char *s)
 {
   /* Release the previous storage at `charp' and replace by a dynamic storage
@@ -526,21 +555,15 @@ static CURLcode setopt_long(struct Curl_easy *data, CURLoption option,
      * Option that specifies how quickly a server response must be obtained
      * before it is considered failure. For pingpong protocols.
      */
-    if((arg >= 0) && (arg <= (INT_MAX/1000)))
-      data->set.server_response_timeout = (unsigned int)arg * 1000;
-    else
-      return CURLE_BAD_FUNCTION_ARGUMENT;
-    break;
+    return setopt_set_timeout_sec(&data->set.server_response_timeout, arg);
+
   case CURLOPT_SERVER_RESPONSE_TIMEOUT_MS:
     /*
      * Option that specifies how quickly a server response must be obtained
      * before it is considered failure. For pingpong protocols.
      */
-    if((arg >= 0) && (arg <= INT_MAX))
-      data->set.server_response_timeout = (unsigned int)arg;
-    else
-      return CURLE_BAD_FUNCTION_ARGUMENT;
-    break;
+    return setopt_set_timeout_ms(&data->set.server_response_timeout, arg);
+
 #ifndef CURL_DISABLE_TFTP
   case CURLOPT_TFTP_NO_OPTIONS:
     /*
@@ -883,10 +906,8 @@ static CURLcode setopt_long(struct Curl_easy *data, CURLoption option,
     /*
      * The maximum time for curl to wait for FTP server connect
      */
-    if(uarg > UINT_MAX)
-      uarg = UINT_MAX;
-    data->set.accepttimeout = (unsigned int)uarg;
-    break;
+    return setopt_set_timeout_ms(&data->set.accepttimeout, arg);
+
   case CURLOPT_WILDCARDMATCH:
     data->set.wildcard_enabled = enabled;
     break;
@@ -943,33 +964,19 @@ static CURLcode setopt_long(struct Curl_easy *data, CURLoption option,
      * The maximum time you allow curl to use for a single transfer
      * operation.
      */
-    if((arg >= 0) && (arg <= (INT_MAX/1000)))
-      data->set.timeout = (unsigned int)arg * 1000;
-    else
-      return CURLE_BAD_FUNCTION_ARGUMENT;
-    break;
+    return setopt_set_timeout_sec(&data->set.timeout, arg);
 
   case CURLOPT_TIMEOUT_MS:
-    if(uarg > UINT_MAX)
-      uarg = UINT_MAX;
-    data->set.timeout = (unsigned int)uarg;
-    break;
+    return setopt_set_timeout_ms(&data->set.timeout, arg);
 
   case CURLOPT_CONNECTTIMEOUT:
     /*
      * The maximum time you allow curl to use to connect.
      */
-    if((arg >= 0) && (arg <= (INT_MAX/1000)))
-      data->set.connecttimeout = (unsigned int)arg * 1000;
-    else
-      return CURLE_BAD_FUNCTION_ARGUMENT;
-    break;
+    return setopt_set_timeout_sec(&data->set.connecttimeout, arg);
 
   case CURLOPT_CONNECTTIMEOUT_MS:
-    if(uarg > UINT_MAX)
-      uarg = UINT_MAX;
-    data->set.connecttimeout = (unsigned int)uarg;
-    break;
+    return setopt_set_timeout_ms(&data->set.connecttimeout, arg);
 
   case CURLOPT_RESUME_FROM:
     /*
@@ -1347,10 +1354,8 @@ static CURLcode setopt_long(struct Curl_easy *data, CURLoption option,
     data->set.suppress_connect_headers = enabled;
     break;
   case CURLOPT_HAPPY_EYEBALLS_TIMEOUT_MS:
-    if(uarg > UINT_MAX)
-      uarg = UINT_MAX;
-    data->set.happy_eyeballs_timeout = (unsigned int)uarg;
-    break;
+    return setopt_set_timeout_ms(&data->set.happy_eyeballs_timeout, arg);
+
 #ifndef CURL_DISABLE_SHUFFLE_DNS
   case CURLOPT_DNS_SHUFFLE_ADDRESSES:
     data->set.dns_shuffle_addresses = enabled;
@@ -1366,15 +1371,11 @@ static CURLcode setopt_long(struct Curl_easy *data, CURLoption option,
     data->set.upkeep_interval_ms = arg;
     break;
   case CURLOPT_MAXAGE_CONN:
-    if(arg < 0)
-      return CURLE_BAD_FUNCTION_ARGUMENT;
-    data->set.maxage_conn = arg;
-    break;
+    return setopt_set_timeout_sec(&data->set.conn_max_idle_ms, arg);
+
   case CURLOPT_MAXLIFETIME_CONN:
-    if(arg < 0)
-      return CURLE_BAD_FUNCTION_ARGUMENT;
-    data->set.maxlifetime_conn = arg;
-    break;
+    return setopt_set_timeout_sec(&data->set.conn_max_age_ms, arg);
+
 #ifndef CURL_DISABLE_HSTS
   case CURLOPT_HSTS_CTRL:
     if(arg & CURLHSTS_ENABLE) {

diff --git a/lib/url.c b/lib/url.c
index 11fd3f8c2d9ab3a9a0ad6c974d623dab82cdc990..3597c5cb4575593ae8246491c85cf071490dd136 100644 (file)
--- a/lib/url.c
+++ b/lib/url.c
@@ -473,8 +473,8 @@ CURLcode Curl_init_userdefined(struct Curl_easy *data)
   set->happy_eyeballs_timeout = CURL_HET_DEFAULT;
   set->upkeep_interval_ms = CURL_UPKEEP_INTERVAL_DEFAULT;
   set->maxconnects = DEFAULT_CONNCACHE_SIZE; /* for easy handles */
-  set->maxage_conn = 118;
-  set->maxlifetime_conn = 0;
+  set->conn_max_idle_ms = 118 * 1000;
+  set->conn_max_age_ms = 0;
   set->http09_allowed = FALSE;
   set->httpwant = CURL_HTTP_VERSION_NONE
     ;
@@ -671,36 +671,36 @@ socks_proxy_info_matches(const struct proxy_info *data,
 #define socks_proxy_info_matches(x,y) FALSE
 #endif
 
-/* A connection has to have been idle for a shorter time than 'maxage_conn'
+/* A connection has to have been idle for less than 'conn_max_idle_ms'
    (the success rate is just too low after this), or created less than
-   'maxlifetime_conn' ago, to be subject for reuse. */
-
+   'conn_max_age_ms' ago, to be subject for reuse. */
 static bool conn_maxage(struct Curl_easy *data,
                         struct connectdata *conn,
                         struct curltime now)
 {
-  timediff_t idletime, lifetime;
-
-  idletime = curlx_timediff(now, conn->lastused);
-  idletime /= 1000; /* integer seconds is fine */
-
-  if(idletime > data->set.maxage_conn) {
-    infof(data, "Too old connection (%" FMT_TIMEDIFF_T
-          " seconds idle), disconnect it", idletime);
-    return TRUE;
+  timediff_t age_ms;
+
+  if(data->set.conn_max_idle_ms) {
+    age_ms = curlx_timediff(now, conn->lastused);
+    if(age_ms > data->set.conn_max_idle_ms) {
+      infof(data, "Too old connection (%" FMT_TIMEDIFF_T
+            " ms idle, max idle is %" FMT_TIMEDIFF_T " ms), disconnect it",
+            age_ms, data->set.conn_max_idle_ms);
+      return TRUE;
+    }
   }
 
-  lifetime = curlx_timediff(now, conn->created);
-  lifetime /= 1000; /* integer seconds is fine */
-
-  if(data->set.maxlifetime_conn && lifetime > data->set.maxlifetime_conn) {
-    infof(data,
-          "Too old connection (%" FMT_TIMEDIFF_T
-          " seconds since creation), disconnect it", lifetime);
-    return TRUE;
+  if(data->set.conn_max_age_ms) {
+    age_ms = curlx_timediff(now, conn->created);
+    if(age_ms > data->set.conn_max_age_ms) {
+      infof(data,
+            "Too old connection (created %" FMT_TIMEDIFF_T
+            " ms ago, max lifetime is %" FMT_TIMEDIFF_T " ms), disconnect it",
+            age_ms, data->set.conn_max_age_ms);
+      return TRUE;
+    }
   }
 
-
   return FALSE;
 }
 

diff --git a/lib/urldata.h b/lib/urldata.h
index d7fff0b2fa98655a6bc818738cbaa71960a9ddcd..df00466c77ed630b25381de36ad48782a0e82952 100644 (file)
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -701,7 +701,7 @@ struct connectdata {
   struct Curl_cfilter *cfilter[2]; /* connection filters */
   struct {
     struct curltime start[2]; /* when filter shutdown started */
-    unsigned int timeout_ms; /* 0 means no timeout */
+    timediff_t timeout_ms; /* 0 means no timeout */
   } shutdown;
 
   struct ssl_primary_config ssl_config;
@@ -1412,9 +1412,9 @@ struct UserDefined {
 #endif
   void *progress_client; /* pointer to pass to the progress callback */
   void *ioctl_client;   /* pointer to pass to the ioctl callback */
-  long maxage_conn;     /* in seconds, max idle time to allow a connection that
+  timediff_t conn_max_idle_ms; /* max idle time to allow a connection that
                            is to be reused */
-  long maxlifetime_conn; /* in seconds, max time since creation to allow a
+  timediff_t conn_max_age_ms; /* max time since creation to allow a
                             connection that is to be reused */
 #ifndef CURL_DISABLE_TFTP
   long tftp_blksize;    /* in bytes, 0 means use default */
@@ -1460,7 +1460,7 @@ struct UserDefined {
 #endif
   curl_off_t max_filesize; /* Maximum file size to download */
 #ifndef CURL_DISABLE_FTP
-  unsigned int accepttimeout;   /* in milliseconds, 0 means no timeout */
+  timediff_t accepttimeout;   /* in milliseconds, 0 means no timeout */
   unsigned char ftp_filemethod; /* how to get to a file: curl_ftpfile  */
   unsigned char ftpsslauth; /* what AUTH XXX to try: curl_ftpauth */
   unsigned char ftp_ccc;   /* FTP CCC options: curl_ftpccc */
@@ -1504,11 +1504,11 @@ struct UserDefined {
   void *wildcardptr;
 #endif
 
-  unsigned int timeout;        /* ms, 0 means no timeout */
-  unsigned int connecttimeout; /* ms, 0 means default timeout */
-  unsigned int happy_eyeballs_timeout; /* ms, 0 is a valid value */
-  unsigned int server_response_timeout; /* ms, 0 means no timeout */
-  unsigned int shutdowntimeout; /* ms, 0 means default timeout */
+  timediff_t timeout;   /* ms, 0 means no timeout */
+  timediff_t connecttimeout; /* ms, 0 means default timeout */
+  timediff_t happy_eyeballs_timeout; /* ms, 0 is a valid value */
+  timediff_t server_response_timeout; /* ms, 0 means no timeout */
+  timediff_t shutdowntimeout; /* ms, 0 means default timeout */
   int tcp_keepidle;     /* seconds in idle before sending keepalive probe */
   int tcp_keepintvl;    /* seconds between TCP keepalive probes */
   int tcp_keepcnt;      /* maximum number of keepalive probes */