http: accept https:// proxies again
PATCH 1/1 http: accept https:// proxies again
From: Johannes Schindelin <[email protected]>
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.
---
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(+)
http.c +2 -0
@@ -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
Signature · 1 line
gitgitgadget
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 8 lines · writes:
http.c | 2 ++ 1 file changed, 2 insertions(+)http.c +0
-0
http.c static int set_curl_proxy_type(CURL *result, const char *protocol) · line 809
Quoted 8 lines
http.c +2
-0@@ -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.
Quoted 8 lines · Junio C Hamano
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.
Quoted 11 lines · Junio C Hamano
Quoted 8 lines · Junio C Hamano
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?
Quoted 6 lines
Nah, I was being stupid. Disregard the above.
The whole point of
663d7abewas 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.
And as an important bugfix, this patch of course has been fast-tracked. I'll make sure we have it in 'master' before Git 2.55 gets tagged.
Thanks.