Hi Aaron, On Sun, Jun 14, 2026 at 10:56:26PM -0400, Aaron Merey wrote: > If the $DEBUGINFOD_TIMEOUT environment variable is set then it is applied > to CURLOPT_LOW_SPEED_TIME/_LIMIT in init_handle(). This causes a server > to be skipped if it does not transfer 100K of data within the given timeout. > > CURLOPT_LOW_SPEED_TIME/_LIMIT begins tracking this timeout only after the > connection to a server has been established. This results in > $DEBUGINFOD_TIMEOUT failing to be enforced if a server is unresponsive > when attempting to establish connection. > > Fix this by also setting CURLOPT_CONNECTTIMEOUT_MS with the > $DEBUGINFOD_TIMEOUT as well. > > CURLOPT_CONNECTTIMEOUT_MS uses type long. If converting the timeout to > milliseconds will exceed LONG_MAX, then set CURLOPT_CONNECTTIMEOUT_MS to > LONG_MAX.
This looks good to me. > https://sourceware.org/bugzilla/show_bug.cgi?id=34158 > > Signed-off-by: Aaron Merey <[email protected]> > --- > debuginfod/debuginfod-client.c | 12 +++++++++++- > doc/debuginfod-client-config.7 | 7 ++++--- > 2 files changed, 15 insertions(+), 4 deletions(-) > > diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c > index f2b82ac7..04c4a0eb 100644 > --- a/debuginfod/debuginfod-client.c > +++ b/debuginfod/debuginfod-client.c > @@ -233,7 +233,9 @@ static const char *cache_xdg_name = "debuginfod_client"; > /* URLs of debuginfods, separated by url_delim. */ > static const char *url_delim = " "; > > -/* Timeout for debuginfods, in seconds (to get at least 100K). */ > +/* Timeout for debuginfods, in seconds. Applies separately to the server > + connect phase and to download at least 100K once connection is > + established. */ > static const long default_timeout = 90; Thanks for updating the comment. > /* Default retry count for download error. */ > @@ -1005,6 +1007,14 @@ init_handle(debuginfod_client *client, > timeout); > curl_easy_setopt_ck (data->handle, CURLOPT_LOW_SPEED_LIMIT, > 100 * 1024L); > + > + /* CURLOPT_LOW_SPEED_LIMIT/_TIME don't apply during the server > connection > + phase. Set CURLOPT_CONNECTTIMEOUT_MS as well so the timeout also > + limits how long we wait for an unresponsive server. */ > + long connect_timeout_ms = (timeout > LONG_MAX / 1000L > + ? LONG_MAX : timeout * 1000L); > + curl_easy_setopt_ck (data->handle, CURLOPT_CONNECTTIMEOUT_MS, > + connect_timeout_ms); > } Nice overflow protection. This is wrapped in an if (timeout > 0) so no zero or negative values. What is our minimum libcurl support? CURLOPT_CONNECTTIMEOUT_MS was introduced in 7.16.2 - April 11 2007. Debian old-old-stable has 7.88 and alma8 has 7.61.1 so we are probably good. > diff --git a/doc/debuginfod-client-config.7 b/doc/debuginfod-client-config.7 > index 708601e5..19bbae10 100644 > --- a/doc/debuginfod-client-config.7 > +++ b/doc/debuginfod-client-config.7 > @@ -85,9 +85,10 @@ within the limit. > .TP > .B $DEBUGINFOD_TIMEOUT > This environment variable governs the download \fIcommencing\fP > -timeout for each debuginfod HTTP connection. A server that fails to > -provide at least 100K of data within this many seconds is skipped. The > -default is 90 seconds. (Zero or negative means "no timeout".) > +timeout, in seconds, for each debuginfod HTTP connection. A server is > +skipped if it either fails to establish a connection within the timeout or > +fails to provide at least 100K of data within the timeout. The default is > +90 seconds. (Zero or negative means "no timeout".) Nice doc update. Thanks, Mark
