RPi-Distro / repo

Issue tracking for the archive.raspberrypi.com repo
37 stars 1 forks source link

Download statistics on curl command line are wrong #169

Closed Bylon2 closed 2 years ago

Bylon2 commented 4 years ago

Hello, not sure how to file bug into Raspbian packages inherited from Debian.

In the standard Raspbian Buster the curl package is 7.64.0 (as per Debian Buster Stable).

This package exhibits a bug when in 32 bits (it is the case here) when a download takes longer than 35m and 47s. It is a signed 32 bit overflow because times are in millseconds the maximum that can be expressed is 2147,483647 seconds, which is 35 minutes 47 seconds and some milliseconds.

I filed the bug at curl here: https://github.com/curl/curl/issues/5079

This is a known bug and has been fixed around 7.66

I am currently trying a patch, taken from the commit that fixed it on 7.66, this patch is taken from the commit on Github and seems to apply on 7.64 except the second non important snippet.

Questions:

Here is the patch I am testing. I have build the package successfully including that patch, and I'll let you know if all works well:


From b1616dad8f088d873d88f88b4d884335a4ca285f Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Wed, 31 Jul 2019 15:30:31 +0200
Subject: [PATCH] timediff: make it 64 bit (if possible) even with 32 bit
 time_t

... to make it hold microseconds too.

Fixes #4165
Closes #4168
---
 lib/asyn-thread.c |  7 ++++---
 lib/connect.c     | 10 +++++-----
 lib/ftp.c         |  4 ++--
 lib/hostip.c      |  2 +-
 lib/hostip.h      |  3 ++-
 lib/http2.c       |  2 +-
 lib/multi.c       |  4 ++--
 lib/multiif.h     |  2 +-
 lib/pingpong.c    |  4 ++--
 lib/progress.c    | 11 ++++++-----
 lib/select.h      |  6 +++---
 lib/timeval.c     | 24 ++++++++----------------
 lib/timeval.h     | 10 +++++-----
 lib/url.c         |  3 ++-
 lib/urldata.h     | 18 +++++++++---------
 15 files changed, 53 insertions(+), 57 deletions(-)

diff --git a/lib/asyn-thread.c b/lib/asyn-thread.c
index dcc2e8a9d35..222e78d98aa 100755
--- a/lib/asyn-thread.c
+++ b/lib/asyn-thread.c
@@ -603,8 +603,9 @@ CURLcode Curl_resolver_is_resolved(struct connectdata *conn,
   }
   else {
     /* poll for name lookup done with exponential backoff up to 250ms */
-    timediff_t elapsed = Curl_timediff(Curl_now(),
-                                       data->progress.t_startsingle);
+    /* should be fine even if this converts to 32 bit */
+    time_t elapsed = (time_t)Curl_timediff(Curl_now(),
+                                           data->progress.t_startsingle);
     if(elapsed < 0)
       elapsed = 0;

diff --git a/lib/connect.c b/lib/connect.c
index 59084f251dd..be9fcba66c8 100644
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -795,8 +795,8 @@ CURLcode Curl_is_connected(struct connectdata *conn,
     if(rc == 0) { /* no connection yet */
       error = 0;
       if(Curl_timediff(now, conn->connecttime) >= conn->timeoutms_per_addr) {
-        infof(data, "After %ldms connect time, move on!\n",
-              conn->timeoutms_per_addr);
+        infof(data, "After %" CURL_FORMAT_TIMEDIFF_T
+              "ms connect time, move on!\n", conn->timeoutms_per_addr);
         error = ETIMEDOUT;
       }

@@ -862,11 +862,11 @@ CURLcode Curl_is_connected(struct connectdata *conn,
               Curl_strerror(error, buffer, sizeof(buffer)));

         conn->timeoutms_per_addr = conn->tempaddr[i]->ai_next == NULL ?
-                                   allow : allow / 2;
+          allow : allow / 2;

         status = trynextip(conn, sockindex, i);
-        if(status != CURLE_COULDNT_CONNECT
-            || conn->tempsock[other] == CURL_SOCKET_BAD)
+        if((status != CURLE_COULDNT_CONNECT) ||
+           conn->tempsock[other] == CURL_SOCKET_BAD)
           /* the last attempt failed and no other sockets remain open */
           result = status;
       }
diff --git a/lib/ftp.c b/lib/ftp.c
index 916efe6f82e..e807a2acdee 100644
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -380,7 +380,7 @@ static CURLcode ReceivedServerConnect(struct connectdata *conn, bool *received)
   struct ftp_conn *ftpc = &conn->proto.ftpc;
   struct pingpong *pp = &ftpc->pp;
   int result;
-  time_t timeout_ms;
+  timediff_t timeout_ms;
   ssize_t nread;
   int ftpcode;

@@ -491,7 +491,7 @@ static CURLcode InitiateTransfer(struct connectdata *conn)
 static CURLcode AllowServerConnect(struct connectdata *conn, bool *connected)
 {
   struct Curl_easy *data = conn->data;
-  time_t timeout_ms;
+  timediff_t timeout_ms;
   CURLcode result = CURLE_OK;

   *connected = FALSE;
diff --git a/lib/hostip.c b/lib/hostip.c
index 5122070a0e9..bd532a891ed 100644
--- a/lib/hostip.c
+++ b/lib/hostip.c
@@ -624,7 +624,7 @@ int Curl_resolv_timeout(struct connectdata *conn,
                         const char *hostname,
                         int port,
                         struct Curl_dns_entry **entry,
-                        time_t timeoutms)
+                        timediff_t timeoutms)
 {
 #ifdef USE_ALARM_TIMEOUT
 #ifdef HAVE_SIGACTION
diff --git a/lib/hostip.h b/lib/hostip.h
index 6742e4d29fa..e0597ea96a0 100644
--- a/lib/hostip.h
+++ b/lib/hostip.h
@@ -25,6 +25,7 @@
 #include "curl_setup.h"
 #include "hash.h"
 #include "curl_addrinfo.h"
+#include "timeval.h" /* for timediff_t */
 #include "asyn.h"

 #ifdef HAVE_SETJMP_H
@@ -89,7 +90,7 @@ int Curl_resolv(struct connectdata *conn,
                 struct Curl_dns_entry **dnsentry);
 int Curl_resolv_timeout(struct connectdata *conn, const char *hostname,
                         int port, struct Curl_dns_entry **dnsentry,
-                        time_t timeoutms);
+                        timediff_t timeoutms);

 #ifdef CURLRES_IPV6
 /*
diff --git a/lib/http2.c b/lib/http2.c
index bfc0837b024..930e8516530 100644
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -233,7 +233,7 @@ static unsigned int http2_conncheck(struct connectdata *check,

   if(checks_to_perform & CONNCHECK_KEEPALIVE) {
     struct curltime now = Curl_now();
-    time_t elapsed = Curl_timediff(now, check->keepalive);
+    timediff_t elapsed = Curl_timediff(now, check->keepalive);

     if(elapsed > check->upkeep_interval_ms) {
       /* Perform an HTTP/2 PING */
diff --git a/lib/multi.c b/lib/multi.c
index 9b6801db76e..6e16eafe0d5 100755
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -2838,7 +2838,7 @@ multi_addtimeout(struct Curl_easy *data,
  *
  * Expire replaces a former timeout using the same id if already set.
  */
-void Curl_expire(struct Curl_easy *data, time_t milli, expire_id id)
+void Curl_expire(struct Curl_easy *data, timediff_t milli, expire_id id)
 {
   struct Curl_multi *multi = data->multi;
   struct curltime *nowp = &data->state.expiretime;
@@ -2852,7 +2852,7 @@ void Curl_expire(struct Curl_easy *data, time_t milli, expire_id id)
   DEBUGASSERT(id < EXPIRE_LAST);

   set = Curl_now();
-  set.tv_sec += milli/1000;
+  set.tv_sec += (time_t)(milli/1000); /* might be a 64 to 32 bit conversion */
   set.tv_usec += (unsigned int)(milli%1000)*1000;

   if(set.tv_usec >= 1000000) {
diff --git a/lib/multiif.h b/lib/multiif.h
index a6445586778..57b6145eb7e 100644
--- a/lib/multiif.h
+++ b/lib/multiif.h
@@ -27,7 +27,7 @@
  */

 void Curl_updatesocket(struct Curl_easy *data);
-void Curl_expire(struct Curl_easy *data, time_t milli, expire_id);
+void Curl_expire(struct Curl_easy *data, timediff_t milli, expire_id);
 void Curl_expire_clear(struct Curl_easy *data);
 void Curl_expire_done(struct Curl_easy *data, expire_id id);
 void Curl_update_timer(struct Curl_multi *multi);
diff --git a/lib/pingpong.c b/lib/pingpong.c
index 76783d41e8c..d0710053bd3 100644
--- a/lib/pingpong.c
+++ b/lib/pingpong.c
@@ -60,12 +60,12 @@ time_t Curl_pp_state_timeout(struct pingpong *pp, bool disconnecting)
   /* Without a requested timeout, we only wait 'response_time' seconds for the
      full response to arrive before we bail out */
   timeout_ms = response_time -
-    Curl_timediff(Curl_now(), pp->response); /* spent time */
+    (time_t)Curl_timediff(Curl_now(), pp->response); /* spent time */

   if(data->set.timeout && !disconnecting) {
     /* if timeout is requested, find out how much remaining time we have */
     time_t timeout2_ms = data->set.timeout - /* timeout time */
-      Curl_timediff(Curl_now(), conn->now); /* spent time */
+      (time_t)Curl_timediff(Curl_now(), conn->now); /* spent time */

     /* pick the lowest number */
     timeout_ms = CURLMIN(timeout_ms, timeout2_ms);
diff --git a/lib/progress.c b/lib/progress.c
index 8f81f28b7be..2aa92959931 100644
--- a/lib/progress.c
+++ b/lib/progress.c
@@ -26,6 +26,7 @@
 #include "sendf.h"
 #include "multiif.h"
 #include "progress.h"
+#include "timeval.h"
 #include "curl_printf.h"

 /* check rate limits within this many recent milliseconds, at minimum. */
@@ -168,7 +169,7 @@ void Curl_pgrsResetTransferSizes(struct Curl_easy *data)
 void Curl_pgrsTime(struct Curl_easy *data, timerid timer)
 {
   struct curltime now = Curl_now();
-  time_t *delta = NULL;
+  timediff_t *delta = NULL;

   switch(timer) {
   default:
@@ -270,8 +271,8 @@ timediff_t Curl_pgrsLimitWaitTime(curl_off_t cursize,
                                   struct curltime now)
 {
   curl_off_t size = cursize - startsize;
-  time_t minimum;
-  time_t actual;
+  timediff_t minimum;
+  timediff_t actual;

   if(!limit || !size)
     return 0;
@@ -284,10 +285,10 @@ timediff_t Curl_pgrsLimitWaitTime(curl_off_t cursize,
     minimum = (time_t) (CURL_OFF_T_C(1000) * size / limit);
   else {
     minimum = (time_t) (size / limit);
-    if(minimum < TIME_T_MAX/1000)
+    if(minimum < TIMEDIFF_T_MAX/1000)
       minimum *= 1000;
     else
-      minimum = TIME_T_MAX;
+      minimum = TIMEDIFF_T_MAX;
   }

   /*
diff --git a/lib/select.h b/lib/select.h
index 9a1ba45a7d6..f5652a74f7d 100644
--- a/lib/select.h
+++ b/lib/select.h
@@ -7,7 +7,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2017, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -77,9 +77,9 @@ int Curl_socket_check(curl_socket_t readfd, curl_socket_t readfd2,
                       time_t timeout_ms);

 #define SOCKET_READABLE(x,z) \
-  Curl_socket_check(x, CURL_SOCKET_BAD, CURL_SOCKET_BAD, z)
+  Curl_socket_check(x, CURL_SOCKET_BAD, CURL_SOCKET_BAD, (time_t)z)
 #define SOCKET_WRITABLE(x,z) \
-  Curl_socket_check(CURL_SOCKET_BAD, CURL_SOCKET_BAD, x, z)
+  Curl_socket_check(CURL_SOCKET_BAD, CURL_SOCKET_BAD, x, (time_t)z)

 int Curl_poll(struct pollfd ufds[], unsigned int nfds, int timeout_ms);

diff --git a/lib/timeval.c b/lib/timeval.c
index e2bd7fd143b..9b05cf05124 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -174,14 +174,6 @@ struct curltime Curl_now(void)

 #endif

-#if SIZEOF_TIME_T < 8
-#define TIME_MAX INT_MAX
-#define TIME_MIN INT_MIN
-#else
-#define TIME_MAX 9223372036854775807LL
-#define TIME_MIN -9223372036854775807LL
-#endif
-
 /*
  * Returns: time difference in number of milliseconds. For too large diffs it
  * returns max value.
@@ -191,10 +183,10 @@ struct curltime Curl_now(void)
 timediff_t Curl_timediff(struct curltime newer, struct curltime older)
 {
   timediff_t diff = (timediff_t)newer.tv_sec-older.tv_sec;
-  if(diff >= (TIME_MAX/1000))
-    return TIME_MAX;
-  else if(diff <= (TIME_MIN/1000))
-    return TIME_MIN;
+  if(diff >= (TIMEDIFF_T_MAX/1000))
+    return TIMEDIFF_T_MAX;
+  else if(diff <= (TIMEDIFF_T_MIN/1000))
+    return TIMEDIFF_T_MIN;
   return diff * 1000 + (newer.tv_usec-older.tv_usec)/1000;
 }

@@ -205,9 +197,9 @@ timediff_t Curl_timediff(struct curltime newer, struct curltime older)
 timediff_t Curl_timediff_us(struct curltime newer, struct curltime older)
 {
   timediff_t diff = (timediff_t)newer.tv_sec-older.tv_sec;
-  if(diff >= (TIME_MAX/1000000))
-    return TIME_MAX;
-  else if(diff <= (TIME_MIN/1000000))
-    return TIME_MIN;
+  if(diff >= (TIMEDIFF_T_MAX/1000000))
+    return TIMEDIFF_T_MAX;
+  else if(diff <= (TIMEDIFF_T_MIN/1000000))
+    return TIMEDIFF_T_MIN;
   return diff * 1000000 + newer.tv_usec-older.tv_usec;
 }
diff --git a/lib/timeval.h b/lib/timeval.h
index 96867d71390..53e063607cf 100644
--- a/lib/timeval.h
+++ b/lib/timeval.h
@@ -24,13 +24,13 @@

 #include "curl_setup.h"

-#if SIZEOF_TIME_T < 8
-typedef int timediff_t;
-#define CURL_FORMAT_TIMEDIFF_T "d"
-#else
+/* Use a larger type even for 32 bit time_t systems so that we can keep
+   microsecond accuracy in it */
 typedef curl_off_t timediff_t;
 #define CURL_FORMAT_TIMEDIFF_T CURL_FORMAT_CURL_OFF_T
-#endif
+
+#define TIMEDIFF_T_MAX CURL_OFF_T_MAX
+#define TIMEDIFF_T_MIN CURL_OFF_T_MIN

 struct curltime {
   time_t tv_sec; /* seconds */
diff --git a/lib/url.c b/lib/url.c
index d17016607ea..68cc5b23206 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -972,7 +972,8 @@ static int call_extract_if_dead(struct connectdata *conn, void *param)
 static void prune_dead_connections(struct Curl_easy *data)
 {
   struct curltime now = Curl_now();
-  time_t elapsed = Curl_timediff(now, data->state.conn_cache->last_cleanup);
+  timediff_t elapsed =
+    Curl_timediff(now, data->state.conn_cache->last_cleanup);

   if(elapsed >= 1000L) {
     struct prunedead prune;
diff --git a/lib/urldata.h b/lib/urldata.h
index 563ec9c6fab..b9daf12deba 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -909,8 +909,8 @@ struct connectdata {
   struct curltime connecttime;
   /* The two fields below get set in Curl_connecthost */
   int num_addr; /* number of addresses to try to connect to */
-  time_t timeoutms_per_addr; /* how long time in milliseconds to spend on
-                                trying to connect to each IP address */
+  timediff_t timeoutms_per_addr; /* how long time in milliseconds to spend on
+                                    trying to connect to each IP address */

   const struct Curl_handler *handler; /* Connection's protocol handler */
   const struct Curl_handler *given;   /* The protocol first given */
@@ -1108,17 +1108,17 @@ struct Progress {
   int width; /* screen width at download start */
   int flags; /* see progress.h */

-  time_t timespent;
+  timediff_t timespent;

   curl_off_t dlspeed;
   curl_off_t ulspeed;

-  time_t t_nslookup;
-  time_t t_connect;
-  time_t t_appconnect;
-  time_t t_pretransfer;
-  time_t t_starttransfer;
-  time_t t_redirect;
+  timediff_t t_nslookup;
+  timediff_t t_connect;
+  timediff_t t_appconnect;
+  timediff_t t_pretransfer;
+  timediff_t t_starttransfer;
+  timediff_t t_redirect;

   struct curltime start;
   struct curltime t_startsingle;
Bylon2 commented 4 years ago

Follow up: The patch above fixes the issue of curl 7.64 that is in the "stable" repo. See linked bug report at curl github.

Now awaiting from Raspbian Gurus to tell us if the right route is either to fix curl 7.64 or use curl 7.68 from "unstable".

XECDesign commented 2 years ago

Looks like bullseye is on 7.74 so hopefully this one has resolved itself.