Subject: Re: [PATCH v2 0/4] Strengthen fsck checks for submodule URLs

From: Junio C Hamano <[email-masked]>
Date: Thu, 18 Jan 2024 10:24:51 -0800

"Victoria Dye via GitGitGadget" <[email-masked]> writes:

> While testing 'git fsck' checks on .gitmodules URLs, I noticed that some
> invalid URLs were passing the checks. Digging into it a bit more, the issue
> turned out to be that 'credential_from_url_gently()' parses certain URLs
> (like "http://example.com:something/deeper/path") incorrectly, in a way that
> appeared to return a valid result.
>
> Fortunately, these URLs are rejected in fetches/clones/pushes anyway because
> 'url_normalize()' (called in 'validate_remote_url()') correctly identifies
> them as invalid. So, to bring 'git fsck' in line with other (stronger)
> validation done on remote URLs, this series replaces the
> 'credential_from_url_gently()' check with one that uses 'url_normalize()'.
>
>  * Patch 1 moves 'check_submodule_url()' to a public location so that it can
>    be used outside of 'fsck.c'.
>  * Patch 2 removes the obsolete/never-used code in 'test-tool submodule
>    check-name' handling names provided on the command line.
>  * Patch 3 adds a 'check-url' mode to 'test-tool submodule', calling the
>    now-public 'check_submodule_url()' method on a given URL, and adds new
>    tests checking valid and invalid submodule URLs.
>  * Patch 4 replaces the 'credential_from_url_gently()' check with
>    'url_normalize()' followed by 'url_decode()' and an explicit check for
>    newlines (to preserve the newline handling added in 07259e74ec1 (fsck:
>    detect gitmodules URLs with embedded newlines, 2020-03-11)).

Nicely done.  I'll wait for a few days to see if anybody else has
reaction but after reading the patches myself, my inclination is to
suggest merging it to 'next'.

Thanks.