8000 repo: simplify safe.directory comparison · russell/libgit2@516749f · GitHub < 8000 meta name="color-scheme" content="light dark" />
[go: up one dir, main page]
Skip to content

Commit 516749f

Browse files
ethomsonethomson
committed
repo: simplify safe.directory comparison
Keep the `git_str` buf that prevents unnecessary small allocations, and simplify the comparisons compared to what was there previously.
1 parent 73f034c commit 516749f

File tree

2 files changed

+23
-33
lines changed

2 files changed

+23
-33
lines changed

‎src/libgit2/repository.c

Lines changed: 21 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,8 @@ static int read_gitfile(git_str *path_out, const char *file_path)
537537
}
538538

539539
typedef struct {
540-
git_str repo_path;
540+
const char *repo_path;
541+
git_str tmp;
541542
bool *is_safe;
542543
} validate_ownership_data;
543544

@@ -552,6 +553,18 @@ static int validate_ownership_cb(const git_config_entry *entry, void *payload)
552553
} else {
553554
const char *test_path = entry->value;
554555

556+
if (git_str_sets(&data->tmp, test_path) < 0 ||
557+
git_fs_path_to_dir(&data->tmp) < 0)
558+
return -1;
559+
560+
/*
561+
* Ensure that `git_fs_path_to_dir` mutated the
562+
* input path by adding a trailing backslash.
563+
* A trailing backslash on the input is not allowed.
564+
*/
565+
if (strcmp(data->tmp.ptr, test_path) == 0)
566+
return 0;
567+
555568
#ifdef GIT_WIN32
556569
/*
557570
* Git for Windows does some truly bizarre things with
@@ -581,7 +594,8 @@ static int validate_ownership_cb(const git_config_entry *entry, void *payload)
581594
strncmp(test_path, "//wsl.localhost/", strlen("//wsl.localhost/")) != 0)
582595
test_path++;
583596
#endif
584-
if (strcmp(test_path, data->repo_path.ptr) == 0)
597+
598+
if (strcmp(data->tmp.ptr, data->repo_path) == 0)
585599
*data->is_safe = true;
586600
}
587601

@@ -594,21 +608,14 @@ static int validate_ownership_config(
594608
bool use_env)
595609
{
596610
validate_ownership_data ownership_data = {
597-
GIT_STR_INIT, is_safe
611+
path, GIT_STR_INIT, is_safe
598612
};
599613
git_config *config;
600614
int error;
601615

602616
if (load_global_config(&config, use_env) != 0)
603617
return 0;
604618

605-
git_str_sets(&ownership_data.repo_path, path);
606-
if (git_str_oom(&ownership_data.repo_path))
607-
return -1;
608-
if (git_str_len(&ownership_data.repo_path) > 1 &&
609-
ownership_data.repo_path.ptr[git_str_len(&ownership_data.repo_path) - 1] == '/')
610-
git_str_shorten(&ownership_data.repo_path, 1);
611-
612619
error = git_config_get_multivar_foreach(config,
613620
"safe.directory", NULL,
614621
validate_ownership_cb,
@@ -618,7 +625,7 @@ static int validate_ownership_config(
618625
error = 0;
619626

620627
git_config_free(config);
621-
git_str_dispose(&ownership_data.repo_path);
628+
git_str_dispose(&ownership_data.tmp);
622629

623630
return error;
624631
}
@@ -685,26 +692,9 @@ static int validate_ownership(git_repository *repo)
685692
goto done;
686693

687694
if (!is_safe) {
688-
git_str nice_path = GIT_STR_INIT;
689-
#ifdef GIT_WIN32
690-
/* see comment above in validate_ownership_cb */
691-
if (!strncasecmp(path, "//", strlen("//")))
692-
git_str_puts(&nice_path, "%(prefix)/");
693-
#endif
694-
git_str_puts(&nice_path, path);
695-
if (!git_str_oom(&nice_path)) {
696-
if (git_str_len(&nice_path) > 1 && nice_path.ptr[git_str_len(&nice_path) - 1] == '/')
697-
git_str_shorten(&nice_path, 1);
698-
git_error_set(
699-
GIT_ERROR_CONFIG,
700-
"repository path '%s' is not owned by current user.\n\nTo add an exception use the path '%s'.",
701-
path, nice_path.ptr);
702-
} else
703-
git_error_set(
704-
GIT_ERROR_CONFIG,
705-
"repository path '%s' is not owned by current user.",
706-
path);
707-
git_str_dispose(&nice_path);
695+
git_error_set(GIT_ERROR_CONFIG,
696+
"repository path '%s' is not owned by current user",
697+
path);
708698
error = GIT_EOWNER;
709699
}
710700

‎tests/libgit2/repo/open.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,7 @@ void test_repo_open__can_allowlist_dirs_with_problematic_ownership(void)
555555

556556
git_str_joinpath(&config_filename, config_path.ptr, ".gitconfig");
557557

558-
// Test with incorrect exception (slash at the end)
558+
/* Test with incorrect exception (slash at the end) */
559559
git_str_printf(&config_data,
560560
"[foo]\n" \
561561
"\tbar = Foobar\n" \
@@ -572,7 +572,7 @@ void test_repo_open__can_allowlist_dirs_with_problematic_ownership(void)
572572
cl_git_rewritefile(config_filename.ptr, config_data.ptr);
573573
cl_git_fail_with(GIT_EOWNER, git_repository_open(&repo, "empty_standard_repo"));
574574

575-
// Test with correct exception
575+
/* Test with correct exception */
576576
git_str_clear(&config_data);
577577
git_str_printf(&config_data,
578578
"[foo]\n" \

0 commit comments

Comments
 (0)
0