* [PATCH] http: accept https:// proxies again
@ 2026-06-27 17:17 Johannes Schindelin via GitGitGadget
2026-06-28 1:54 ` Junio C Hamano
0 siblings, 1 reply; 3+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-06-27 17:17 UTC (permalink / raw)
To: git; +Cc: Aliwoto, Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Since 663d7abe07ea (http: reject unsupported proxy URL schemes,
2026-05-05), set_curl_proxy_type() returns 0 only for the "http"
and SOCKS variants via dedicated early returns, and -1 for
everything else. The "https" branch configures the CURL handle for
HTTPS proxying but then falls through to the trailing `return -1`
intended for unknown schemes, so the caller in get_curl_handle()
treats a perfectly valid https:// proxy URL as unsupported and
refuses to use it.
Noticed while looking into a Coverity report against the same
function; the unchecked curl_easy_setopt() return values it flags
are orthogonal to this fix.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
http: accept https:// proxies again
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2161%2Fdscho%2Ffix-bug-in-validate-proxy-url-scheme-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2161/dscho/fix-bug-in-validate-proxy-url-scheme-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2161
http.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/http.c b/http.c
index 8e5a4d8bcf..8c0f831365 100644
--- a/http.c
+++ b/http.c
@@ -802,6 +802,8 @@ static int set_curl_proxy_type(CURL *result, const char *protocol)
if (has_proxy_cert_password())
curl_easy_setopt(result, CURLOPT_PROXY_KEYPASSWD,
proxy_cert_auth.password);
+
+ return 0;
}
return -1;
base-commit: 663d7abe07ea376c2657019a03297ae87037c993
--
gitgitgadget
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] http: accept https:// proxies again
2026-06-27 17:17 [PATCH] http: accept https:// proxies again Johannes Schindelin via GitGitGadget
@ 2026-06-28 1:54 ` Junio C Hamano
2026-06-28 5:10 ` Junio C Hamano
0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2026-06-28 1:54 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Aliwoto, Johannes Schindelin
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> http.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/http.c b/http.c
> index 8e5a4d8bcf..8c0f831365 100644
> --- a/http.c
> +++ b/http.c
> @@ -802,6 +802,8 @@ static int set_curl_proxy_type(CURL *result, const char *protocol)
> if (has_proxy_cert_password())
> curl_easy_setopt(result, CURLOPT_PROXY_KEYPASSWD,
> proxy_cert_auth.password);
> +
> + return 0;
> }
>
> return -1;
That lack of "return 0" is so glaringly obvious when you point it
out like this patch does, and it is surprising it has been missed
initially.
From this function nothing returns an error anymore, and looking at
the preimage of 663d7abe (http: reject unsupported proxy URL
schemes, 2026-05-05) that is the source of the bug, the original did
not do anything when the corresponding code did not find and set any
proxy settings, either.
So perhaps it is a better fix to make it just a function that
returns void with early returns?
Thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] http: accept https:// proxies again
2026-06-28 1:54 ` Junio C Hamano
@ 2026-06-28 5:10 ` Junio C Hamano
0 siblings, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2026-06-28 5:10 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Aliwoto, Johannes Schindelin
Junio C Hamano <gitster@pobox.com> writes:
> From this function nothing returns an error anymore, and looking at
> the preimage of 663d7abe (http: reject unsupported proxy URL
> schemes, 2026-05-05) that is the source of the bug, the original did
> not do anything when the corresponding code did not find and set any
> proxy settings, either.
>
> So perhaps it is a better fix to make it just a function that
> returns void with early returns?
Nah, I was being stupid. Disregard the above.
The whole point of 663d7abe was that we wanted to reject what we did
not recognise, and we cannot do so without returning "good/bad" from
that function. The bug was that we did recognise https:// but still
returned -1 because of the bug, which the patch in the thread fixed.
Thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-06-28 5:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-27 17:17 [PATCH] http: accept https:// proxies again Johannes Schindelin via GitGitGadget
2026-06-28 1:54 ` Junio C Hamano
2026-06-28 5:10 ` Junio C Hamano
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.