submodule-config.c: strengthen URL fsck check

Update the validation of "curl URL" submodule URLs (i.e. those that specify an "http[s]" or "ftp[s]" protocol) in 'check_submodule_url()' to catch more invalid URLs. The existing validation using 'credential_from_url_gently()' parses certain URLs incorrectly, leading to invalid submodule URLs passing 'git fsck' checks. Conversely, 'url_normalize()' - used to validate remote URLs in 'remote_get()' - correctly identifies the invalid URLs missed by 'credential_from_url_gently()'. To catch more invalid cases, replace 'credential_from_url_gently()' with 'url_normalize()' followed by a 'url_decode()' and a check for newlines (mirroring 'check_url_component()' in the 'credential_from_url_gently()' validation). Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Victoria Dye committed Jan 18, 2024 at 01:55 UTC 8430b438f628f2f0df08622a550e750158167f28
2 files changed +12 -15
submodule-config.c
+11 -5
@@ -15,7 +15,7 @@
15 #include "thread-utils.h"
16 #include "tree-walk.h"
17 #include "url.h"
18 -#include "credential.h"
18 +#include "urlmatch.h"
19
20 /*
21 * submodule cache lookup structure
@@ -350,12 +350,18 @@ int check_submodule_url(const char *url)
350 }
351
352 else if (url_to_curl_url(url, &curl_url)) {
353 - struct credential c = CREDENTIAL_INIT;
353 int ret = 0;
355 - if (credential_from_url_gently(&c, curl_url, 1) ||
356 - !*c.host)
354 + char *normalized = url_normalize(curl_url, NULL);
355 + if (normalized) {
356 + char *decoded = url_decode(normalized);
357 + if (strchr(decoded, '\n'))
358 + ret = -1;
359 + free(normalized);
360 + free(decoded);
361 + } else {
362 ret = -1;
358 - credential_clear(&c);
363 + }
364 +
365 return ret;
366 }
367
t/t7450-bad-git-dotfiles.sh
+1 -10
@@ -63,6 +63,7 @@ test_expect_success 'check urls' '
63 ./%0ahost=example.com/foo.git
64 https://one.example.com/evil?%0ahost=two.example.com
65 https:///example.com/foo.git
66 + http://example.com:test/foo.git
67 https::example.com/foo.git
68 http:::example.com/foo.git
69 EOF
@@ -70,16 +71,6 @@ test_expect_success 'check urls' '
71 test_cmp expect actual
72 '
73
73 -# NEEDSWORK: the URL checked here is not valid (and will not work as a remote if
74 -# a user attempts to clone it), but the fsck check passes.
75 -test_expect_failure 'url check misses invalid cases' '
76 - test-tool submodule check-url >actual <<-\EOF &&
77 - http://example.com:test/foo.git
78 - EOF
79 -
80 - test_must_be_empty actual
81 -'
82 -
74 test_expect_success 'create innocent subrepo' '
75 git init innocent &&
76 git -C innocent commit --allow-empty -m foo