Hi Mark, On Mon, Jun 15, 2026 at 5:50 AM Mark Wielaard <[email protected]> wrote: > 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.
Thanks, merged. > > > 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. configure.ac checks for libcurl >= 7.29.0 so we are good here. Aaron > > > 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 >
