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

[Jeff King <peff@peff.net>]

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