On 5/27/2022 10:22 AM, Ævar Arnfjörð Bjarmason wrote:
>
> 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.
The name means "Do what needs to be done when creds are detected
in a URL".
If we decide to change the response to creds in a URL, then we
would change the implementation without changing the name.
>> 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,
Unfortunately, at this point 'url' is not equal to 'orig_url' and
'colon_ptr' isn't even within the 'orig_url' string. It's been
copied and mutated. It was my first instinct to share as much as
possible, which is why 'schema' is reused.
Thanks,
-Stolee
