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>

shejialuo committed Aug 8, 2024 at 19:31 UTC 1c31be45b3b263670c7d2a91c27cc119b77dd2e2
4 files changed +127
Documentation/fsck-msgids.txt
+3
@@ -22,6 +22,9 @@
22 `badRefFiletype`::
23 (ERROR) A ref has a bad file type.
24
25 +`badRefName`::
26 + (ERROR) A ref has an invalid format.
27 +
28 `badTagName`::
29 (INFO) A tag has an invalid format.
30
fsck.h
+1
@@ -32,6 +32,7 @@ enum fsck_msg_type {
32 FUNC(BAD_OBJECT_SHA1, ERROR) \
33 FUNC(BAD_PARENT_SHA1, ERROR) \
34 FUNC(BAD_REF_FILETYPE, ERROR) \
35 + FUNC(BAD_REF_NAME, ERROR) \
36 FUNC(BAD_TIMEZONE, ERROR) \
37 FUNC(BAD_TREE, ERROR) \
38 FUNC(BAD_TREE_SHA1, ERROR) \
refs/files-backend.c
+31
@@ -3419,6 +3419,36 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store,
3419 const char *refs_check_dir,
3420 struct dir_iterator *iter);
3421
3422 +static int files_fsck_refs_name(struct ref_store *ref_store UNUSED,
3423 + struct fsck_options *o,
3424 + const char *refs_check_dir,
3425 + struct dir_iterator *iter)
3426 +{
3427 + struct strbuf sb = STRBUF_INIT;
3428 + int ret = 0;
3429 +
3430 + /*
3431 + * Ignore the files ending with ".lock" as they may be lock files
3432 + * However, do not allow bare ".lock" files.
3433 + */
3434 + if (iter->basename[0] != '.' && ends_with(iter->basename, ".lock"))
3435 + goto cleanup;
3436 +
3437 + if (check_refname_format(iter->basename, REFNAME_ALLOW_ONELEVEL)) {
3438 + struct fsck_ref_report report = { .path = NULL };
3439 +
3440 + strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path);
3441 + report.path = sb.buf;
3442 + ret = fsck_report_ref(o, &report,
3443 + FSCK_MSG_BAD_REF_NAME,
3444 + "invalid refname format");
3445 + }
3446 +
3447 +cleanup:
3448 + strbuf_release(&sb);
3449 + return ret;
3450 +}
3451 +
3452 static int files_fsck_refs_dir(struct ref_store *ref_store,
3453 struct fsck_options *o,
3454 const char *refs_check_dir,
@@ -3470,6 +3500,7 @@ static int files_fsck_refs(struct ref_store *ref_store,
3500 struct fsck_options *o)
3501 {
3502 files_fsck_refs_fn fsck_refs_fn[]= {
3503 + files_fsck_refs_name,
3504 NULL,
3505 };
3506
t/t0602-reffiles-fsck.sh new
+92
@@ -0,0 +1,92 @@
1 +#!/bin/sh
2 +
3 +test_description='Test reffiles backend consistency check'
4 +
5 +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
6 +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
7 +GIT_TEST_DEFAULT_REF_FORMAT=files
8 +export GIT_TEST_DEFAULT_REF_FORMAT
9 +TEST_PASSES_SANITIZE_LEAK=true
10 +
11 +. ./test-lib.sh
12 +
13 +test_expect_success 'ref name should be checked' '
14 + test_when_finished "rm -rf repo" &&
15 + git init repo &&
16 + branch_dir_prefix=.git/refs/heads &&
17 + tag_dir_prefix=.git/refs/tags &&
18 + cd repo &&
19 +
20 + git commit --allow-empty -m initial &&
21 + git checkout -b branch-1 &&
22 + git tag tag-1 &&
23 + git commit --allow-empty -m second &&
24 + git checkout -b branch-2 &&
25 + git tag tag-2 &&
26 + git tag multi_hierarchy/tag-2 &&
27 +
28 + cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 &&
29 + test_must_fail git refs verify 2>err &&
30 + cat >expect <<-EOF &&
31 + error: refs/heads/.branch-1: badRefName: invalid refname format
32 + EOF
33 + rm $branch_dir_prefix/.branch-1 &&
34 + test_cmp expect err &&
35 +
36 + cp $branch_dir_prefix/branch-1 $branch_dir_prefix/@ &&
37 + test_must_fail git refs verify 2>err &&
38 + cat >expect <<-EOF &&
39 + error: refs/heads/@: badRefName: invalid refname format
40 + EOF
41 + rm $branch_dir_prefix/@ &&
42 + test_cmp expect err &&
43 +
44 + cp $tag_dir_prefix/multi_hierarchy/tag-2 $tag_dir_prefix/multi_hierarchy/@ &&
45 + test_must_fail git refs verify 2>err &&
46 + cat >expect <<-EOF &&
47 + error: refs/tags/multi_hierarchy/@: badRefName: invalid refname format
48 + EOF
49 + rm $tag_dir_prefix/multi_hierarchy/@ &&
50 + test_cmp expect err &&
51 +
52 + cp $tag_dir_prefix/tag-1 $tag_dir_prefix/tag-1.lock &&
53 + git refs verify 2>err &&
54 + rm $tag_dir_prefix/tag-1.lock &&
55 + test_must_be_empty err &&
56 +
57 + cp $tag_dir_prefix/tag-1 $tag_dir_prefix/.lock &&
58 + test_must_fail git refs verify 2>err &&
59 + cat >expect <<-EOF &&
60 + error: refs/tags/.lock: badRefName: invalid refname format
61 + EOF
62 + rm $tag_dir_prefix/.lock &&
63 + test_cmp expect err
64 +'
65 +
66 +test_expect_success 'ref name check should be adapted into fsck messages' '
67 + test_when_finished "rm -rf repo" &&
68 + git init repo &&
69 + branch_dir_prefix=.git/refs/heads &&
70 + tag_dir_prefix=.git/refs/tags &&
71 + cd repo &&
72 + git commit --allow-empty -m initial &&
73 + git checkout -b branch-1 &&
74 + git tag tag-1 &&
75 + git commit --allow-empty -m second &&
76 + git checkout -b branch-2 &&
77 + git tag tag-2 &&
78 +
79 + cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 &&
80 + git -c fsck.badRefName=warn refs verify 2>err &&
81 + cat >expect <<-EOF &&
82 + warning: refs/heads/.branch-1: badRefName: invalid refname format
83 + EOF
84 + rm $branch_dir_prefix/.branch-1 &&
85 + test_cmp expect err &&
86 +
87 + cp $branch_dir_prefix/branch-1 $branch_dir_prefix/@ &&
88 + git -c fsck.badRefName=ignore refs verify 2>err &&
89 + test_must_be_empty err
90 +'
91 +
92 +test_done