Skip to content

Commit db13f18

Browse files
shejialuogitster
shejialuo
authored and
gitster
committed
ref: port git-fsck(1) regular refs check for files backend
"git-fsck(1)" implicitly checks the ref content by passing the callback "fsck_handle_ref" to the "refs.c::refs_for_each_rawref". Then, it will check whether the ref content (eventually "oid") is valid. If not, it will report the following error to the user. error: refs/heads/main: invalid sha1 pointer 0000... And it will also report above errors when there are dangling symrefs in the repository wrongly. This does not align with the behavior of the "git symbolic-ref" command which allows users to create dangling symrefs. As we have already introduced the "git refs verify" command, we'd better check the ref content explicitly in the "git refs verify" command thus later we could remove these checks in "git-fsck(1)" and launch a subprocess to call "git refs verify" in "git-fsck(1)" to make the "git-fsck(1)" more clean. Following what "git-fsck(1)" does, add a similar check to "git refs verify". Then add a new fsck error message "badRefContent(ERROR)" to represent that a ref has an invalid content. Mentored-by: Patrick Steinhardt <[email protected]> Mentored-by: Karthik Nayak <[email protected]> Signed-off-by: shejialuo <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent d849989 commit db13f18

File tree

4 files changed

+157
-0
lines changed

4 files changed

+157
-0
lines changed

‎Documentation/fsck-msgids.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
`badParentSha1`::
2020
(ERROR) A commit object has a bad parent sha1.
2121

22+
`badRefContent`::
23+
(ERROR) A ref has bad content.
24+
2225
`badRefFiletype`::
2326
(ERROR) A ref has a bad file type.
2427

‎fsck.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ enum fsck_msg_type {
3131
FUNC(BAD_NAME, ERROR) \
3232
FUNC(BAD_OBJECT_SHA1, ERROR) \
3333
FUNC(BAD_PARENT_SHA1, ERROR) \
34+
FUNC(BAD_REF_CONTENT, ERROR) \
3435
FUNC(BAD_REF_FILETYPE, ERROR) \
3536
FUNC(BAD_REF_NAME, ERROR) \
3637
FUNC(BAD_TIMEZONE, ERROR) \

‎refs/files-backend.c‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3505,6 +3505,53 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store,
35053505
const char *refname,
35063506
struct dir_iterator *iter);
35073507

3508+
static int files_fsck_refs_content(struct ref_store *ref_store,
3509+
struct fsck_options *o,
3510+
const char *target_name,
3511+
struct dir_iterator *iter)
3512+
{
3513+
struct strbuf ref_content = STRBUF_INIT;
3514+
struct strbuf referent = STRBUF_INIT;
3515+
struct fsck_ref_report report = { 0 };
3516+
unsigned int type = 0;
3517+
int failure_errno = 0;
3518+
struct object_id oid;
3519+
int ret = 0;
3520+
3521+
report.path = target_name;
3522+
3523+
if (S_ISLNK(iter->st.st_mode))
3524+
goto cleanup;
3525+
3526+
if (strbuf_read_file(&ref_content, iter->path.buf, 0) < 0 ) {
3527+
/*
3528+
* Ref file could be removed by another concurrent process. We should
3529+
* ignore this error and continue to the next ref.
3530+
*/
3531+
if (errno == ENOENT)
3532+
goto cleanup;
3533+
3534+
ret = error_errno(_("cannot read ref file '%s': %s"),
3535+
iter->path.buf, strerror(errno));
3536+
goto cleanup;
3537+
}
3538+
3539+
if (parse_loose_ref_contents(ref_store->repo->hash_algo,
3540+
ref_content.buf, &oid, &referent,
3541+
&type, &failure_errno)) {
3542+
strbuf_rtrim(&ref_content);
3543+
ret = fsck_report_ref(o, &report,
3544+
FSCK_MSG_BAD_REF_CONTENT,
3545+
"%s", ref_content.buf);
3546+
goto cleanup;
3547+
}
3548+
3549+
cleanup:
3550+
strbuf_release(&ref_content);
3551+
strbuf_release(&referent);
3552+
return ret;
3553+
}
3554+
35083555
static int files_fsck_refs_name(struct ref_store *ref_store UNUSED,
35093556
struct fsck_options *o,
35103557
const char *refname,
@@ -3600,6 +3647,7 @@ static int files_fsck_refs(struct ref_store *ref_store,
36003647
{
36013648
files_fsck_refs_fn fsck_refs_fn[]= {
36023649
files_fsck_refs_name,
3650+
files_fsck_refs_content,
36033651
NULL,
36043652
};
36053653

‎t/t0602-reffiles-fsck.sh‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,4 +158,109 @@ test_expect_success 'ref name check should work for multiple worktrees' '
158158
done
159159
'
160160

161+
test_expect_success 'regular ref content should be checked (individual)' '
162+
test_when_finished "rm -rf repo" &&
163+
git init repo &&
164+
branch_dir_prefix=.git/refs/heads &&
165+
cd repo &&
166+
test_commit default &&
167+
mkdir -p "$branch_dir_prefix/a/b" &&
168+
169+
git refs verify 2>err &&
170+
test_must_be_empty err &&
171+
172+
for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas"
173+
do
174+
printf "%s" $bad_content >$branch_dir_prefix/branch-bad &&
175+
test_must_fail git refs verify 2>err &&
176+
cat >expect <<-EOF &&
177+
error: refs/heads/branch-bad: badRefContent: $bad_content
178+
EOF
179+
rm $branch_dir_prefix/branch-bad &&
180+
test_cmp expect err || return 1
181+
done &&
182+
183+
for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas"
184+
do
185+
printf "%s" $bad_content >$branch_dir_prefix/a/b/branch-bad &&
186+
test_must_fail git refs verify 2>err &&
187+
cat >expect <<-EOF &&
188+
error: refs/heads/a/b/branch-bad: badRefContent: $bad_content
189+
EOF
190+
rm $branch_dir_prefix/a/b/branch-bad &&
191+
test_cmp expect err || return 1
192+
done
193+
'
194+
195+
test_expect_success 'regular ref content should be checked (aggregate)' '
196+
test_when_finished "rm -rf repo" &&
197+
git init repo &&
198+
branch_dir_prefix=.git/refs/heads &&
199+
tag_dir_prefix=.git/refs/tags &&
200+
cd repo &&
201+
test_commit default &&
202+
mkdir -p "$branch_dir_prefix/a/b" &&
203+
204+
bad_content_1=$(git rev-parse main)x &&
205+
bad_content_2=xfsazqfxcadas &&
206+
bad_content_3=Xfsazqfxcadas &&
207+
printf "%s" $bad_content_1 >$tag_dir_prefix/tag-bad-1 &&
208+
printf "%s" $bad_content_2 >$tag_dir_prefix/tag-bad-2 &&
209+
printf "%s" $bad_content_3 >$branch_dir_prefix/a/b/branch-bad &&
210+
211+
test_must_fail git refs verify 2>err &&
212+
cat >expect <<-EOF &&
213+
error: refs/heads/a/b/branch-bad: badRefContent: $bad_content_3
214+
error: refs/tags/tag-bad-1: badRefContent: $bad_content_1
215+
error: refs/tags/tag-bad-2: badRefContent: $bad_content_2
216+
EOF
217+
sort err >sorted_err &&
218+
test_cmp expect sorted_err
219+
'
220+
221+
test_expect_success 'ref content checks should work with worktrees' '
222+
test_when_finished "rm -rf repo" &&
223+
git init repo &&
224+
cd repo &&
225+
test_commit default &&
226+
git branch branch-1 &&
227+
git branch branch-2 &&
228+
git branch branch-3 &&
229+
git worktree add ./worktree-1 branch-2 &&
230+
git worktree add ./worktree-2 branch-3 &&
231+
worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree &&
232+
worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree &&
233+
234+
(
235+
cd worktree-1 &&
236+
git update-ref refs/worktree/branch-4 refs/heads/branch-1
237+
) &&
238+
(
239+
cd worktree-2 &&
240+
git update-ref refs/worktree/branch-4 refs/heads/branch-1
241+
) &&
242+
243+
for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas"
244+
do
245+
printf "%s" $bad_content >$worktree1_refdir_prefix/bad-branch-1 &&
246+
test_must_fail git refs verify 2>err &&
247+
cat >expect <<-EOF &&
248+
error: worktrees/worktree-1/refs/worktree/bad-branch-1: badRefContent: $bad_content
249+
EOF
250+
rm $worktree1_refdir_prefix/bad-branch-1 &&
251+
test_cmp expect err || return 1
252+
done &&
253+
254+
for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas"
255+
do
256+
printf "%s" $bad_content >$worktree2_refdir_prefix/bad-branch-2 &&
257+
test_must_fail git refs verify 2>err &&
258+
cat >expect <<-EOF &&
259+
error: worktrees/worktree-2/refs/worktree/bad-branch-2: badRefContent: $bad_content
260+
EOF
261+
rm $worktree2_refdir_prefix/bad-branch-2 &&
262+
test_cmp expect err || return 1
263+
done
264+
'
265+
161266
test_done

0 commit comments

Comments
 (0)