connect: limit update IP info

Update IP related information at the connection and the transfer in two
places only: once the filter chain connects and when a transfer is added
to a connection. The latter only updates on reuse when the filters
already are connected.

The only user of that information before a full connect is the HAProxy
filter. Add cfilter CF_QUERY_IP_INFO query to let it find the
information from the filters "below".

This solves two issues with the previous version:
- updates where often done twice with the same info
- happy eyeballing filter "forks" could overwrite each others
  updates before the full winner was determined.

Closes #14699
This commit is contained in:
Stefan Eissing 2024-08-27 12:12:37 +02:00 committed by Daniel Stenberg
parent 87f0a79439
commit ea6f5c9f0f
No known key found for this signature in database
GPG Key ID: 5CC908FDB71E12C2
15 changed files with 122 additions and 77 deletions

View File

@ -70,8 +70,9 @@ static CURLcode cf_haproxy_date_out_set(struct Curl_cfilter*cf,
{
struct cf_haproxy_ctx *ctx = cf->ctx;
CURLcode result;
const char *tcp_version;
const char *client_ip;
struct ip_quadruple ipquad;
int is_ipv6;
DEBUGASSERT(ctx);
DEBUGASSERT(ctx->state == HAPROXY_INIT);
@ -81,19 +82,20 @@ static CURLcode cf_haproxy_date_out_set(struct Curl_cfilter*cf,
result = Curl_dyn_addn(&ctx->data_out, STRCONST("PROXY UNKNOWN\r\n"));
else {
#endif /* USE_UNIX_SOCKETS */
result = Curl_conn_cf_get_ip_info(cf->next, data, &is_ipv6, &ipquad);
if(result)
return result;
/* Emit the correct prefix for IPv6 */
tcp_version = cf->conn->bits.ipv6 ? "TCP6" : "TCP4";
if(data->set.str[STRING_HAPROXY_CLIENT_IP])
client_ip = data->set.str[STRING_HAPROXY_CLIENT_IP];
else
client_ip = data->info.primary.local_ip;
client_ip = ipquad.local_ip;
result = Curl_dyn_addf(&ctx->data_out, "PROXY %s %s %s %i %i\r\n",
tcp_version,
client_ip,
data->info.primary.remote_ip,
data->info.primary.local_port,
data->info.primary.remote_port);
is_ipv6? "TCP6" : "TCP4",
client_ip, ipquad.remote_ip,
ipquad.local_port, ipquad.remote_port);
#ifdef USE_UNIX_SOCKETS
}

View File

@ -210,8 +210,6 @@ static CURLcode baller_connected(struct Curl_cfilter *cf,
}
ctx->state = CF_HC_SUCCESS;
cf->connected = TRUE;
Curl_conn_cf_cntrl(cf->next, data, TRUE,
CF_CTRL_CONN_INFO_UPDATE, 0, NULL);
return result;
}

View File

@ -1614,6 +1614,18 @@ static ssize_t cf_socket_recv(struct Curl_cfilter *cf, struct Curl_easy *data,
return nread;
}
static void cf_socket_update_data(struct Curl_cfilter *cf,
struct Curl_easy *data)
{
/* Update the IP info held in the transfer, if we have that. */
if(cf->connected && (cf->sockindex == FIRSTSOCKET)) {
struct cf_socket_ctx *ctx = cf->ctx;
data->info.primary = ctx->ip;
/* not sure if this is redundant... */
data->info.conn_remote_port = cf->conn->remote_port;
}
}
static void cf_socket_active(struct Curl_cfilter *cf, struct Curl_easy *data)
{
struct cf_socket_ctx *ctx = cf->ctx;
@ -1621,17 +1633,15 @@ static void cf_socket_active(struct Curl_cfilter *cf, struct Curl_easy *data)
/* use this socket from now on */
cf->conn->sock[cf->sockindex] = ctx->sock;
set_local_ip(cf, data);
if(cf->sockindex == SECONDARYSOCKET)
cf->conn->secondary = ctx->ip;
else
cf->conn->primary = ctx->ip;
/* the first socket info gets some specials */
if(cf->sockindex == FIRSTSOCKET) {
cf->conn->primary = ctx->ip;
cf->conn->remote_addr = &ctx->addr;
#ifdef USE_IPV6
cf->conn->bits.ipv6 = (ctx->addr.family == AF_INET6)? TRUE : FALSE;
#endif
Curl_persistconninfo(data, cf->conn, &ctx->ip);
}
else {
cf->conn->secondary = ctx->ip;
}
ctx->active = TRUE;
}
@ -1647,9 +1657,10 @@ static CURLcode cf_socket_cntrl(struct Curl_cfilter *cf,
switch(event) {
case CF_CTRL_CONN_INFO_UPDATE:
cf_socket_active(cf, data);
cf_socket_update_data(cf, data);
break;
case CF_CTRL_DATA_SETUP:
Curl_persistconninfo(data, cf->conn, &ctx->ip);
cf_socket_update_data(cf, data);
break;
case CF_CTRL_FORGET_SOCKET:
ctx->sock = CURL_SOCKET_BAD;
@ -1732,6 +1743,10 @@ static CURLcode cf_socket_query(struct Curl_cfilter *cf,
}
return CURLE_OK;
}
case CF_QUERY_IP_INFO:
*pres1 = (ctx->addr.family == AF_INET6)? TRUE : FALSE;
*(struct ip_quadruple *)pres2 = ctx->ip;
return CURLE_OK;
default:
break;
}

View File

@ -45,6 +45,9 @@
#define ARRAYSIZE(A) (sizeof(A)/sizeof((A)[0]))
#endif
static void cf_cntrl_update_info(struct Curl_easy *data,
struct connectdata *conn);
#ifdef UNITTESTS
/* used by unit2600.c */
void Curl_cf_def_close(struct Curl_cfilter *cf, struct Curl_easy *data)
@ -428,7 +431,10 @@ CURLcode Curl_conn_connect(struct Curl_easy *data,
result = cf->cft->do_connect(cf, data, blocking, done);
if(!result && *done) {
Curl_conn_ev_update_info(data, data->conn);
/* Now that the complete filter chain is connected, let all filters
* persist information at the connection. E.g. cf-socket sets the
* socket and ip related information. */
cf_cntrl_update_info(data, data->conn);
conn_report_connect_stats(data, data->conn);
data->conn->keepalive = Curl_now();
}
@ -652,6 +658,15 @@ curl_socket_t Curl_conn_cf_get_socket(struct Curl_cfilter *cf,
return CURL_SOCKET_BAD;
}
CURLcode Curl_conn_cf_get_ip_info(struct Curl_cfilter *cf,
struct Curl_easy *data,
int *is_ipv6, struct ip_quadruple *ipquad)
{
if(cf)
return cf->cft->query(cf, data, CF_QUERY_IP_INFO, is_ipv6, ipquad);
return CURLE_UNKNOWN_OPTION;
}
curl_socket_t Curl_conn_get_socket(struct Curl_easy *data, int sockindex)
{
struct Curl_cfilter *cf;
@ -749,7 +764,7 @@ CURLcode Curl_conn_ev_data_pause(struct Curl_easy *data, bool do_pause)
CF_CTRL_DATA_PAUSE, do_pause, NULL);
}
void Curl_conn_ev_update_info(struct Curl_easy *data,
static void cf_cntrl_update_info(struct Curl_easy *data,
struct connectdata *conn)
{
cf_cntrl_all(conn, data, TRUE, CF_CTRL_CONN_INFO_UPDATE, 0, NULL);

View File

@ -30,6 +30,7 @@ struct Curl_cfilter;
struct Curl_easy;
struct Curl_dns_entry;
struct connectdata;
struct ip_quadruple;
/* Callback to destroy resources held by this filter instance.
* Implementations MUST NOT chain calls to cf->next.
@ -165,6 +166,8 @@ typedef CURLcode Curl_cft_cntrl(struct Curl_cfilter *cf,
* -1 if not determined yet.
* - CF_QUERY_SOCKET: the socket used by the filter chain
* - CF_QUERY_NEED_FLUSH: TRUE iff any of the filters have unsent data
* - CF_QUERY_IP_INFO: res1 says if connection used IPv6, res2 is the
* ip quadruple
*/
/* query res1 res2 */
#define CF_QUERY_MAX_CONCURRENT 1 /* number - */
@ -174,6 +177,7 @@ typedef CURLcode Curl_cft_cntrl(struct Curl_cfilter *cf,
#define CF_QUERY_TIMER_APPCONNECT 5 /* - struct curltime */
#define CF_QUERY_STREAM_ERROR 6 /* error code - */
#define CF_QUERY_NEED_FLUSH 7 /* TRUE/FALSE - */
#define CF_QUERY_IP_INFO 8 /* TRUE/FALSE struct ip_quadruple */
/**
* Query the cfilter for properties. Filters ignorant of a query will
@ -344,6 +348,10 @@ bool Curl_conn_cf_is_ssl(struct Curl_cfilter *cf);
curl_socket_t Curl_conn_cf_get_socket(struct Curl_cfilter *cf,
struct Curl_easy *data);
CURLcode Curl_conn_cf_get_ip_info(struct Curl_cfilter *cf,
struct Curl_easy *data,
int *is_ipv6, struct ip_quadruple *ipquad);
bool Curl_conn_cf_needs_flush(struct Curl_cfilter *cf,
struct Curl_easy *data);
@ -515,12 +523,6 @@ void Curl_conn_ev_data_done(struct Curl_easy *data, bool premature);
*/
CURLcode Curl_conn_ev_data_pause(struct Curl_easy *data, bool do_pause);
/**
* Inform connection filters to update their info in `conn`.
*/
void Curl_conn_ev_update_info(struct Curl_easy *data,
struct connectdata *conn);
/**
* Check if FIRSTSOCKET's cfilter chain deems connection alive.
*/

View File

@ -208,31 +208,6 @@ bool Curl_shutdown_started(struct Curl_easy *data, int sockindex)
return (pt->tv_sec > 0) || (pt->tv_usec > 0);
}
/* Copies connection info into the transfer handle to make it available when
the transfer handle is no longer associated with the connection. */
void Curl_persistconninfo(struct Curl_easy *data, struct connectdata *conn,
struct ip_quadruple *ip)
{
if(ip)
data->info.primary = *ip;
else {
memset(&data->info.primary, 0, sizeof(data->info.primary));
data->info.primary.remote_port = -1;
data->info.primary.local_port = -1;
}
data->info.conn_scheme = conn->handler->scheme;
/* conn_protocol can only provide "old" protocols */
data->info.conn_protocol = (conn->handler->protocol) & CURLPROTO_MASK;
data->info.conn_remote_port = conn->remote_port;
data->info.used_proxy =
#ifdef CURL_DISABLE_PROXY
0
#else
conn->bits.proxy
#endif
;
}
static const struct Curl_addrinfo *
addr_first_match(const struct Curl_addrinfo *addr, int family)
{
@ -992,8 +967,6 @@ static CURLcode cf_he_connect(struct Curl_cfilter *cf,
cf->next = ctx->winner->cf;
ctx->winner->cf = NULL;
cf_he_ctx_clear(cf, data);
Curl_conn_cf_cntrl(cf->next, data, TRUE,
CF_CTRL_CONN_INFO_UPDATE, 0, NULL);
if(cf->conn->handler->protocol & PROTO_FAMILY_SSH)
Curl_pgrsTime(data, TIMER_APPCONNECT); /* we are connected already */

View File

@ -72,9 +72,6 @@ curl_socket_t Curl_getconnectinfo(struct Curl_easy *data,
bool Curl_addr2string(struct sockaddr *sa, curl_socklen_t salen,
char *addr, int *port);
void Curl_persistconninfo(struct Curl_easy *data, struct connectdata *conn,
struct ip_quadruple *ip);
/*
* Curl_conncontrol() marks the end of a connection/stream. The 'closeit'
* argument specifies if it is the end of a connection or a stream.

View File

@ -2082,7 +2082,6 @@ static CURLcode ftp_state_pasv_resp(struct Curl_easy *data,
/* postponed address resolution in case of tcp fastopen */
if(conn->bits.tcp_fastopen && !conn->bits.reuse && !ftpc->newhost[0]) {
Curl_conn_ev_update_info(data, conn);
Curl_safefree(ftpc->newhost);
ftpc->newhost = strdup(control_address(conn));
if(!ftpc->newhost)

View File

@ -77,10 +77,9 @@ CURLcode Curl_initinfo(struct Curl_easy *data)
free(info->wouldredirect);
info->wouldredirect = NULL;
info->primary.remote_ip[0] = '\0';
info->primary.local_ip[0] = '\0';
info->primary.remote_port = 0;
info->primary.local_port = 0;
memset(&info->primary, 0, sizeof(info->primary));
info->primary.remote_port = -1;
info->primary.local_port = -1;
info->retry_after = 0;
info->conn_scheme = 0;

View File

@ -3434,7 +3434,9 @@ static CURLcode create_conn(struct Curl_easy *data,
/* this is supposed to be the connect function so we better at least check
that the file is present here! */
DEBUGASSERT(conn->handler->connect_it);
Curl_persistconninfo(data, conn, NULL);
data->info.conn_scheme = conn->handler->scheme;
/* conn_protocol can only provide "old" protocols */
data->info.conn_protocol = (conn->handler->protocol) & CURLPROTO_MASK;
result = conn->handler->connect_it(data, &done);
/* Setup a "faked" transfer that will do nothing */
@ -3630,9 +3632,20 @@ static CURLcode create_conn(struct Curl_easy *data,
goto out;
}
/* persist the scheme and handler the transfer is using */
data->info.conn_scheme = conn->handler->scheme;
/* conn_protocol can only provide "old" protocols */
data->info.conn_protocol = (conn->handler->protocol) & CURLPROTO_MASK;
data->info.used_proxy =
#ifdef CURL_DISABLE_PROXY
0
#else
conn->bits.proxy
#endif
;
/* Everything general done, inform filters that they need
* to prepare for a data transfer.
*/
* to prepare for a data transfer. */
result = Curl_conn_ev_data_setup(data);
out:

View File

@ -94,7 +94,9 @@ class TestBasic:
curl = CurlClient(env=env)
url = f'https://{env.authority_for(env.domain1, proto)}/data.json'
r = curl.http_download(urls=[url], alpn_proto=proto, with_stats=True)
r.check_stats(http_status=200, count=1)
r.check_stats(http_status=200, count=1,
remote_port=env.port_for(alpn_proto=proto),
remote_ip='127.0.0.1')
assert r.stats[0]['time_connect'] > 0, f'{r.stats[0]}'
assert r.stats[0]['time_appconnect'] > 0, f'{r.stats[0]}'
@ -108,7 +110,9 @@ class TestBasic:
url = f'https://{env.authority_for(env.domain1, proto)}/data.json'
r = curl.http_download(urls=[url], with_stats=True, with_headers=True,
extra_args=['-I'])
r.check_stats(http_status=200, count=1, exitcode=0)
r.check_stats(http_status=200, count=1, exitcode=0,
remote_port=env.port_for(alpn_proto=proto),
remote_ip='127.0.0.1')
# got the Conten-Length: header, but did not download anything
assert r.responses[0]['header']['content-length'] == '30', f'{r.responses[0]}'
assert r.stats[0]['size_download'] == 0, f'{r.stats[0]}'

View File

@ -272,7 +272,9 @@ class TestDownload:
r = curl.http_download(urls=[urln], alpn_proto=proto, extra_args=[
'--parallel'
])
r.check_stats(count=count, http_status=404, exitcode=0)
r.check_stats(count=count, http_status=404, exitcode=0,
remote_port=env.port_for(alpn_proto=proto),
remote_ip='127.0.0.1')
@pytest.mark.parametrize("proto", ['h2', 'h3'])
def test_02_15_fail_not_found(self, env: Env, httpd, nghttpx, repeat, proto):
@ -286,7 +288,9 @@ class TestDownload:
r = curl.http_download(urls=[urln], alpn_proto=proto, extra_args=[
'--fail'
])
r.check_stats(count=count, http_status=404, exitcode=22)
r.check_stats(count=count, http_status=404, exitcode=22,
remote_port=env.port_for(alpn_proto=proto),
remote_ip='127.0.0.1')
@pytest.mark.skipif(condition=Env().slow_network, reason="not suitable for slow network tests")
@pytest.mark.skipif(condition=Env().ci_run, reason="not suitable for CI runs")

View File

@ -62,7 +62,9 @@ class TestInfo:
curl = CurlClient(env=env)
url = f'https://{env.authority_for(env.domain1, proto)}/data.json?[0-{count-1}]'
r = curl.http_download(urls=[url], alpn_proto=proto, with_stats=True)
r.check_stats(count=count, http_status=200, exitcode=0)
r.check_stats(count=count, http_status=200, exitcode=0,
remote_port=env.port_for(alpn_proto=proto),
remote_ip='127.0.0.1')
for idx, s in enumerate(r.stats):
self.check_stat(idx, s, r, dl_size=30, ul_size=0)
@ -77,7 +79,9 @@ class TestInfo:
r = curl.http_download(urls=[url], alpn_proto=proto, with_stats=True, extra_args=[
'--location'
])
r.check_stats(count=count, http_status=200, exitcode=0)
r.check_stats(count=count, http_status=200, exitcode=0,
remote_port=env.port_for(alpn_proto=proto),
remote_ip='127.0.0.1')
for idx, s in enumerate(r.stats):
self.check_stat(idx, s, r, dl_size=30, ul_size=0)
@ -95,7 +99,9 @@ class TestInfo:
'--trace-config', 'http/2,http/3'
])
r.check_response(count=count, http_status=200)
r.check_stats(count=count, http_status=200, exitcode=0)
r.check_stats(count=count, http_status=200, exitcode=0,
remote_port=env.port_for(alpn_proto=proto),
remote_ip='127.0.0.1')
for idx, s in enumerate(r.stats):
self.check_stat(idx, s, r, dl_size=fsize, ul_size=fsize)
@ -106,7 +112,8 @@ class TestInfo:
curl = CurlClient(env=env)
url = f'http://{env.domain1}:{env.http_port}/data.json?[0-{count-1}]'
r = curl.http_download(urls=[url], alpn_proto=proto, with_stats=True)
r.check_stats(count=count, http_status=200, exitcode=0)
r.check_stats(count=count, http_status=200, exitcode=0,
remote_port=env.http_port, remote_ip='127.0.0.1')
for idx, s in enumerate(r.stats):
self.check_stat(idx, s, r, dl_size=30, ul_size=0)

View File

@ -390,7 +390,9 @@ class ExecResult:
f'were made\n{self.dump_logs()}'
def check_stats(self, count: int, http_status: Optional[int] = None,
exitcode: Optional[int] = None):
exitcode: Optional[int] = None,
remote_port: Optional[int] = None,
remote_ip: Optional[str] = None):
if exitcode is None:
self.check_exit_code(0)
assert len(self.stats) == count, \
@ -408,6 +410,18 @@ class ExecResult:
assert x['exitcode'] == exitcode, \
f'status #{idx} exitcode: expected {exitcode}, '\
f'got {x["exitcode"]}\n{self.dump_stat(x)}'
if remote_port is not None:
for idx, x in enumerate(self.stats):
assert 'remote_port' in x, f'remote_port missing\n{self.dump_stat(x)}'
assert x['remote_port'] == remote_port, \
f'status #{idx} remote_port: expected {remote_port}, '\
f'got {x["remote_port"]}\n{self.dump_stat(x)}'
if remote_ip is not None:
for idx, x in enumerate(self.stats):
assert 'remote_ip' in x, f'remote_ip missing\n{self.dump_stat(x)}'
assert x['remote_ip'] == remote_ip, \
f'status #{idx} remote_ip: expected {remote_ip}, '\
f'got {x["remote_ip"]}\n{self.dump_stat(x)}'
def dump_logs(self):
lines = ['>>--stdout ----------------------------------------------\n']

View File

@ -552,13 +552,16 @@ class Env:
def ci_run(self) -> bool:
return "CURL_CI" in os.environ
def authority_for(self, domain: str, alpn_proto: Optional[str] = None):
def port_for(self, alpn_proto: Optional[str] = None):
if alpn_proto is None or \
alpn_proto in ['h2', 'http/1.1', 'http/1.0', 'http/0.9']:
return f'{domain}:{self.https_port}'
return self.https_port
if alpn_proto in ['h3']:
return f'{domain}:{self.h3_port}'
return f'{domain}:{self.http_port}'
return self.h3_port
return self.http_port
def authority_for(self, domain: str, alpn_proto: Optional[str] = None):
return f'{domain}:{self.port_for(alpn_proto=alpn_proto)}'
def make_data_file(self, indir: str, fname: str, fsize: int) -> str:
fpath = os.path.join(indir, fname)