submodule: fix case-folding gitdir filesystem collisions

Add a new check when extension.submodulePathConfig is enabled, to
detect and prevent case-folding filesystem colisions. When this
new check is triggered, a stricter casefolding aware URI encoding
is used to percent-encode uppercase characters.

By using this check/retry mechanism the uppercase encoding is
only applied when necessary, so case-sensitive filesystems are
not affected.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f19672f01a609cb5aa8f356d06b70d40712d2885..f9fc4c6eb54d612b3baa4ee66302c48dfeb269f0 100644 (file)
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -473,7 +473,7 @@ static void create_default_gitdir_config(const char *submodule_name)
                return;
        }
 
-       /* Case 2: Try URI-safe (RFC3986) encoding first, this fixes nested gitdirs */
+       /* Case 2.1: Try URI-safe (RFC3986) encoding first, this fixes nested gitdirs */
        strbuf_reset(&gitdir_path);
        repo_git_path_append(the_repository, &gitdir_path, "modules/");
        strbuf_addstr_urlencode(&gitdir_path, submodule_name, is_rfc3986_unreserved);
@@ -482,6 +482,30 @@ static void create_default_gitdir_config(const char *submodule_name)
                return;
        }
 
+       /* Case 2.2: Try extended uppercase URI (RFC3986) encoding, to fix case-folding */
+       strbuf_reset(&gitdir_path);
+       repo_git_path_append(the_repository, &gitdir_path, "modules/");
+       strbuf_addstr_urlencode(&gitdir_path, submodule_name, is_casefolding_rfc3986_unreserved);
+       if (!validate_and_set_submodule_gitdir(&gitdir_path, submodule_name))
+               return;
+
+       /* Case 2.3: Try some derived gitdir names, see if one sticks */
+       for (char c = '0'; c <= '9'; c++) {
+               strbuf_reset(&gitdir_path);
+               repo_git_path_append(the_repository, &gitdir_path, "modules/");
+               strbuf_addstr_urlencode(&gitdir_path, submodule_name, is_rfc3986_unreserved);
+               strbuf_addch(&gitdir_path, c);
+               if (!validate_and_set_submodule_gitdir(&gitdir_path, submodule_name))
+                       return;
+
+               strbuf_reset(&gitdir_path);
+               repo_git_path_append(the_repository, &gitdir_path, "modules/");
+               strbuf_addstr_urlencode(&gitdir_path, submodule_name, is_casefolding_rfc3986_unreserved);
+               strbuf_addch(&gitdir_path, c);
+               if (!validate_and_set_submodule_gitdir(&gitdir_path, submodule_name))
+                       return;
+       }
+
        /* Case 3: nothing worked, error out */
        die(_("failed to set a valid default config for 'submodule.%s.gitdir'. "
              "Please ensure it is set, for example by running something like: "

diff --git a/submodule.c b/submodule.c
index 0dc1fedcae56c9526905210db83397d265325dc7..8b7f6ed4e56b39f9555aa4cae7f2cc12ac5ca4fb 100644 (file)
--- a/submodule.c
+++ b/submodule.c
@@ -2253,15 +2253,58 @@ out:
        return ret;
 }
 
+static int check_casefolding_conflict(const char *git_dir,
+                                     const char *submodule_name,
+                                     const bool suffixes_match)
+{
+       char *p, *modules_dir = xstrdup(git_dir);
+       struct dirent *de;
+       DIR *dir = NULL;
+       int ret = 0;
+
+       if ((p = find_last_dir_sep(modules_dir)))
+               *p = '\0';
+
+       /* No conflict is possible if modules_dir doesn't exist (first clone) */
+       if (!is_directory(modules_dir))
+               goto cleanup;
+
+       dir = opendir(modules_dir);
+       if (!dir) {
+               ret = -1;
+               goto cleanup;
+       }
+
+       /* Check for another directory under .git/modules that differs only in case. */
+       while ((de = readdir(dir))) {
+               if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))
+                       continue;
+
+               if ((suffixes_match || is_git_directory(git_dir)) &&
+                   !strcasecmp(de->d_name, submodule_name) &&
+                   strcmp(de->d_name, submodule_name)) {
+                       ret = -1; /* collision found */
+                       break;
+               }
+       }
+
+cleanup:
+       if (dir)
+               closedir(dir);
+       free(modules_dir);
+       return ret;
+}
+
 /*
  * Encoded gitdir validation, only used when extensions.submodulePathConfig is enabled.
  * This does not print errors like the non-encoded version, because encoding is supposed
  * to mitigate / fix all these.
  */
-static int validate_submodule_encoded_git_dir(char *git_dir, const char *submodule_name UNUSED)
+static int validate_submodule_encoded_git_dir(char *git_dir, const char *submodule_name)
 {
        const char *modules_marker = "/modules/";
        char *p = git_dir, *last_submodule_name = NULL;
+       int config_ignorecase = 0;
 
        if (!the_repository->repository_format_submodule_path_cfg)
                BUG("validate_submodule_encoded_git_dir() must be called with "
@@ -2277,6 +2320,14 @@ static int validate_submodule_encoded_git_dir(char *git_dir, const char *submodu
        if (!last_submodule_name || strchr(last_submodule_name, '/'))
                return -1;
 
+       /* Prevent conflicts on case-folding filesystems */
+       repo_config_get_bool(the_repository, "core.ignorecase", &config_ignorecase);
+       if (ignore_case || config_ignorecase) {
+               bool suffixes_match = !strcmp(last_submodule_name, submodule_name);
+               return check_casefolding_conflict(git_dir, submodule_name,
+                                                 suffixes_match);
+       }
+
        return 0;
 }
 

diff --git a/t/t7425-submodule-gitdir-path-extension.sh b/t/t7425-submodule-gitdir-path-extension.sh
index dbe18f2925dca35159b1dbeb129a7d91d0f64838..eb9c80787c3d004208a174d7592c0220f3ea58c5 100755 (executable)
--- a/t/t7425-submodule-gitdir-path-extension.sh
+++ b/t/t7425-submodule-gitdir-path-extension.sh
@@ -384,4 +384,39 @@ test_expect_success 'disabling extensions.submodulePathConfig prevents nested su
        )
 '
 
+test_expect_success CASE_INSENSITIVE_FS 'verify case-folding conflicts are correctly encoded' '
+       git clone -c extensions.submodulePathConfig=true main cloned-folding &&
+       (
+               cd cloned-folding &&
+
+               # conflict: the "folding" gitdir will already be taken
+               git submodule add ../new-sub "folding" &&
+               test_commit lowercase &&
+               git submodule add ../new-sub "FoldinG" &&
+               test_commit uppercase &&
+
+               # conflict: the "foo" gitdir will already be taken
+               git submodule add ../new-sub "FOO" &&
+               test_commit uppercase-foo &&
+               git submodule add ../new-sub "foo" &&
+               test_commit lowercase-foo &&
+
+               # create a multi conflict between foobar, fooBar and foo%42ar
+               # the "foo" gitdir will already be taken
+               git submodule add ../new-sub "foobar" &&
+               test_commit lowercase-foobar &&
+               git submodule add ../new-sub "foo%42ar" &&
+               test_commit encoded-foo%42ar &&
+               git submodule add ../new-sub "fooBar" &&
+               test_commit mixed-fooBar
+       ) &&
+       verify_submodule_gitdir_path cloned-folding "folding" "modules/folding" &&
+       verify_submodule_gitdir_path cloned-folding "FoldinG" "modules/%46oldin%47" &&
+       verify_submodule_gitdir_path cloned-folding "FOO" "modules/FOO" &&
+       verify_submodule_gitdir_path cloned-folding "foo" "modules/foo0" &&
+       verify_submodule_gitdir_path cloned-folding "foobar" "modules/foobar" &&
+       verify_submodule_gitdir_path cloned-folding "foo%42ar" "modules/foo%42ar" &&
+       verify_submodule_gitdir_path cloned-folding "fooBar" "modules/fooBar0"
+'
+
 test_done

diff --git a/url.c b/url.c
index adc289229c6491b4cba42b7d94c16884b8cdee49..3ca5987e905d5943b9177a8a5b11fe374ac36373 100644 (file)
--- a/url.c
+++ b/url.c
@@ -9,6 +9,13 @@ int is_rfc3986_unreserved(char ch)
                ch == '-' || ch == '_' || ch == '.' || ch == '~';
 }
 
+int is_casefolding_rfc3986_unreserved(char c)
+{
+       return (c >= 'a' && c <= 'z') ||
+              (c >= '0' && c <= '9') ||
+              c == '-' || c == '.' || c == '_' || c == '~';
+}
+
 int is_urlschemechar(int first_flag, int ch)
 {
        /*

diff --git a/url.h b/url.h
index e644c3c809602833dd42eed4376a115547f1f1b2..cd9140e9946b16d10989dbf11b4830d69fc72857 100644 (file)
--- a/url.h
+++ b/url.h
@@ -28,4 +28,11 @@ void str_end_url_with_slash(const char *url, char **dest);
  */
 int is_rfc3986_unreserved(char ch);
 
+/*
+ * This is a variant of is_rfc3986_unreserved() that treats uppercase
+ * letters as "reserved". This forces them to be percent-encoded, allowing
+ * 'Foo' (%46oo) and 'foo' (foo) to be distinct on case-folding filesystems.
+ */
+int is_casefolding_rfc3986_unreserved(char c);
+
 #endif /* URL_H */