"Victoria Dye via GitGitGadget" <gitgitgadget@xxxxxxxxx> 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.
