Commit 5ae22a6373d7a135accf550d12150e2b9bfa8739
2019-06-21T08:13:31
fileops: fix creation of directory in filesystem root In commit 45f24e787 (git_repository_init: stop traversing at windows root, 2019-04-12), we have fixed `git_futils_mkdir` to correctly handle the case where we create a directory in Windows-style filesystem roots like "C:\repo". The problem here is an off-by-one: previously, to that commit, we've been checking wether the parent directory's length is equal to the root directory's length incremented by one. When we call the function with "/example", then the parent directory's length ("/") is 1, but the root directory offset is 0 as the path is directly rooted without a drive prefix. This resulted in `1 == 0 + 1`, which was true. With the change, we've stopped incrementing the root directory length, and thus now compare `1 <= 0`, which is false. The previous way of doing it was kind of finicky any non-obvious, which is also why the error was introduced. So instead of just re-adding the increment, let's explicitly add a condition that aborts finding the parent if the current parent path is "/". Making this change causes Azure Pipelines to fail the testcase repo::init::nonexistent_paths on Unix-based systems. This is because we have just fixed creating directories in the filesystem root, which previously didn't work. As Docker-based tests are running as root user, we are thus able to create the non-existing path and will now succeed to create the repository that was expected to actually fail. Let's split this up into three different tests: - A test to verify that we do not create repos in a non-existing parent directoy if the flag `GIT_REPOSITORY_INIT_MKPATH` is not set. - A test to verify that we fail if the root directory does not exist. As there is a common root directory on Unix-based systems that always exist, we can only test for this on Windows-based systems. - A test to verify that we fail if trying to create a repository in an unwriteable parent directory. We can only test this if not running tests as root user, as CAP_DAC_OVERRIDE will cause us to ignore permissions when creating files.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77
diff --git a/src/fileops.c b/src/fileops.c
index 0b732aa..648ffbe 100644
--- a/src/fileops.c
+++ b/src/fileops.c
@@ -495,7 +495,9 @@ int git_futils_mkdir(
* equal to length of the root path). The path may be less than the
* root path length on Windows, where `C:` == `C:/`.
*/
- if ((len == 1 && parent_path.ptr[0] == '.') || len <= root_len) {
+ if ((len == 1 && parent_path.ptr[0] == '.') ||
+ (len == 1 && parent_path.ptr[0] == '/') ||
+ len <= root_len) {
relative = make_path.ptr;
break;
}
diff --git a/tests/repo/init.c b/tests/repo/init.c
index 6e6e652..f2155b4 100644
--- a/tests/repo/init.c
+++ b/tests/repo/init.c
@@ -878,14 +878,55 @@ void test_repo_init__at_filesystem_root(void)
git_repository_free(repo);
}
-void test_repo_init__nonexistent_paths(void)
+void test_repo_init__nonexisting_directory(void)
{
+ git_repository_init_options opts = GIT_REPOSITORY_INIT_OPTIONS_INIT;
git_repository *repo;
+ /*
+ * If creating a repo with non-existing parent directories, then libgit2
+ * will by default create the complete directory hierarchy if using
+ * `git_repository_init`. Thus, let's use the extended version and not
+ * set the `GIT_REPOSITORY_INIT_MKPATH` flag.
+ */
+ cl_git_fail(git_repository_init_ext(&repo, "nonexisting/path", &opts));
+}
+
+void test_repo_init__nonexisting_root(void)
+{
#ifdef GIT_WIN32
+ git_repository *repo;
+
+ /*
+ * This really only depends on the nonexistence of the Q: drive. We
+ * cannot implement the equivalent test on Unix systems, as there is
+ * fundamentally no path that is disconnected from the root directory.
+ */
cl_git_fail(git_repository_init(&repo, "Q:/non/existent/path", 0));
cl_git_fail(git_repository_init(&repo, "Q:\\non\\existent\\path", 0));
#else
- cl_git_fail(git_repository_init(&repo, "/non/existent/path", 0));
+ clar__skip();
+#endif
+}
+
+void test_repo_init__unwriteable_directory(void)
+{
+#ifndef GIT_WIN32
+ git_repository *repo;
+
+ if (geteuid() == 0)
+ clar__skip();
+
+ /*
+ * Create a non-writeable directory so that we cannot create directories
+ * inside of it. The root user has CAP_DAC_OVERRIDE, so he doesn't care
+ * for the directory permissions and thus we need to skip the test if
+ * run as root user.
+ */
+ cl_must_pass(p_mkdir("unwriteable", 0444));
+ cl_git_fail(git_repository_init(&repo, "unwriteable/repo", 0));
+ cl_must_pass(p_rmdir("unwriteable"));
+#else
+ clar__skip();
#endif
}