From: Junio C Hamano Date: Tue, 25 Aug 2026 00:40:49 GMT Subject: Re: [PATCH v2] http: preserve wwwauth_headers across redirects Message-ID: In-Reply-To: Junio C Hamano writes: > For those of you who are watching from the sidelines, this v2 lacks > the threading history. > > The v1 is at https://lore.kernel.org/git/20260602161150.1527493-1-aplattner@nvidia.com/ Any takers? It should be quite easy to be more interested and be a better reviewer in the http transfer codepaths than I am. Thanks. > Aaron Plattner writes: > >> When cURL follows a redirect, it calls the CURLOPT_HEADERFUNCTION for >> each header received including ones from a redirect. http_request() sets >> fwrite_wwwauth() as the header function, which will record the wwwauth[] >> entries for the last step in the redirection chain. >> >> However, when http_request_recoverable() sees that cURL followed a >> redirect, it attempts to update the credentials for the request from the >> new URL using credential_from_url(). The first thing that does is call >> credential_clear(), which clears everything including wwwauth_headers. >> >> If the new URL should use a credential helper rather than credentials >> embedded in the URL, this loses the list of authentication methods that >> the server provided in the redirect. >> >> The WWW-Authenticate challenge is not derived from the URL; it is >> populated from the server's response, and after a redirect it describes >> how to authenticate to the redirect target and it needs to survive the >> URL update so that credential helpers can know which authentication >> methods are allowed. >> >> Add a new credential_update_url() that wraps credential_from_url() and >> preserves wwwauth_headers specifically. Use SWAP() to avoid having to >> copy the whole strbuf. >> >> Signed-off-by: Aaron Plattner >> --- >> I decided to come back to this after I noticed that at least one other >> person had run into the same bug: >> >> https://lore.kernel.org/all/CADoNwcscDrx+YcfbcW4YKONDZZQgnPiwEOxL4QYV_C7_=FOFcg@mail.gmail.com/ >> >> Rather than reworking everything about how credentials are stored, I >> took your advice in [1] and just moved the code to preserve the wwwauth_headers >> into credential.c. That way any future credential fields that need to be >> preserved can be added there without having to hunt down other places >> like http.c that are reaching into it. >> >> [1] https://lore.kernel.org/all/xmqqpl28scll.fsf@gitster.g/ >> --- >> Changes in v2: >> - Move strvec preservation into a helper function in credential.c >> - Use SWAP instead of strvec_pushv() to avoid having to copy the >> contents of the strvec. >> - Link to v1: https://patch.msgid.link/20260602161150.1527493-1-aplattner@nvidia.com >> --- >> credential.c | 16 ++++++++++++++++ >> credential.h | 8 ++++++++ >> http.c | 9 ++++++++- >> t/lib-httpd/apache.conf | 1 + >> t/t5563-simple-http-auth.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++ >> 5 files changed, 78 insertions(+), 1 deletion(-) >> >> diff --git a/credential.c b/credential.c >> index 2594c0c422..035399d7ee 100644 >> --- a/credential.c >> +++ b/credential.c >> @@ -708,3 +708,19 @@ void credential_from_url(struct credential *c, const char *url) >> if (credential_from_url_gently(c, url, 0) < 0) >> die(_("credential url cannot be parsed: %s"), url); >> } >> + >> +void credential_update_url(struct credential *c, const char *url) >> +{ >> + struct strvec wwwauth_headers = STRVEC_INIT; >> + >> + /* >> + * credential_from_url() clears the whole credential. Preserve the >> + * WWW-Authenticate list, which is derived from the server's original >> + * response rather than from the URL and is required to authenticate to >> + * the new URL. >> + */ >> + SWAP(wwwauth_headers, c->wwwauth_headers); >> + credential_from_url(c, url); >> + SWAP(c->wwwauth_headers, wwwauth_headers); >> + strvec_clear(&wwwauth_headers); >> +} >> diff --git a/credential.h b/credential.h >> index c78b72d110..b90f666e33 100644 >> --- a/credential.h >> +++ b/credential.h >> @@ -305,6 +305,14 @@ void credential_write(const struct credential *, FILE *, >> void credential_from_url(struct credential *, const char *url); >> int credential_from_url_gently(struct credential *, const char *url, int quiet); >> >> +/* >> + * Update the URL-derived fields (protocol, host, path) of an existing >> + * credential to match a new URL. Unlike credential_from_url(), this function >> + * preserves state that was derived from a server's HTTP redirect response, >> + * such as the WWW-Authenticate headers. >> + */ >> +void credential_update_url(struct credential *c, const char *url); >> + >> int credential_match(const struct credential *want, >> const struct credential *have, int match_password); >> >> diff --git a/http.c b/http.c >> index a0d399b274..e8abb9f95a 100644 >> --- a/http.c >> +++ b/http.c >> @@ -2427,7 +2427,14 @@ static int http_request_recoverable(const char *url, >> if (options->effective_url && options->base_url) { >> if (update_url_from_redirect(options->base_url, >> url, options->effective_url)) { >> - credential_from_url(&http_auth, options->base_url->buf); >> + /* >> + * Use credential_update_url() rather than >> + * credential_from_url() so that the WWW-Authenticate >> + * challenge the server sent with the redirect target's >> + * response is preserved and handed to the credential >> + * helper. >> + */ >> + credential_update_url(&http_auth, options->base_url->buf); >> url = options->effective_url->buf; >> } >> } >> diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf >> index 4149fc1078..0627ef1433 100644 >> --- a/t/lib-httpd/apache.conf >> +++ b/t/lib-httpd/apache.conf >> @@ -203,6 +203,7 @@ RewriteRule ^/dumb-redir/(.*)$ /dumb/$1 [R=301] >> RewriteRule ^/smart-redir-perm/(.*)$ /smart/$1 [R=301] >> RewriteRule ^/smart-redir-temp/(.*)$ /smart/$1 [R=302] >> RewriteRule ^/smart-redir-auth/(.*)$ /auth/smart/$1 [R=301] >> +RewriteRule ^/custom_auth_redir/(.*)$ /custom_auth/$1 [R=302] >> RewriteRule ^/smart-redir-limited/(.*)/info/refs$ /smart/$1/info/refs [R=301] >> RewriteRule ^/ftp-redir/(.*)$ ftp://localhost:1000/$1 [R=302] >> >> diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh >> index a7d475dd68..349ae4ab39 100755 >> --- a/t/t5563-simple-http-auth.sh >> +++ b/t/t5563-simple-http-auth.sh >> @@ -557,6 +557,51 @@ test_expect_success 'access using bearer auth' ' >> EOF >> ' >> >> +test_expect_success 'bearer auth after redirect preserves wwwauth headers' ' >> + test_when_finished "per_test_cleanup" && >> + >> + set_credential_reply get <<-EOF && >> + capability[]=authtype >> + authtype=Bearer >> + credential=YS1naXQtdG9rZW4= >> + EOF >> + >> + cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF && >> + id=1 creds=Bearer YS1naXQtdG9rZW4= >> + EOF >> + >> + cat >"$HTTPD_ROOT_PATH/custom-auth.challenge" <<-EOF && >> + id=1 status=200 >> + id=default response=WWW-Authenticate: FooBar param1="value1" param2="value2" >> + id=default response=WWW-Authenticate: Bearer authorize_uri="id.example.com" p=1 q=0 >> + id=default response=WWW-Authenticate: Basic realm="example.com" >> + EOF >> + >> + test_config_global credential.helper test-helper && >> + test_config_global credential.useHttpPath true && >> + git ls-remote "$HTTPD_URL/custom_auth_redir/repo.git" && >> + >> + expect_credential_query get <<-EOF && >> + capability[]=authtype >> + capability[]=state >> + protocol=http >> + host=$HTTPD_DEST >> + path=custom_auth/repo.git >> + wwwauth[]=FooBar param1="value1" param2="value2" >> + wwwauth[]=Bearer authorize_uri="id.example.com" p=1 q=0 >> + wwwauth[]=Basic realm="example.com" >> + EOF >> + >> + expect_credential_query store <<-EOF >> + capability[]=authtype >> + authtype=Bearer >> + credential=YS1naXQtdG9rZW4= >> + protocol=http >> + host=$HTTPD_DEST >> + path=custom_auth/repo.git >> + EOF >> +' >> + >> test_expect_success 'access using bearer auth with invalid credentials' ' >> test_when_finished "per_test_cleanup" && >> >> >> --- >> base-commit: dea0ea3582e6980ddbc1173cc8e3e9f9db91cde0 >> change-id: 20260819-http-preserve-wwwauth-redirect-a3fe4dab6b35