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

From: Derrick Stolee <derrickstolee@github.com>
Date: May 24, 2022, 20:16 UTC
Message-ID: <14f71d47-e2c1-2a43-6434-af6f23d18754@github.com>
In-Reply-To: <nycvar.QRO.7.76.6.2205241339440.352@tvgsbejvaqbjf.bet>

On 5/24/2022 7:42 AM, Johannes Schindelin wrote:

Show 8 quoted lines

> Hi Stolee,
> 
> On Mon, 23 May 2022, Derrick Stolee via GitGitGadget wrote:
>> +	at_ptr = strchr(url, '@');
>> +	colon_ptr = strchr(url + scheme_len + 3, ':');
> 
> How certain are we that `url + scheme_len + 3` is still inside the
> NUL-separated `url`?

I'll update the method comment to make this clear.

Show 7 quoted lines

>> +
>> +	if (!colon_ptr)
>> +		BUG("failed to find colon in url '%s' with scheme_len %"PRIuMAX,
>> +		    url, (uintmax_t) scheme_len);
> 
> Wouldn't this mean that `https://github.com/git/git` with a `scheme_len`
> of 5 would hit that `BUG()` code path?

Yes. The method is about what to do once we've detected a URL with a "username:password@" combination after the scheme. I mentioned in a different reply, but I'll make this clear with a comment.

Thanks, -Stolee