connnect: fix timeout handling to use full duration

- connect timeout was used at half the configured value, if the
  destination had 1 ip version 4 and other version 6 addresses
  (or the other way around)
- extended test2600 to reproduce these cases

Reported-by: Michael Kaufmann
Fixes #10514
Closes #10517
This commit is contained in:
Stefan Eissing 2023-02-15 10:31:52 +01:00 committed by Daniel Stenberg
parent ce24ea373f
commit cc52bc45f6
No known key found for this signature in database
GPG Key ID: 5CC908FDB71E12C2
2 changed files with 29 additions and 13 deletions

View File

@ -186,7 +186,7 @@ addr_first_match(const struct Curl_addrinfo *addr, int family)
static const struct Curl_addrinfo *
addr_next_match(const struct Curl_addrinfo *addr, int family)
{
while(addr->ai_next) {
while(addr && addr->ai_next) {
addr = addr->ai_next;
if(addr->ai_family == family)
return addr;
@ -406,7 +406,8 @@ static CURLcode eyeballer_new(struct eyeballer **pballer,
baller->ai_family = ai_family;
baller->primary = primary;
baller->delay_ms = delay_ms;
baller->timeoutms = (addr && addr->ai_next)? timeout_ms / 2 : timeout_ms;
baller->timeoutms = addr_next_match(baller->addr, baller->ai_family)?
timeout_ms / 2 : timeout_ms;
baller->timeout_id = timeout_id;
baller->result = CURLE_COULDNT_CONNECT;
@ -467,7 +468,7 @@ static void baller_initiate(struct Curl_cfilter *cf,
wcf->sockindex = cf->sockindex;
}
if(baller->addr && baller->addr->ai_next) {
if(addr_next_match(baller->addr, baller->ai_family)) {
Curl_expire(data, baller->timeoutms, baller->timeout_id);
}
@ -498,8 +499,8 @@ static CURLcode baller_start(struct Curl_cfilter *cf,
while(baller->addr) {
baller->started = Curl_now();
baller->timeoutms = (baller->addr->ai_next == NULL) ?
timeoutms : timeoutms / 2;
baller->timeoutms = addr_next_match(baller->addr, baller->ai_family) ?
timeoutms / 2 : timeoutms;
baller_initiate(cf, data, baller);
if(!baller->result)
break;
@ -662,7 +663,8 @@ evaluate:
DEBUGF(LOG_CF(data, cf, "%s done", baller->name));
}
else {
DEBUGF(LOG_CF(data, cf, "%s starting", baller->name));
DEBUGF(LOG_CF(data, cf, "%s starting (timeout=%ldms)",
baller->name, baller->timeoutms));
++ongoing;
++added;
}
@ -799,6 +801,8 @@ static CURLcode start_connect(struct Curl_cfilter *cf,
timeout_ms, EXPIRE_DNS_PER_NAME);
if(result)
return result;
DEBUGF(LOG_CF(data, cf, "created %s (timeout %ldms)",
ctx->baller[0]->name, ctx->baller[0]->timeoutms));
if(addr1) {
/* second one gets a delayed start */
result = eyeballer_new(&ctx->baller[1], ctx->cf_create, addr1, ai_family1,
@ -808,6 +812,8 @@ static CURLcode start_connect(struct Curl_cfilter *cf,
timeout_ms, EXPIRE_DNS_PER_NAME2);
if(result)
return result;
DEBUGF(LOG_CF(data, cf, "created %s (timeout %ldms)",
ctx->baller[1]->name, ctx->baller[1]->timeoutms));
}
Curl_expire(data, data->set.happy_eyeballs_timeout,

View File

@ -56,6 +56,7 @@ struct test_case {
int id;
const char *url;
const char *resolve_info;
unsigned char ip_version;
timediff_t connect_timeout_ms;
timediff_t he_timeout_ms;
timediff_t cf4_fail_delay_ms;
@ -270,6 +271,7 @@ static void test_connect(struct test_case *tc)
list = curl_slist_append(NULL, tc->resolve_info);
fail_unless(list, "error allocating resolve list entry");
curl_easy_setopt(easy, CURLOPT_RESOLVE, list);
curl_easy_setopt(easy, CURLOPT_IPRESOLVE, (long)tc->ip_version);
curl_easy_setopt(easy, CURLOPT_CONNECTTIMEOUT_MS,
(long)tc->connect_timeout_ms);
curl_easy_setopt(easy, CURLOPT_HAPPY_EYEBALLS_TIMEOUT_MS,
@ -314,29 +316,37 @@ static void test_connect(struct test_case *tc)
static struct test_case TEST_CASES[] = {
/* TIMEOUT_MS, FAIL_MS CREATED DURATION Result, HE_PREF */
/* CNCT HE v4 v6 v4 v6 MIN MAX */
{ 1, TURL, "test.com:123:192.0.2.1",
{ 1, TURL, "test.com:123:192.0.2.1", CURL_IPRESOLVE_WHATEVER,
250, 150, 200, 200, 1, 0, 200, 500, R_FAIL, NULL },
/* 1 ipv4, fails after ~200ms, reports COULDNT_CONNECT */
{ 2, TURL, "test.com:123:192.0.2.1,192.0.2.2",
{ 2, TURL, "test.com:123:192.0.2.1,192.0.2.2", CURL_IPRESOLVE_WHATEVER,
500, 150, 200, 200, 2, 0, 400, 800, R_FAIL, NULL },
/* 2 ipv4, fails after ~400ms, reports COULDNT_CONNECT */
#ifdef ENABLE_IPV6
{ 3, TURL, "test.com:123:::1",
{ 3, TURL, "test.com:123:::1", CURL_IPRESOLVE_WHATEVER,
250, 150, 200, 200, 0, 1, 200, 500, R_FAIL, NULL },
/* 1 ipv6, fails after ~200ms, reports COULDNT_CONNECT */
{ 4, TURL, "test.com:123:::1,::2",
{ 4, TURL, "test.com:123:::1,::2", CURL_IPRESOLVE_WHATEVER,
500, 150, 200, 200, 0, 2, 400, 800, R_FAIL, NULL },
/* 2 ipv6, fails after ~400ms, reports COULDNT_CONNECT */
{ 5, TURL, "test.com:123:192.0.2.1,::1",
{ 5, TURL, "test.com:123:192.0.2.1,::1", CURL_IPRESOLVE_WHATEVER,
500, 150, 200, 200, 1, 1, 350, 800, R_FAIL, "v4" },
/* mixed ip4+6, v4 starts, v6 kicks in on HE, fails after ~350ms */
{ 6, TURL, "test.com:123:::1,192.0.2.1",
{ 6, TURL, "test.com:123:::1,192.0.2.1", CURL_IPRESOLVE_WHATEVER,
500, 150, 200, 200, 1, 1, 350, 800, R_FAIL, "v6" },
/* mixed ip6+4, v6 starts, v4 kicks in on HE, fails after ~350ms */
{ 7, TURL, "test.com:123:::1,192.0.2.1,::2,::3",
{ 7, TURL, "test.com:123:::1,192.0.2.1,::2,::3", CURL_IPRESOLVE_WHATEVER,
500, 600, 200, 200, 0, 3, 350, 800, R_TIME, "v6" },
/* mixed ip6+4, v6 starts, v4 never starts due to high HE, TIMEOUT */
{ 8, TURL, "test.com:123:192.0.2.1,::1", CURL_IPRESOLVE_V4,
400, 150, 500, 500, 1, 0, 400, 600, R_FAIL, NULL },
/* mixed ip4+6, but only use v4, check it uses full connect timeout,
although another address of the 'wrong' family is availbale */
{ 9, TURL, "test.com:123:::1,192.0.2.1", CURL_IPRESOLVE_V6,
400, 150, 500, 500, 0, 1, 400, 600, R_FAIL, NULL },
/* mixed ip4+6, but only use v6, check it uses full connect timeout,
although another address of the 'wrong' family is availbale */
#endif
};