ref: check the full refname instead of basename
In "files-backend.c::files_fsck_refs_name", we validate the refname format by using "check_refname_format" to check the basename of the iterator with "REFNAME_ALLOW_ONELEVEL" flag. However, this is a bad implementation. Although we doesn't allow a single "@" in ".git" directory, we do allow "refs/heads/@". So, we will report an error wrongly when there is a "refs/heads/@" ref by using one level refname "@". Because we just check one level refname, we either cannot check the other parts of the full refname. And we will ignore the following errors: "refs/heads/ new-feature/test" "refs/heads/~new-feature/test" In order to fix the above problem, enhance "files_fsck_refs_name" to use the full name for "check_refname_format". Then, replace the tests which are related to "@" and add tests to exercise the above situations using for loop to avoid repetition. 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
Nov 20, 2024 at 19:51 UTC
32dc1c7ec3c4e92704f221abbfee5cc1d50bf473
2 files changed
+60
-39
refs/files-backend.c
+5
-2
@@ -3519,10 +3519,13 @@ static int files_fsck_refs_name(struct ref_store *ref_store UNUSED,
3519
if (iter->basename[0] != '.' && ends_with(iter->basename, ".lock"))
3520
goto cleanup;
3521
3522
- if (check_refname_format(iter->basename, REFNAME_ALLOW_ONELEVEL)) {
3522
+ /*
3523
+ * This works right now because we never check the root refs.
3524
+ */
3525
+ strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path);
3526
+ if (check_refname_format(sb.buf, 0)) {
3527
struct fsck_ref_report report = { 0 };
3528
3525
- strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path);
3529
report.path = sb.buf;
3530
ret = fsck_report_ref(o, &report,
3531
FSCK_MSG_BAD_REF_NAME,
t/t0602-reffiles-fsck.sh
+55
-37
@@ -18,63 +18,81 @@ test_expect_success 'ref name should be checked' '
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 &&
21
+ git checkout -b default-branch &&
22
+ git tag default-tag &&
23
+ git tag multi_hierarchy/default-tag &&
24
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
25
+ cp $branch_dir_prefix/default-branch $branch_dir_prefix/@ &&
26
+ git refs verify 2>err &&
27
+ test_must_be_empty err &&
28
rm $branch_dir_prefix/@ &&
42
- test_cmp expect err &&
29
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 &&
30
+ cp $tag_dir_prefix/default-tag $tag_dir_prefix/tag-1.lock &&
31
git refs verify 2>err &&
32
rm $tag_dir_prefix/tag-1.lock &&
33
test_must_be_empty err &&
34
57
- cp $tag_dir_prefix/tag-1 $tag_dir_prefix/.lock &&
35
+ cp $tag_dir_prefix/default-tag $tag_dir_prefix/.lock &&
36
test_must_fail git refs verify 2>err &&
37
cat >expect <<-EOF &&
38
error: refs/tags/.lock: badRefName: invalid refname format
39
EOF
40
rm $tag_dir_prefix/.lock &&
63
- test_cmp expect err
41
+ test_cmp expect err &&
42
+
43
+ for refname in ".refname-starts-with-dot" "~refname-has-stride"
44
+ do
45
+ cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname" &&
46
+ test_must_fail git refs verify 2>err &&
47
+ cat >expect <<-EOF &&
48
+ error: refs/heads/$refname: badRefName: invalid refname format
49
+ EOF
50
+ rm "$branch_dir_prefix/$refname" &&
51
+ test_cmp expect err || return 1
52
+ done &&
53
+
54
+ for refname in ".refname-starts-with-dot" "~refname-has-stride"
55
+ do
56
+ cp $tag_dir_prefix/default-tag "$tag_dir_prefix/$refname" &&
57
+ test_must_fail git refs verify 2>err &&
58
+ cat >expect <<-EOF &&
59
+ error: refs/tags/$refname: badRefName: invalid refname format
60
+ EOF
61
+ rm "$tag_dir_prefix/$refname" &&
62
+ test_cmp expect err || return 1
63
+ done &&
64
+
65
+ for refname in ".refname-starts-with-dot" "~refname-has-stride"
66
+ do
67
+ cp $tag_dir_prefix/multi_hierarchy/default-tag "$tag_dir_prefix/multi_hierarchy/$refname" &&
68
+ test_must_fail git refs verify 2>err &&
69
+ cat >expect <<-EOF &&
70
+ error: refs/tags/multi_hierarchy/$refname: badRefName: invalid refname format
71
+ EOF
72
+ rm "$tag_dir_prefix/multi_hierarchy/$refname" &&
73
+ test_cmp expect err || return 1
74
+ done &&
75
+
76
+ for refname in ".refname-starts-with-dot" "~refname-has-stride"
77
+ do
78
+ mkdir "$branch_dir_prefix/$refname" &&
79
+ cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname/default-branch" &&
80
+ test_must_fail git refs verify 2>err &&
81
+ cat >expect <<-EOF &&
82
+ error: refs/heads/$refname/default-branch: badRefName: invalid refname format
83
+ EOF
84
+ rm -r "$branch_dir_prefix/$refname" &&
85
+ test_cmp expect err || return 1
86
+ done
87
'
88
89
test_expect_success 'ref name check should be adapted into fsck messages' '
90
test_when_finished "rm -rf repo" &&
91
git init repo &&
92
branch_dir_prefix=.git/refs/heads &&
70
- tag_dir_prefix=.git/refs/tags &&
93
cd repo &&
94
git commit --allow-empty -m initial &&
95
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 &&
96
97
cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 &&
98
git -c fsck.badRefName=warn refs verify 2>err &&
@@ -84,7 +102,7 @@ test_expect_success 'ref name check should be adapted into fsck messages' '
102
rm $branch_dir_prefix/.branch-1 &&
103
test_cmp expect err &&
104
87
- cp $branch_dir_prefix/branch-1 $branch_dir_prefix/@ &&
105
+ cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 &&
106
git -c fsck.badRefName=ignore refs verify 2>err &&
107
test_must_be_empty err
108
'