From cc52bc45f625d285a3634fe28372d2db7854a614 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Wed, 15 Feb 2023 10:31:52 +0100 Subject: [PATCH] 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 --- lib/connect.c | 18 ++++++++++++------ tests/unit/unit2600.c | 24 +++++++++++++++++------- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/lib/connect.c b/lib/connect.c index 78aa47b8e9..993a7f919b 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -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, diff --git a/tests/unit/unit2600.c b/tests/unit/unit2600.c index ad5dbb9d15..1d5cf010b7 100644 --- a/tests/unit/unit2600.c +++ b/tests/unit/unit2600.c @@ -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 }; -- 2.47.2