[PATCH v2] http: preserve wwwauth_headers across redirects

[PATCH v2] http: preserve wwwauth_headers across redirects

[Aaron Plattner]

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 <aplattner@nvidia.com>
---
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

Re: [PATCH v2] http: preserve wwwauth_headers across redirects

[Junio C Hamano]

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/

Thanks.

Aaron Plattner <aplattner@nvidia.com> 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 <aplattner@nvidia.com>
> ---
> 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

Re: [PATCH v2] http: preserve wwwauth_headers across redirects

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> 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 <aplattner@nvidia.com> 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 <aplattner@nvidia.com>
>> ---
>> 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

Re: [PATCH v2] http: preserve wwwauth_headers across redirects

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> 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.

Second call.  Anybody?

Unless we see some volunteers to give it a review in a few weeks, I
will discard the topic out of my tree, as 'seen' has been crowded
with topics that have been collecting dust without getting many
reviews.

Thanks.

Re: [PATCH v2] http: preserve wwwauth_headers across redirects

[Jeff King]

On Mon, Nov 02, 2026 at 12:51:14AM -0800, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Junio C Hamano <gitster@pobox.com> 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.
> 
> Second call.  Anybody?
> 
> Unless we see some volunteers to give it a review in a few weeks, I
> will discard the topic out of my tree, as 'seen' has been crowded
> with topics that have been collecting dust without getting many
> reviews.

I gave it a review. In short, I think the patch is OK as-is, though I had a
few small suggestions for the code (and it might be worth describing the
"why is this safe" part in the commit message).

I think one of the reasons it didn't get much review is that the wwwauth
is not well understood by other folks who work on HTTP, and the original
author of the feature wasn't cc'd (I did in my review, though he's not
super active these days, so I also wouldn't hold my breath).

-Peff

Re: [PATCH v2] http: preserve wwwauth_headers across redirects

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Nov 02, 2026 at 12:51:14AM -0800, Junio C Hamano wrote:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>> 
>> > Junio C Hamano <gitster@pobox.com> 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.
>> 
>> Second call.  Anybody?
>> 
>> Unless we see some volunteers to give it a review in a few weeks, I
>> will discard the topic out of my tree, as 'seen' has been crowded
>> with topics that have been collecting dust without getting many
>> reviews.
>
> I gave it a review. In short, I think the patch is OK as-is, though I had a
> few small suggestions for the code (and it might be worth describing the
> "why is this safe" part in the commit message).
>
> I think one of the reasons it didn't get much review is that the wwwauth
> is not well understood by other folks who work on HTTP, and the original
> author of the feature wasn't cc'd (I did in my review, though he's not
> super active these days, so I also wouldn't hold my breath).

I see there were a few responses to your review.  Let's see how
Aaron responds---perhaps this time we see a small update to move it
beyond the finish line ;-).

Thanks.

Re: [PATCH v2] http: preserve wwwauth_headers across redirects

[Jeff King]

On Wed, Aug 19, 2026 at 08:21:04PM -0700, Aaron Plattner wrote:

> 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.

OK, the problem description makes sense. I think the root cause here is
that wwwauth_headers should not really be in the credential struct in
the first place. They are a property of the HTTP request, and we get
them _back_ from the server; we don't send them. They are stuffed into
the credential struct because we pass them to helpers, but they're
otherwise unlike the rest of the struct in that respect.

So one possible path forward would be to pull them out of the struct,
though I think that does create new headaches. We'd need to pass them in
to credential_fill(), for example, so it can feed them to the helpers.

So it may be the past of least resistance to just hack around this reset
problem and preserve them (i.e., your patch).

> 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.

So my biggest question/fear would be: are we creating a security problem
wherein we use auth information from the original server with the
redirected-to server (the avoidance of which is the whole point of
clearing the credential struct in the first place).

And I think the answer is "no". These wwwauth headers are coming from
the server back to us. And as described in 6b8dda9a4f (http: read HTTP
WWW-Authenticate response headers, 2023-02-27), we get the headers from
_all_ servers we talk to (the original and the redirect target). So we
have to deal with this problem already, and do so by resetting the list
when we see a new HTTP request start. So by definition, the wwwauth
headers we get back are the ones from the redirected-to server. And so
preserving them as we move to that new URL is the correct thing.

I see that the credential_update_url() wrapper is new in this iteration
of the patch. I actually think it is a slight step in the wrong
direction. We know that retaining wwwauth_headers is safe here only
because we know we the updated URL we are feeding matches the response
from that server. Any other hypothetical callers of the new function
would also need to consider that (and if they get it wrong, it fails in
the insecure direction).

That said, I'd highly doubt we'd ever get another caller either way (nor
add any other data to the credential that is treated this way). So I am
OK with it, but have a slight preference for the original inline
treatment from the v1 patch.

> +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);
> +}

OK. This strvec_clear() at the end is almost certainly a noop. We
swapped in the value set by credential_from_url(), which will always be
blank. But it probably does not hurt. OTOH, if that assumption is wrong,
we are throwing away useful bits that we pulled from the URL, without
even knowing what they are. So maybe:

  if (wwwauth_headers.nr)
	BUG("credential_from_url put something in the wwwauth_headers?");

would better describe what's going on.

I'm adding Matthew Cheetham to the cc, as he was the original author of
these wwwauth bits.

-Peff

Re: [PATCH v2] http: preserve wwwauth_headers across redirects

[Matthew John Cheetham]

On 04/11/2026 06:24, Jeff King wrote:

> On Wed, Aug 19, 2026 at 08:21:04PM -0700, Aaron Plattner wrote:
> 
>> 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.
> 
> OK, the problem description makes sense. I think the root cause here is
> that wwwauth_headers should not really be in the credential struct in
> the first place. They are a property of the HTTP request, and we get
> them _back_ from the server; we don't send them. They are stuffed into
> the credential struct because we pass them to helpers, but they're
> otherwise unlike the rest of the struct in that respect.

Yeah, this was one of the awkward parts of passing WWW-Authenticate
headers through to credential helpers in the original implementation.

http.c has per-request slots, but much of its authentication and
configuration state is global. The credential struct already carried
helper configuration and interaction state, such as the helpers list,
use_http_path, approved and quit, so it was a convenient place to carry
the challenges to helpers too. That does leave URL-derived and
response-derived state with different lifetimes in the same object,
though.

I've wondered whether a clearer separation between shared HTTP client
state and per-request state would make this easier to reason about.
That would also need to account for callers like remote-curl.c and
http-push.c, which configure curl handles through the lower-level slot
API exposed by http.h.

The current split can make ownership and retries harder to follow, and
I've certainly contributed some of that complexity myself! ;-)

I've made a few exploratory attempts in that direction: a reusable
client with no global state and explicit per-request context and
policies for authentication and retries. Nothing ready to propose yet -
it's a messy space!

That's a broader cleanup, though, rather than a prerequisite for this
fix.

> So one possible path forward would be to pull them out of the struct,
> though I think that does create new headaches. We'd need to pass them in
> to credential_fill(), for example, so it can feed them to the helpers.

Right, I remember thinking the other alternative would be to expand the
credential_fill() function to take headers. Other than the churn, I also
didn't do this because WWW-Authenticate headers are an HTTP only
construct, and credential_fill is used for other things like SMTP creds
and the password to certificates.

You raised the smell in the original series about adding wwwauth_headers
and header_is_last_match to the credential struct:
https://lore.kernel.org/git/Y9JWnQeEV0weV4yu@coredump.intra.peff.net

Your follow-up clarified that the main concern was the internal HTTP
parsing flag. Moving it out would require some extra plumbing across
curl callbacks, so we left it as-is at the time:
https://lore.kernel.org/git/Y+TxRDatM9Iirwwu@coredump.intra.peff.net

> So it may be the past of least resistance to just hack around this reset
> problem and preserve them (i.e., your patch).
> 
>> 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.
> 
> So my biggest question/fear would be: are we creating a security problem
> wherein we use auth information from the original server with the
> redirected-to server (the avoidance of which is the whole point of
> clearing the credential struct in the first place).
> 
> And I think the answer is "no". These wwwauth headers are coming from
> the server back to us. And as described in 6b8dda9a4f (http: read HTTP
> WWW-Authenticate response headers, 2023-02-27), we get the headers from
> _all_ servers we talk to (the original and the redirect target). So we
> have to deal with this problem already, and do so by resetting the list
> when we see a new HTTP request start. So by definition, the wwwauth
> headers we get back are the ones from the redirected-to server. And so
> preserving them as we move to that new URL is the correct thing.

Right. fwrite_wwwauth() clears the challenge list whenever it sees a
new HTTP status line. After libcurl follows the redirects, the list
therefore belongs to the final response, not an earlier response in
the chain. Preserving it while updating the credential context to
match that response's URL is the intended behaviour.

> I see that the credential_update_url() wrapper is new in this iteration
> of the patch. I actually think it is a slight step in the wrong
> direction. We know that retaining wwwauth_headers is safe here only
> because we know we the updated URL we are feeding matches the response
> from that server. Any other hypothetical callers of the new function
> would also need to consider that (and if they get it wrong, it fails in
> the insecure direction).
> 
> That said, I'd highly doubt we'd ever get another caller either way (nor
> add any other data to the credential that is treated this way). So I am
> OK with it, but have a slight preference for the original inline
> treatment from the v1 patch.
> 
>> +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);
>> +}
> 
> OK. This strvec_clear() at the end is almost certainly a noop. We
> swapped in the value set by credential_from_url(), which will always be
> blank. But it probably does not hurt. OTOH, if that assumption is wrong,
> we are throwing away useful bits that we pulled from the URL, without
> even knowing what they are. So maybe:
> 
>    if (wwwauth_headers.nr)
> 	BUG("credential_from_url put something in the wwwauth_headers?");
> 
> would better describe what's going on.
> 
> I'm adding Matthew Cheetham to the cc, as he was the original author of
> these wwwauth bits.

I appreciate the cc! :-)

> -Peff


Thanks,
Matthew

Re: [PATCH v2] http: preserve wwwauth_headers across redirects

[Michal Suchánek]

Hello,

On Wed, Nov 04, 2026 at 01:24:23AM -0500, Jeff King wrote:
> On Wed, Aug 19, 2026 at 08:21:04PM -0700, Aaron Plattner wrote:
> 
> > 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.

I think the decscription here is inconsistent.

In the first paragraph it sounds like the wwwauth comes from the
redirected-to URL, while in the last paragraph it sounds like it comes
from the initial URL.

I think it would be good to word this more clearly.

Thanks

Michal