| Message ID | Ztb_JuMjaoAbIZXq@ArchLinux (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | add ref content check for files backend | expand |
On Tue, Sep 03, 2024 at 08:20:54PM +0800, shejialuo wrote: > We have already introduced the checks for regular refs. There is no need > to check the consistency of the target which the symref points to. > Instead, we just need to check the content of teh symref itself. s/teh/the > In order to check the content of the symref, create a function > "files_fsck_symref_target". It will first check whether the "referent" > is under the "refs/" directory and then we will check the symref > contents. > > A regular file is accepted as a textual symref if it begins with > "ref:", followed by zero or more whitespaces, followed by the full > refname, followed only by whitespace characters. We always write > a single SP after "ref:" and a single LF after the refname, but > third-party reimplementations of Git may have taken advantage of the > looser syntax. Put it more specific, we accept the following contents > of the symref: > > 1. "ref: refs/heads/master " > 2. "ref: refs/heads/master \n \n" > 3. "ref: refs/heads/master\n\n" > > But we do not allow any other trailing garbage. The followings are bad > symref contents which will be reported as fsck error by "git-fsck(1)". > > 1. "ref: refs/heads/master garbage\n" > 2. "ref: refs/heads/master \n\n\n garbage " > > In order to provide above checks, we will first check whether the symref > content misses the newline by peeking the last byte of the "referent" to > see whether it is '\n'. I'd still argue that we should do the same retroactive tightening as we introduce for normal references, also with an INFO level at first. Otherwise we're being inconsistent across the ref types. > And we will remember the untrimmed length of the "referent" and call > "strbuf_rtrim()" on "referent". Then, we will call "check_refname_format" > to chceck whether the trimmed referent format is valid. If not, we will > report to the user that the symref points to referent which has invalid > format. If it is valid, we will compare the untrimmed length and trimmed > length, if they are not the same, we need to warn the user there is some > trailing garbage in the symref content. > > At last, we need to check whether the referent is the directory. We > cannot distinguish whether the "refs/heads/a" is a directory or not by > using "check_refname_format". We have already checked bad file type when > iterating the "refs/" directory but we ignore the directory. Thus, we > need to explicitly add check here. > > Mentored-by: Patrick Steinhardt <ps@pks.im> > Mentored-by: Karthik Nayak <karthik.188@gmail.com> > Signed-off-by: shejialuo <shejialuo@gmail.com> > --- > Documentation/fsck-msgids.txt | 4 ++ > fsck.h | 1 + > refs/files-backend.c | 81 +++++++++++++++++++++++ > t/t0602-reffiles-fsck.sh | 117 ++++++++++++++++++++++++++++++++++ > 4 files changed, 203 insertions(+) > > diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt > index 06d045ac48..beb6c4e49e 100644 > --- a/Documentation/fsck-msgids.txt > +++ b/Documentation/fsck-msgids.txt > @@ -28,6 +28,10 @@ > `badRefName`:: > (ERROR) A ref has an invalid format. > > +`badSymrefTarget`:: > + (ERROR) The symref target points outside the ref directory or > + the name of the symref target is invalid. These are two separate error cases, and we even have different code paths raising them. Shouldn't we thus also have two different diagnostic codes for this? > `badTagName`:: > (INFO) A tag has an invalid format. > > diff --git a/fsck.h b/fsck.h > index b85072df57..5ea874916d 100644 > --- a/fsck.h > +++ b/fsck.h > @@ -34,6 +34,7 @@ enum fsck_msg_type { > FUNC(BAD_REF_CONTENT, ERROR) \ > FUNC(BAD_REF_FILETYPE, ERROR) \ > FUNC(BAD_REF_NAME, ERROR) \ > + FUNC(BAD_SYMREF_TARGET, ERROR) \ > FUNC(BAD_TIMEZONE, ERROR) \ > FUNC(BAD_TREE, ERROR) \ > FUNC(BAD_TREE_SHA1, ERROR) \ > diff --git a/refs/files-backend.c b/refs/files-backend.c > index 0187b85c5f..fef32e607f 100644 > --- a/refs/files-backend.c > +++ b/refs/files-backend.c > @@ -3434,11 +3434,80 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, > const char *refs_check_dir, > struct dir_iterator *iter); > > +/* > + * Check the symref "referent" and "referent_path". For textual symref, > + * "referent" would be the content after "refs:". > + */ > +static int files_fsck_symref_target(struct fsck_options *o, > + struct fsck_ref_report *report, > + struct strbuf *referent, > + struct strbuf *referent_path) > +{ > + size_t len = referent->len - 1; > + const char *p = NULL; > + struct stat st; > + int ret = 0; > + > + if (!skip_prefix(referent->buf, "refs/", &p)) { > + There's a superfluous newline here. Also, you never use the value of `p`, so you can instead use `starts_with()`. > + ret = fsck_report_ref(o, report, > + FSCK_MSG_BAD_SYMREF_TARGET, > + "points to ref outside the refs directory"); > + goto out; > + } > + > + if (referent->buf[referent->len - 1] != '\n') { > + ret = fsck_report_ref(o, report, > + FSCK_MSG_REF_MISSING_NEWLINE, > + "missing newline"); > + len++; > + } > + > + strbuf_rtrim(referent); > + if (check_refname_format(referent->buf, 0)) { > + ret = fsck_report_ref(o, report, > + FSCK_MSG_BAD_SYMREF_TARGET, > + "points to refname with invalid format"); > + goto out; > + } > + > + if (len != referent->len) { > + ret = fsck_report_ref(o, report, > + FSCK_MSG_TRAILING_REF_CONTENT, > + "trailing garbage in ref"); > + } > + > + /* > + * Missing target should not be treated as any error worthy event and > + * not even warn. It is a common case that a symbolic ref points to a > + * ref that does not exist yet. If the target ref does not exist, just > + * skip the check for the file type. > + */ > + if (lstat(referent_path->buf, &st)) > + goto out; We may also want to verify that `errno == ENOENT` here. Patrick
On Mon, Sep 09, 2024 at 05:04:11PM +0200, Patrick Steinhardt wrote: > > In order to provide above checks, we will first check whether the symref > > content misses the newline by peeking the last byte of the "referent" to > > see whether it is '\n'. > > I'd still argue that we should do the same retroactive tightening as we > introduce for normal references, also with an INFO level at first. > Otherwise we're being inconsistent across the ref types. > Actually, for above situations, we will use the same fsck error message ids introduce in [PATCH v3 2/4]. And I think we must refer to this in this commit message. But it makes me wonder should we use a new commit to introduce these two fsck message ids? > > diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt > > index 06d045ac48..beb6c4e49e 100644 > > --- a/Documentation/fsck-msgids.txt > > +++ b/Documentation/fsck-msgids.txt > > @@ -28,6 +28,10 @@ > > `badRefName`:: > > (ERROR) A ref has an invalid format. > > > > +`badSymrefTarget`:: > > + (ERROR) The symref target points outside the ref directory or > > + the name of the symref target is invalid. > > These are two separate error cases, and we even have different code > paths raising them. Shouldn't we thus also have two different diagnostic > codes for this? > I agree. I will improve in the next version. > > + if (!skip_prefix(referent->buf, "refs/", &p)) { > > + > > There's a superfluous newline here. > > Also, you never use the value of `p`, so you can instead use > `starts_with()`. > Thanks, actually I have searched the code with "is_prefix". Well, I didn't think about "starts_<>". > > + /* > > + * Missing target should not be treated as any error worthy event and > > + * not even warn. It is a common case that a symbolic ref points to a > > + * ref that does not exist yet. If the target ref does not exist, just > > + * skip the check for the file type. > > + */ > > + if (lstat(referent_path->buf, &st)) > > + goto out; > > We may also want to verify that `errno == ENOENT` here. > I agree, if "errno != ENOENT", we should report to the user about this ref-unrelated failure. Thanks, Jialuo
shejialuo <shejialuo@gmail.com> writes: [snip] > And we will remember the untrimmed length of the "referent" and call > "strbuf_rtrim()" on "referent". Then, we will call "check_refname_format" > to chceck whether the trimmed referent format is valid. If not, we will s/chceck/check > report to the user that the symref points to referent which has invalid > format. If it is valid, we will compare the untrimmed length and trimmed > length, if they are not the same, we need to warn the user there is some > trailing garbage in the symref content. > > At last, we need to check whether the referent is the directory. We s/is the/is a/ > cannot distinguish whether the "refs/heads/a" is a directory or not by It would be a little clearer if we say We cannot distinguish whether a given reference like 'refs/heads/a' is a file or a directory. > using "check_refname_format". We have already checked bad file type when > iterating the "refs/" directory but we ignore the directory. Thus, we > need to explicitly add check here. > [snip] > +/* > + * Check the symref "referent" and "referent_path". For textual symref, > + * "referent" would be the content after "refs:". > + */ > +static int files_fsck_symref_target(struct fsck_options *o, > + struct fsck_ref_report *report, > + struct strbuf *referent, > + struct strbuf *referent_path) > +{ > + size_t len = referent->len - 1; > + const char *p = NULL; > + struct stat st; > + int ret = 0; > + > + if (!skip_prefix(referent->buf, "refs/", &p)) { > + > + ret = fsck_report_ref(o, report, > + FSCK_MSG_BAD_SYMREF_TARGET, > + "points to ref outside the refs directory"); > + goto out; > + } > + > + if (referent->buf[referent->len - 1] != '\n') { > + ret = fsck_report_ref(o, report, > + FSCK_MSG_REF_MISSING_NEWLINE, > + "missing newline"); > + len++; > + } > + > + strbuf_rtrim(referent); > + if (check_refname_format(referent->buf, 0)) { > + ret = fsck_report_ref(o, report, > + FSCK_MSG_BAD_SYMREF_TARGET, > + "points to refname with invalid format"); > + goto out; > + } > + > + if (len != referent->len) { Would this work with a symref containing: ref: refs/heads/feature\ngarbage\n Since we check last character and rtrim, wouldn't this bypass our checks? Isn't it better to find the first `\n` and check if the index < referent->len? > + ret = fsck_report_ref(o, report, > + FSCK_MSG_TRAILING_REF_CONTENT, > + "trailing garbage in ref"); > + } > + > + /* > + * Missing target should not be treated as any error worthy event and > + * not even warn. It is a common case that a symbolic ref points to a > + * ref that does not exist yet. If the target ref does not exist, just > + * skip the check for the file type. > + */ I think the common terminology for this is 'dangling symref'. Perhaps we could shorten this to simply say: Dangling symrefs are common and so we don't report them. > + if (lstat(referent_path->buf, &st)) > + goto out; > + > + /* > + * We cannot distinguish whether "refs/heads/a" is directory or nots by s/is/is a/ s/nots/not/ > + * using "check_refname_format(referent->buf, 0)". Instead, we need to > + * check the file type of the target. > + */ > + if (S_ISDIR(st.st_mode)) { > + ret = fsck_report_ref(o, report, > + FSCK_MSG_BAD_SYMREF_TARGET, > + "points to the directory"); > + goto out; > + } > + > +out: > + return ret; > +} > + [snip]
On Tue, Sep 10, 2024 at 03:19:49PM -0700, karthik nayak wrote: [snip] > > + if (referent->buf[referent->len - 1] != '\n') { > > + ret = fsck_report_ref(o, report, > > + FSCK_MSG_REF_MISSING_NEWLINE, > > + "missing newline"); > > + len++; > > + } > > + > > + strbuf_rtrim(referent); > > + if (check_refname_format(referent->buf, 0)) { > > + ret = fsck_report_ref(o, report, > > + FSCK_MSG_BAD_SYMREF_TARGET, > > + "points to refname with invalid format"); > > + goto out; > > + } > > + > > + if (len != referent->len) { > > Would this work with a symref containing: > > ref: refs/heads/feature\ngarbage\n > > Since we check last character and rtrim, wouldn't this bypass our > checks? Isn't it better to find the first `\n` and check if the index < > referent->len? > We will check the above example by "check_refname_format". It will report the following message: error: ... : badSymrefTarget: points to refname with invalid format From the context, I guess you suggest that we should report there is a trailing garbage in the ref. However, for the above situation, we should report an error which is align with the behavior of the "git-fsck(1)". So there is no need to check whether there is a trailing garbage when we encounter an error. And we cannot use this way, for example: ref: refs/heads/feature \n If we find the first '\n' index. In this example, index will be equal to "referent->len". And we totally ignore this case. > > + ret = fsck_report_ref(o, report, > > + FSCK_MSG_TRAILING_REF_CONTENT, > > + "trailing garbage in ref"); > > + } > > + > > + /* > > + * Missing target should not be treated as any error worthy event and > > + * not even warn. It is a common case that a symbolic ref points to a > > + * ref that does not exist yet. If the target ref does not exist, just > > + * skip the check for the file type. > > + */ > > I think the common terminology for this is 'dangling symref'. Perhaps we > could shorten this to simply say: > > Dangling symrefs are common and so we don't report them. > Thanks, I will improve this in the next version.
diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 06d045ac48..beb6c4e49e 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -28,6 +28,10 @@ `badRefName`:: (ERROR) A ref has an invalid format. +`badSymrefTarget`:: + (ERROR) The symref target points outside the ref directory or + the name of the symref target is invalid. + `badTagName`:: (INFO) A tag has an invalid format. diff --git a/fsck.h b/fsck.h index b85072df57..5ea874916d 100644 --- a/fsck.h +++ b/fsck.h @@ -34,6 +34,7 @@ enum fsck_msg_type { FUNC(BAD_REF_CONTENT, ERROR) \ FUNC(BAD_REF_FILETYPE, ERROR) \ FUNC(BAD_REF_NAME, ERROR) \ + FUNC(BAD_SYMREF_TARGET, ERROR) \ FUNC(BAD_TIMEZONE, ERROR) \ FUNC(BAD_TREE, ERROR) \ FUNC(BAD_TREE_SHA1, ERROR) \ diff --git a/refs/files-backend.c b/refs/files-backend.c index 0187b85c5f..fef32e607f 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3434,11 +3434,80 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, const char *refs_check_dir, struct dir_iterator *iter); +/* + * Check the symref "referent" and "referent_path". For textual symref, + * "referent" would be the content after "refs:". + */ +static int files_fsck_symref_target(struct fsck_options *o, + struct fsck_ref_report *report, + struct strbuf *referent, + struct strbuf *referent_path) +{ + size_t len = referent->len - 1; + const char *p = NULL; + struct stat st; + int ret = 0; + + if (!skip_prefix(referent->buf, "refs/", &p)) { + + ret = fsck_report_ref(o, report, + FSCK_MSG_BAD_SYMREF_TARGET, + "points to ref outside the refs directory"); + goto out; + } + + if (referent->buf[referent->len - 1] != '\n') { + ret = fsck_report_ref(o, report, + FSCK_MSG_REF_MISSING_NEWLINE, + "missing newline"); + len++; + } + + strbuf_rtrim(referent); + if (check_refname_format(referent->buf, 0)) { + ret = fsck_report_ref(o, report, + FSCK_MSG_BAD_SYMREF_TARGET, + "points to refname with invalid format"); + goto out; + } + + if (len != referent->len) { + ret = fsck_report_ref(o, report, + FSCK_MSG_TRAILING_REF_CONTENT, + "trailing garbage in ref"); + } + + /* + * Missing target should not be treated as any error worthy event and + * not even warn. It is a common case that a symbolic ref points to a + * ref that does not exist yet. If the target ref does not exist, just + * skip the check for the file type. + */ + if (lstat(referent_path->buf, &st)) + goto out; + + /* + * We cannot distinguish whether "refs/heads/a" is directory or nots by + * using "check_refname_format(referent->buf, 0)". Instead, we need to + * check the file type of the target. + */ + if (S_ISDIR(st.st_mode)) { + ret = fsck_report_ref(o, report, + FSCK_MSG_BAD_SYMREF_TARGET, + "points to the directory"); + goto out; + } + +out: + return ret; +} + static int files_fsck_refs_content(struct ref_store *ref_store, struct fsck_options *o, const char *refs_check_dir, struct dir_iterator *iter) { + struct strbuf referent_path = STRBUF_INIT; struct strbuf ref_content = STRBUF_INIT; struct strbuf referent = STRBUF_INIT; struct strbuf refname = STRBUF_INIT; @@ -3484,12 +3553,24 @@ static int files_fsck_refs_content(struct ref_store *ref_store, "trailing garbage in ref"); goto cleanup; } + } else { + strbuf_addf(&referent_path, "%s/%s", + ref_store->gitdir, referent.buf); + /* + * the referent may contain the spaces and the newline, need to + * trim for path. + */ + strbuf_rtrim(&referent_path); + ret = files_fsck_symref_target(o, &report, + &referent, + &referent_path); } cleanup: strbuf_release(&refname); strbuf_release(&ref_content); strbuf_release(&referent); + strbuf_release(&referent_path); return ret; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index a06ad044f2..e0bf8c8c8b 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -209,4 +209,121 @@ test_expect_success 'regular ref content should be checked (aggregate)' ' test_cmp expect sorted_err ' +test_expect_success 'textual symref content should be checked (individual)' ' + test_when_finished "rm -rf repo" && + git init repo && + branch_dir_prefix=.git/refs/heads && + tag_dir_prefix=.git/refs/tags && + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + printf "ref: refs/heads/branch\n" >$branch_dir_prefix/branch-good && + git refs verify 2>err && + rm $branch_dir_prefix/branch-good && + test_must_be_empty err && + + printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline-1 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-no-newline-1: refMissingNewline: missing newline + EOF + rm $branch_dir_prefix/branch-no-newline-1 && + test_cmp expect err && + + printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: missing newline + warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: trailing garbage in ref + EOF + rm $branch_dir_prefix/a/b/branch-trailing-1 && + test_cmp expect err && + + printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: trailing garbage in ref + EOF + rm $branch_dir_prefix/a/b/branch-trailing-2 && + test_cmp expect err && + + printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: trailing garbage in ref + EOF + rm $branch_dir_prefix/a/b/branch-trailing-3 && + test_cmp expect err && + + printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-complicated: refMissingNewline: missing newline + warning: refs/heads/a/b/branch-complicated: trailingRefContent: trailing garbage in ref + EOF + rm $branch_dir_prefix/a/b/branch-complicated && + test_cmp expect err && + + printf "ref: refs/heads/.branch\n" >$branch_dir_prefix/branch-bad-1 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad-1: badSymrefTarget: points to refname with invalid format + EOF + rm $branch_dir_prefix/branch-bad-1 && + test_cmp expect err && + + printf "ref: reflogs/heads/main\n" >$branch_dir_prefix/branch-bad-2 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad-2: badSymrefTarget: points to ref outside the refs directory + EOF + rm $branch_dir_prefix/branch-bad-2 && + test_cmp expect err && + + printf "ref: refs/heads/a\n" >$branch_dir_prefix/branch-bad-3 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad-3: badSymrefTarget: points to the directory + EOF + rm $branch_dir_prefix/branch-bad-3 && + test_cmp expect err +' + +test_expect_success 'textual symref content should be checked (aggregate)' ' + test_when_finished "rm -rf repo" && + git init repo && + branch_dir_prefix=.git/refs/heads && + tag_dir_prefix=.git/refs/tags && + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + printf "ref: refs/heads/branch\n" >$branch_dir_prefix/branch-good && + printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline-1 && + printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && + printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && + printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && + printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && + printf "ref: refs/heads/.branch\n" >$branch_dir_prefix/branch-bad-1 && + printf "ref: reflogs/heads/main\n" >$branch_dir_prefix/branch-bad-2 && + printf "ref: refs/heads/a\n" >$branch_dir_prefix/branch-bad-3 && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad-1: badSymrefTarget: points to refname with invalid format + error: refs/heads/branch-bad-2: badSymrefTarget: points to ref outside the refs directory + error: refs/heads/branch-bad-3: badSymrefTarget: points to the directory + warning: refs/heads/a/b/branch-complicated: refMissingNewline: missing newline + warning: refs/heads/a/b/branch-complicated: trailingRefContent: trailing garbage in ref + warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: missing newline + warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: trailing garbage in ref + warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: trailing garbage in ref + warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: trailing garbage in ref + warning: refs/heads/branch-no-newline-1: refMissingNewline: missing newline + EOF + sort err >sorted_err && + test_cmp expect sorted_err +' + test_done
We have already introduced the checks for regular refs. There is no need to check the consistency of the target which the symref points to. Instead, we just need to check the content of teh symref itself. In order to check the content of the symref, create a function "files_fsck_symref_target". It will first check whether the "referent" is under the "refs/" directory and then we will check the symref contents. A regular file is accepted as a textual symref if it begins with "ref:", followed by zero or more whitespaces, followed by the full refname, followed only by whitespace characters. We always write a single SP after "ref:" and a single LF after the refname, but third-party reimplementations of Git may have taken advantage of the looser syntax. Put it more specific, we accept the following contents of the symref: 1. "ref: refs/heads/master " 2. "ref: refs/heads/master \n \n" 3. "ref: refs/heads/master\n\n" But we do not allow any other trailing garbage. The followings are bad symref contents which will be reported as fsck error by "git-fsck(1)". 1. "ref: refs/heads/master garbage\n" 2. "ref: refs/heads/master \n\n\n garbage " In order to provide above checks, we will first check whether the symref content misses the newline by peeking the last byte of the "referent" to see whether it is '\n'. And we will remember the untrimmed length of the "referent" and call "strbuf_rtrim()" on "referent". Then, we will call "check_refname_format" to chceck whether the trimmed referent format is valid. If not, we will report to the user that the symref points to referent which has invalid format. If it is valid, we will compare the untrimmed length and trimmed length, if they are not the same, we need to warn the user there is some trailing garbage in the symref content. At last, we need to check whether the referent is the directory. We cannot distinguish whether the "refs/heads/a" is a directory or not by using "check_refname_format". We have already checked bad file type when iterating the "refs/" directory but we ignore the directory. Thus, we need to explicitly add check here. Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: shejialuo <shejialuo@gmail.com> --- Documentation/fsck-msgids.txt | 4 ++ fsck.h | 1 + refs/files-backend.c | 81 +++++++++++++++++++++++ t/t0602-reffiles-fsck.sh | 117 ++++++++++++++++++++++++++++++++++ 4 files changed, 203 insertions(+)