Re: [PATCH] http: add a config to limit the connection time
From: Junio C Hamano <hidden>
Date: 2026-07-23 16:47:40
"GalaxySnail via GitGitGadget" [off-list ref] writes:
From: GalaxySnail <redacted> By default, libcurl uses a 300 seconds timeout for the connection phase, which is too long for some use cases.
Can you elaborate a bit more on the use cases in which you want to try connecting to an unreachable host yet want to give up on it very fast?
Add http.connecttimeoutms and GIT_HTTP_CONNECT_TIMEOUT_MS to specify timeout in milliseconds for the connection phase. Both of them call CURLOPT_CONNECTTIMEOUT_MS internally. Signed-off-by: GalaxySnail <redacted>
Documentation/SubmittingPatches:[[real-name]] applies here.
quoted hunk ↗ jump to hunk
Documentation/config/http.adoc | 7 ++++ http.c | 11 ++++++ t/meson.build | 1 + t/t5585-http-connect-timeout.sh | 60 +++++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+) create mode 100755 t/t5585-http-connect-timeout.shdiff --git a/Documentation/config/http.adoc b/Documentation/config/http.adoc index 792a71b413..a4f7afa61e 100644 --- a/Documentation/config/http.adoc +++ b/Documentation/config/http.adoc@@ -300,6 +300,13 @@ for most push problems, but can increase memory consumption significantly since the entire buffer is allocated even for small pushes. +http.connectTimeoutMS:: + Maximum time in milliseconds that you allow the connection phase + to take. The connection phase includes DNS lookup and subsequent + TCP, TLS or QUIC handshakes. + Can be overridden by the `GIT_HTTP_CONNECT_TIMEOUT_MS` + environment variable.
Once a knob is provided, users will want to know what value is used when unspecified, so they can gauge what a reasonable value to set would be.
quoted hunk ↗ jump to hunk
diff --git a/http.c b/http.c index caccf2108e..befe9ea8a0 100644 --- a/http.c +++ b/http.c@@ -68,6 +68,7 @@ static char *ssl_capath; static char *curl_no_proxy; static char *ssl_pinnedkey; static char *ssl_cainfo; +static long curl_connect_timeout_ms = -1; static long curl_low_speed_limit = -1; static long curl_low_speed_time = -1; static int curl_ftp_no_epsv;@@ -450,6 +451,10 @@ static int http_options(const char *var, const char *value, max_requests = git_config_int(var, value, ctx->kvi); return 0; } + if (!strcmp("http.connecttimeoutms", var)) { + curl_connect_timeout_ms = git_config_int(var, value, ctx->kvi); + return 0; + }
We could set it to -1 if we wanted to, and behave as if no configuration variable were given. That may be reasonable, but it should be documented.
quoted hunk ↗ jump to hunk
@@ -1215,6 +1220,10 @@ static CURL *get_curl_handle(void) curl_easy_setopt(result, CURLOPT_PROXY_CAINFO, http_proxy_ssl_ca_info); } + if (curl_connect_timeout_ms > 0) + curl_easy_setopt(result, CURLOPT_CONNECTTIMEOUT_MS, + curl_connect_timeout_ms);
This code silently ignores setting the configuration variable to 0. To the cURL library, however, passing a value of 0 to CURLOPT_CONNECTTIMEOUT_MS signals that it should use the default value (300s). Perhaps we should tweak the above to if (0 <= curlopt_connecttimeout_ms) curl_easy_setopt(result, CURLOPT_CONNECTTIMEOUT_MS, curl_connect_timeout_ms); and then document what 0 means.
quoted hunk ↗ jump to hunk
@@ -1474,6 +1483,8 @@ void http_init(struct remote *remote, const char *url, int proactive_auth) set_from_env(&user_agent, "GIT_HTTP_USER_AGENT"); + set_long_from_env(&curl_connect_timeout_ms, "GIT_HTTP_CONNECT_TIMEOUT_MS"); + set_long_from_env(&curl_low_speed_limit, "GIT_HTTP_LOW_SPEED_LIMIT"); set_long_from_env(&curl_low_speed_time, "GIT_HTTP_LOW_SPEED_TIME");
This, along with other environment variables, is processed after repo_config() collects configured values by triggering the http_options() callback, so the environment overrides the configured value, as expected.
quoted hunk ↗ jump to hunk
diff --git a/t/t5585-http-connect-timeout.sh b/t/t5585-http-connect-timeout.sh new file mode 100755 index 0000000000..7363e23bfe --- /dev/null +++ b/t/t5585-http-connect-timeout.sh@@ -0,0 +1,60 @@ +#!/bin/sh + +test_description='test http.connecttimeoutms and GIT_HTTP_CONNECT_TIMEOUT_MS' + +. ./test-lib.sh +. "$TEST_DIRECTORY"/lib-httpd.sh +start_httpd
What are we testing with this new script, really? As far as I can see, nobody is sitting next to the running test with a stopwatch to ensure that the client times out as specified. Should we really consume a limited shared resource, the four-digit test number, for this instead of adding a few "not a number (should fail to parse)" tests to existing http tests? Thanks.