Re: [PATCH 2/2] http: prevent redirect from dropping credentials during reauth

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 4, 2024, 22:51 UTC
Message-ID: <xmqq8r3zonat.fsf@gitster.g>
In-Reply-To: <20240204185427.39664-3-ypsah@devyard.org>

Quentin Bouget <ypsah@devyard.org> writes:

Show 8 quoted lines

> During a re-authentication (second attempt at authenticating with a
> remote, e.g. after a failed GSSAPI attempt), git allows the remote to
> provide credential overrides in the redirect URL and unconditionnaly
> drops the current HTTP credentials in favors of those, even when there
> aren't any.
>
> This commit makes it so HTTP credentials are only overridden when the
> redirect URL actually contains credentials itself.

"This commit makes it so" -> "Make it so"

Show 6 quoted lines

> +			char *username = NULL, *password = NULL;
> +
> +			if (http_auth.username)
> +				username = xstrdup(http_auth.username);
> +			if (http_auth.password)
> +				password = xstrdup(http_auth.password);

Not a huge deal, but we have xstrdup_or_null() helper function exactly for a use case like this.

Show 11 quoted lines

>  			credential_from_url(&http_auth, options->base_url->buf);
> +
> +			if (http_auth.username)
> +				free(username);
> +			else if (username)
> +				http_auth.username = username;
> +
> +			if (http_auth.password)
> +				free(password);
> +			else if (password)
> +				http_auth.password = password;

This is an interesting change. I wonder what breaks if we completely ignored such credential materials forced by the remote via a redirect?

>  			url = options->effective_url->buf;
>  		}
>  	}

Thanks.