fsck: add ref name check for files backend

The git-fsck(1) only implicitly checks the reference, it does not fully
check refs with bad format name such as standalone "@".

However, a file ending with ".lock" should not be marked as having a bad
ref name. It is expected that concurrent writers may have such lock files.
We currently ignore this situation. But for bare ".lock" file, we will
report it as error.

In order to provide such checks, add a new fsck message id "badRefName"
with default ERROR type. Use existing "check_refname_format" to explicit
check the ref name. And add a new unit test to verify the functionality.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: shejialuo <shejialuo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt
index 7c809fddf1aa0c6e9d468eb8d9eb8c70559f0420..68a2801f1529dfdc310337fd1eebc742860d84ab 100644 (file)
--- a/Documentation/fsck-msgids.txt
+++ b/Documentation/fsck-msgids.txt
@@ -22,6 +22,9 @@
 `badRefFiletype`::
        (ERROR) A ref has a bad file type.
 
+`badRefName`::
+       (ERROR) A ref has an invalid format.
+
 `badTagName`::
        (INFO) A tag has an invalid format.
 

diff --git a/fsck.h b/fsck.h
index af02174973b054296298e1fb806bcb4cb65e2a79..500b4c04d2cbf26fecc0b16ff9b4907fe56b4fc4 100644 (file)
--- a/fsck.h
+++ b/fsck.h
@@ -32,6 +32,7 @@ enum fsck_msg_type {
        FUNC(BAD_OBJECT_SHA1, ERROR) \
        FUNC(BAD_PARENT_SHA1, ERROR) \
        FUNC(BAD_REF_FILETYPE, ERROR) \
+       FUNC(BAD_REF_NAME, 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 e511e1dcce3ef55ea44a9318701864343c0baa8c..7f6eefa960b147e31a4a5148de9656cc0d1a7469 100644 (file)
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3419,6 +3419,36 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store,
                                  const char *refs_check_dir,
                                  struct dir_iterator *iter);
 
+static int files_fsck_refs_name(struct ref_store *ref_store UNUSED,
+                               struct fsck_options *o,
+                               const char *refs_check_dir,
+                               struct dir_iterator *iter)
+{
+       struct strbuf sb = STRBUF_INIT;
+       int ret = 0;
+
+       /*
+        * Ignore the files ending with ".lock" as they may be lock files
+        * However, do not allow bare ".lock" files.
+        */
+       if (iter->basename[0] != '.' && ends_with(iter->basename, ".lock"))
+               goto cleanup;
+
+       if (check_refname_format(iter->basename, REFNAME_ALLOW_ONELEVEL)) {
+               struct fsck_ref_report report = { .path = NULL };
+
+               strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path);
+               report.path = sb.buf;
+               ret = fsck_report_ref(o, &report,
+                                     FSCK_MSG_BAD_REF_NAME,
+                                     "invalid refname format");
+       }
+
+cleanup:
+       strbuf_release(&sb);
+       return ret;
+}
+
 static int files_fsck_refs_dir(struct ref_store *ref_store,
                               struct fsck_options *o,
                               const char *refs_check_dir,
@@ -3470,6 +3500,7 @@ static int files_fsck_refs(struct ref_store *ref_store,
                           struct fsck_options *o)
 {
        files_fsck_refs_fn fsck_refs_fn[]= {
+               files_fsck_refs_name,
                NULL,
        };
 

diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh
new file mode 100755 (executable)
index 0000000..71a4d1a
--- /dev/null
+++ b/t/t0602-reffiles-fsck.sh
@@ -0,0 +1,92 @@
+#!/bin/sh
+
+test_description='Test reffiles backend consistency check'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_REF_FORMAT=files
+export GIT_TEST_DEFAULT_REF_FORMAT
+TEST_PASSES_SANITIZE_LEAK=true
+
+. ./test-lib.sh
+
+test_expect_success 'ref name 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 commit --allow-empty -m second &&
+       git checkout -b branch-2 &&
+       git tag tag-2 &&
+       git tag multi_hierarchy/tag-2 &&
+
+       cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 &&
+       test_must_fail git refs verify 2>err &&
+       cat >expect <<-EOF &&
+       error: refs/heads/.branch-1: badRefName: invalid refname format
+       EOF
+       rm $branch_dir_prefix/.branch-1 &&
+       test_cmp expect err &&
+
+       cp $branch_dir_prefix/branch-1 $branch_dir_prefix/@ &&
+       test_must_fail git refs verify 2>err &&
+       cat >expect <<-EOF &&
+       error: refs/heads/@: badRefName: invalid refname format
+       EOF
+       rm $branch_dir_prefix/@ &&
+       test_cmp expect err &&
+
+       cp $tag_dir_prefix/multi_hierarchy/tag-2 $tag_dir_prefix/multi_hierarchy/@ &&
+       test_must_fail git refs verify 2>err &&
+       cat >expect <<-EOF &&
+       error: refs/tags/multi_hierarchy/@: badRefName: invalid refname format
+       EOF
+       rm $tag_dir_prefix/multi_hierarchy/@ &&
+       test_cmp expect err &&
+
+       cp $tag_dir_prefix/tag-1 $tag_dir_prefix/tag-1.lock &&
+       git refs verify 2>err &&
+       rm $tag_dir_prefix/tag-1.lock &&
+       test_must_be_empty err &&
+
+       cp $tag_dir_prefix/tag-1 $tag_dir_prefix/.lock &&
+       test_must_fail git refs verify 2>err &&
+       cat >expect <<-EOF &&
+       error: refs/tags/.lock: badRefName: invalid refname format
+       EOF
+       rm $tag_dir_prefix/.lock &&
+       test_cmp expect err
+'
+
+test_expect_success 'ref name check should be adapted into fsck messages' '
+       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 commit --allow-empty -m second &&
+       git checkout -b branch-2 &&
+       git tag tag-2 &&
+
+       cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 &&
+       git -c fsck.badRefName=warn refs verify 2>err &&
+       cat >expect <<-EOF &&
+       warning: refs/heads/.branch-1: badRefName: invalid refname format
+       EOF
+       rm $branch_dir_prefix/.branch-1 &&
+       test_cmp expect err &&
+
+       cp $branch_dir_prefix/branch-1 $branch_dir_prefix/@ &&
+       git -c fsck.badRefName=ignore refs verify 2>err &&
+       test_must_be_empty err
+'
+
+test_done