Skip to content

Commit

Permalink
Curl_timeleft: check both timeouts during connect
Browse files Browse the repository at this point in the history
The duration of a connect and the total transfer are calculated from two
different time-stamps. It can end up with the total timeout triggering
before the connect timeout expires and we should make sure to
acknowledge whichever timeout that is reached first.

This is especially notable when a transfer first sits in PENDING, as
that time is counted in the total time but the connect timeout is based
on the time since the handle changed to the CONNECT state.

The CONNECTTIMEOUT is per connect attempt. The TIMEOUT is for the entire
operation.

Fixes #6744
Reported-by: Andrei Bica
  • Loading branch information
bagder committed Mar 16, 2021
1 parent f83d4ea commit a8e27f1
Showing 1 changed file with 42 additions and 40 deletions.
82 changes: 42 additions & 40 deletions lib/connect.c
Expand Up @@ -171,65 +171,67 @@ singleipconnect(struct Curl_easy *data,
* infinite time left). If the value is negative, the timeout time has already
* elapsed.
*
* The start time is stored in progress.t_startsingle - as set with
* Curl_pgrsTime(..., TIMER_STARTSINGLE);
*
* If 'nowp' is non-NULL, it points to the current time.
* 'duringconnect' is FALSE if not during a connect, as then of course the
* connect timeout is not taken into account!
*
* @unittest: 1303
*/

#define TIMEOUT_CONNECT 1
#define TIMEOUT_MAXTIME 2

timediff_t Curl_timeleft(struct Curl_easy *data,
struct curltime *nowp,
bool duringconnect)
{
int timeout_set = 0;
timediff_t timeout_ms = duringconnect?DEFAULT_CONNECT_TIMEOUT:0;
unsigned int timeout_set = 0;
timediff_t connect_timeout_ms = 0;
timediff_t maxtime_timeout_ms = 0;
timediff_t timeout_ms = 0;
struct curltime now;

/* if a timeout is set, use the most restrictive one */

if(data->set.timeout > 0)
timeout_set |= 1;
if(duringconnect && (data->set.connecttimeout > 0))
timeout_set |= 2;

switch(timeout_set) {
case 1:
timeout_ms = data->set.timeout;
break;
case 2:
timeout_ms = data->set.connecttimeout;
break;
case 3:
if(data->set.timeout < data->set.connecttimeout)
timeout_ms = data->set.timeout;
else
timeout_ms = data->set.connecttimeout;
break;
default:
/* use the default */
if(!duringconnect)
/* if we're not during connect, there's no default timeout so if we're
at zero we better just return zero and not make it a negative number
by the math below */
return 0;
break;
/* 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 > 0) {
timeout_set = TIMEOUT_MAXTIME;
maxtime_timeout_ms = data->set.timeout;
}
if(duringconnect) {
timeout_set |= TIMEOUT_CONNECT;
connect_timeout_ms = (data->set.connecttimeout > 0) ?
data->set.connecttimeout : DEFAULT_CONNECT_TIMEOUT;
}
if(!timeout_set)
/* no timeout */
return 0;

if(!nowp) {
now = Curl_now();
nowp = &now;
}

/* subtract elapsed time */
if(duringconnect)
/* since this most recent connect started */
timeout_ms -= Curl_timediff(*nowp, data->progress.t_startsingle);
else
/* since the entire operation started */
timeout_ms -= Curl_timediff(*nowp, data->progress.t_startop);
if(timeout_set & TIMEOUT_MAXTIME) {
maxtime_timeout_ms -= Curl_timediff(*nowp, data->progress.t_startop);
timeout_ms = maxtime_timeout_ms;
}

if(timeout_set & TIMEOUT_CONNECT) {
connect_timeout_ms -= Curl_timediff(*nowp, data->progress.t_startsingle);

if(timeout_set & TIMEOUT_MAXTIME) {
/* when both timeouts are set, use the lowest value */
if(connect_timeout_ms < maxtime_timeout_ms)
timeout_ms = connect_timeout_ms;
}
else
timeout_ms = connect_timeout_ms;
}

if(!timeout_ms)
/* avoid returning 0 as that means no timeout! */
return -1;
Expand Down

0 comments on commit a8e27f1

Please sign in to comment.