Re: [PATCH v2] urlmatch: create fetch.credentialsInUrl config

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, May 27 2022, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <derrickstolee@github.com>

Just real quick, I hadn't taken notice of this before (the rest looks
good at a glance):

> +	/*
> +	 * Let's do some defensive programming to ensure the given
> +	 * URL is of the proper format.
> +	 */
> +	if (!colon_ptr)
> +		BUG("failed to find colon in url '%s' with scheme_len %"PRIuMAX,
> +		    url, (uintmax_t) scheme_len);
> +	if (colon_ptr > at_ptr)
> +		BUG("input url '%s' does not include credentials",
> +		    url);

So the function is renamed to detected_credentials_in_url(), so as a nit
I'd expect some verb like "strip", "redact" or whatever inthe name or
whatever, to make it clear what we're doing.

But since the only caller here below...

> +
> +	/* Include the colon when creating the redacted URL. */
> +	colon_ptr++;
> +	strbuf_addstr(&redacted, url);
> +	strbuf_splice(&redacted, colon_ptr - url, at_ptr - colon_ptr,
> +		      "<redacted>", 10);
> +
> +	if (!strcmp("warn", value))
> +		warning(_("URL '%s' uses plaintext credentials"), redacted.buf);
> +	if (!strcmp("die", value))
> +		die(_("URL '%s' uses plaintext credentials"), redacted.buf);
> +
> +	strbuf_release(&redacted);
> +}
> +
>  static char *url_normalize_1(const char *url, struct url_info *out_info, char allow_globs)
>  {
>  	/*
> @@ -144,6 +198,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
>  	 */
>  
>  	size_t url_len = strlen(url);
> +	const char *orig_url = url;
>  	struct strbuf norm;
>  	size_t spanned;
>  	size_t scheme_len, user_off=0, user_len=0, passwd_off=0, passwd_len=0;
> @@ -191,6 +246,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
>  			}
>  			colon_ptr = strchr(norm.buf + scheme_len + 3, ':');
>  			if (colon_ptr) {
> +				detected_credentials_in_url(orig_url, scheme_len);

Has already done the work of finding the colon_ptr (and at_ptr) why
re-do that paranoia since we have a static function, we could just pass
the two pointers we found already to strbuf_splice().

This also seems really close to something we could just add to strbuf.c
as e.g a strbuf_splice_to(). I.e. just:
	
	int strbuf_splice_to(const struct strbuf *in, struct strbuf *sb,
			     size_t pos, size_t len,
			     const void *data, size_t data_len);
	
Which would be used as:
	
	struct strbuf sb = STRBUF_INIT;
	if (!strbuf_splice_to(url, &redacted, /* same as strbuf_splice(...) */))
		warn("oh noes a password in %s", sb.buf);
	else
		warn("have no password in %s, no replacement done", url->buf);

Which re earlier talk of sharing an implementation with the other
<redacted> code looks like it could be dropped into the relevant part of
pkt-line.c.

But maybe that's all going overboard :)