We have already introduced "files_fsck_symref_target". We should reuse this function to handle the symrefs which are legacy symbolic links. We should not check the trailing garbage for symbolic links. Add a new parameter "symbolic_link" to disable some checks which should only be used for symbolic ref. We firstly use the "strbuf_add_real_path" to resolve the symlinks and get the absolute path "pointee_path" which the symlink ref points to. Then we can get the absolute path "abs_gitdir" of the "gitdir". By combining "pointee_path" and "abs_gitdir", we can extract the "referent". Thus, we can reuse "files_fsck_symref_target" function to seamlessly check the symlink refs. Mentored-by: Patrick Steinhardt <ps@xxxxxx> Mentored-by: Karthik Nayak <karthik.188@xxxxxxxxx> Signed-off-by: shejialuo <shejialuo@xxxxxxxxx> --- refs/files-backend.c | 68 +++++++++++++++++++++++++++++----------- t/t0602-reffiles-fsck.sh | 44 ++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 18 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 382c73fcf7..8641e3ba65 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1,4 +1,5 @@ #include "../git-compat-util.h" +#include "../abspath.h" #include "../copy.h" #include "../environment.h" #include "../gettext.h" @@ -3437,13 +3438,15 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, /* * Check the symref "pointee_name" and "pointee_path". The caller should * make sure that "pointee_path" is absolute. For symbolic ref, "pointee_name" - * would be the content after "refs:". + * would be the content after "refs:". For symblic link, "pointee_name" would + * be the relative path agaignst "gitdir". */ static int files_fsck_symref_target(struct fsck_options *o, struct fsck_ref_report *report, const char *refname, struct strbuf *pointee_name, - struct strbuf *pointee_path) + struct strbuf *pointee_path, + unsigned int symbolic_link) { const char *newline_pos = NULL; const char *p = NULL; @@ -3458,24 +3461,28 @@ static int files_fsck_symref_target(struct fsck_options *o, goto out; } - newline_pos = strrchr(p, '\n'); - if (!newline_pos || *(newline_pos + 1)) { - ret = fsck_report_ref(o, report, - FSCK_MSG_REF_MISSING_NEWLINE, - "missing newline"); + if (!symbolic_link) { + newline_pos = strrchr(p, '\n'); + if (!newline_pos || *(newline_pos + 1)) { + ret = fsck_report_ref(o, report, + FSCK_MSG_REF_MISSING_NEWLINE, + "missing newline"); + } } if (check_refname_format(pointee_name->buf, 0)) { - /* - * When containing null-garbage, "check_refname_format" will - * fail, we should trim the "pointee" to check again. - */ - strbuf_rtrim(pointee_name); - if (!check_refname_format(pointee_name->buf, 0)) { - ret = fsck_report_ref(o, report, - FSCK_MSG_TRAILING_REF_CONTENT, - "trailing null-garbage"); - goto out; + if (!symbolic_link) { + /* + * When containing null-garbage, "check_refname_format" will + * fail, we should trim the "pointee" to check again. + */ + strbuf_rtrim(pointee_name); + if (!check_refname_format(pointee_name->buf, 0)) { + ret = fsck_report_ref(o, report, + FSCK_MSG_TRAILING_REF_CONTENT, + "trailing null-garbage"); + goto out; + } } ret = fsck_report_ref(o, report, @@ -3510,9 +3517,12 @@ static int files_fsck_refs_content(struct ref_store *ref_store, { struct strbuf pointee_path = STRBUF_INIT; struct strbuf ref_content = STRBUF_INIT; + struct strbuf abs_gitdir = STRBUF_INIT; struct strbuf referent = STRBUF_INIT; struct strbuf refname = STRBUF_INIT; struct fsck_ref_report report = {0}; + const char *pointee_name = NULL; + unsigned int symbolic_link = 0; const char *trailing = NULL; unsigned int type = 0; int failure_errno = 0; @@ -3557,16 +3567,38 @@ static int files_fsck_refs_content(struct ref_store *ref_store, ref_store->gitdir, referent.buf); ret = files_fsck_symref_target(o, &report, refname.buf, &referent, - &pointee_path); + &pointee_path, + symbolic_link); } goto cleanup; } + symbolic_link = 1; + + strbuf_add_real_path(&pointee_path, iter->path.buf); + strbuf_add_absolute_path(&abs_gitdir, ref_store->gitdir); + strbuf_normalize_path(&abs_gitdir); + if (!is_dir_sep(abs_gitdir.buf[abs_gitdir.len - 1])) + strbuf_addch(&abs_gitdir, '/'); + + if (!skip_prefix(pointee_path.buf, abs_gitdir.buf, &pointee_name)) { + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_SYMREF_POINTEE, + "point to target outside gitdir"); + goto cleanup; + } + + strbuf_addstr(&referent, pointee_name); + ret = files_fsck_symref_target(o, &report, refname.buf, + &referent, &pointee_path, + symbolic_link); + cleanup: strbuf_release(&refname); strbuf_release(&ref_content); strbuf_release(&referent); strbuf_release(&pointee_path); + strbuf_release(&abs_gitdir); return ret; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 69280795ca..36992fbc7f 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -230,4 +230,48 @@ test_expect_success 'symbolic ref content should be checked' ' test_cmp expect err ' +test_expect_success SYMLINKS 'symbolic ref (symbolic link) content should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + branch_dir_prefix=.git/refs/heads && + tag_dir_prefix=.git/refs/tags && + cd repo && + git commit --allow-empty -m initial && + git checkout -b branch-1 && + git tag tag-1 && + git checkout -b a/b/branch-2 && + + ln -sf ../../../../branch $branch_dir_prefix/branch-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-symbolic: badSymrefPointee: point to target outside gitdir + EOF + rm $branch_dir_prefix/branch-symbolic && + test_cmp expect err && + + ln -sf ../../logs/branch-bad $branch_dir_prefix/branch-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-symbolic: badSymrefPointee: points to ref outside the refs directory + EOF + rm $branch_dir_prefix/branch-symbolic && + test_cmp expect err && + + ln -sf ./"branch space" $branch_dir_prefix/branch-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-symbolic: badSymrefPointee: points to refname with invalid format + EOF + rm $branch_dir_prefix/branch-symbolic && + test_cmp expect err && + + ln -sf ./".branch" $branch_dir_prefix/branch-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-symbolic: badSymrefPointee: points to refname with invalid format + EOF + rm $branch_dir_prefix/branch-symbolic && + test_cmp expect err +' + test_done -- 2.46.0