On Fri, May 27 2022, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
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 :)
