Skip to content

Commit a6354e6

Browse files
shejialuogitster
shejialuo
authored and
gitster
committed
ref: add basic symref content check for files backend
We have code that checks regular ref contents, but we do not yet check the contents of symbolic refs. By using "parse_loose_ref_content" for symbolic refs, we will get the information of the "referent". We do not need to check the "referent" by opening the file. This is because if "referent" exists in the file system, we will eventually check its correctness by inspecting every file in the "refs" directory. If the "referent" does not exist in the filesystem, this is OK as it is seen as the dangling symref. So we just need to check the "referent" string content. A regular ref could be 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. However, we always write a single SP after "ref:" and a single LF after the refname. It may seem that we should report a fsck error message when the "referent" does not apply above rules and we should not be so aggressive because third-party reimplementations of Git may have taken advantage of the looser syntax. Put it more specific, we accept the following contents: 1. "ref: refs/heads/master " 2. "ref: refs/heads/master \n \n" 3. "ref: refs/heads/master\n\n" When introducing the regular ref content checks, we created two fsck infos "refMissingNewline" and "trailingRefContent" which exactly represents above situations. So we will reuse these two fsck messages to write checks to info the user about these situations. 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 " And we introduce a new "badReferentName(ERROR)" fsck message to report above errors by using "is_root_ref" and "check_refname_format" to check the "referent". Since both "is_root_ref" and "check_refname_format" don't work with whitespaces, we use the trimmed version of "referent" with these functions. In order to add checks, we will do the following things: 1. Record the untrimmed length "orig_len" and untrimmed last byte "orig_last_byte". 2. Use "strbuf_rtrim" to trim the whitespaces or newlines to make sure "is_root_ref" and "check_refname_format" won't be failed by them. 3. Use "orig_len" and "orig_last_byte" to check whether the "referent" misses '\n' at the end or it has trailing whitespaces or newlines. 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 1c0e2a0 commit a6354e6

File tree

4 files changed

+155
-0
lines changed

4 files changed

+155
-0
lines changed

‎Documentation/fsck-msgids.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@
2828
`badRefName`::
2929
(ERROR) A ref has an invalid format.
3030

31+
`badReferentName`::
32+
(ERROR) The referent name of a symref is invalid.
33+
3134
`badTagName`::
3235
(INFO) A tag has an invalid format.
3336

‎fsck.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ enum fsck_msg_type {
3434
FUNC(BAD_REF_CONTENT, ERROR) \
3535
FUNC(BAD_REF_FILETYPE, ERROR) \
3636
FUNC(BAD_REF_NAME, ERROR) \
37+
FUNC(BAD_REFERENT_NAME, ERROR) \
3738
FUNC(BAD_TIMEZONE, ERROR) \
3839
FUNC(BAD_TREE, ERROR) \
3940
FUNC(BAD_TREE_SHA1, ERROR) \

‎refs/files-backend.c

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3509,6 +3509,43 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store,
35093509
const char *refname,
35103510
struct dir_iterator *iter);
35113511

3512+
static int files_fsck_symref_target(struct fsck_options *o,
3513+
struct fsck_ref_report *report,
3514+
struct strbuf *referent)
3515+
{
3516+
char orig_last_byte;
3517+
size_t orig_len;
3518+
int ret = 0;
3519+
3520+
orig_len = referent->len;
3521+
orig_last_byte = referent->buf[orig_len - 1];
3522+
strbuf_rtrim(referent);
3523+
3524+
if (!is_root_ref(referent->buf) &&
3525+
check_refname_format(referent->buf, 0)) {
3526+
ret = fsck_report_ref(o, report,
3527+
FSCK_MSG_BAD_REFERENT_NAME,
3528+
"points to invalid refname '%s'", referent->buf);
3529+
goto out;
3530+
}
3531+
3532+
if (referent->len == orig_len ||
3533+
(referent->len < orig_len && orig_last_byte != '\n')) {
3534+
ret = fsck_report_ref(o, report,
3535+
FSCK_MSG_REF_MISSING_NEWLINE,
3536+
"misses LF at the end");
3537+
}
3538+
3539+
if (referent->len != orig_len && referent->len != orig_len - 1) {
3540+
ret = fsck_report_ref(o, report,
3541+
FSCK_MSG_TRAILING_REF_CONTENT,
3542+
"has trailing whitespaces or newlines");
3543+
}
3544+
3545+
out:
3546+
return ret;
3547+
}
3548+
35123549
static int files_fsck_refs_content(struct ref_store *ref_store,
35133550
struct fsck_options *o,
35143551
const char *target_name,
@@ -3563,6 +3600,9 @@ static int files_fsck_refs_content(struct ref_store *ref_store,
35633600
"has trailing garbage: '%s'", trailing);
35643601
goto cleanup;
35653602
}
3603+
} else {
3604+
ret = files_fsck_symref_target(o, &report, &referent);
3605+
goto cleanup;
35663606
}
35673607

35683608
cleanup:

‎t/t0602-reffiles-fsck.sh

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,109 @@ test_expect_success 'regular ref content should be checked (aggregate)' '
263263
test_cmp expect sorted_err
264264
'
265265

266+
test_expect_success 'textual symref content should be checked (individual)' '
267+
test_when_finished "rm -rf repo" &&
268+
git init repo &&
269+
branch_dir_prefix=.git/refs/heads &&
270+
cd repo &&
271+
test_commit default &&
272+
mkdir -p "$branch_dir_prefix/a/b" &&
273+
274+
for good_referent in "refs/heads/branch" "HEAD"
275+
do
276+
printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good &&
277+
git refs verify 2>err &&
278+
rm $branch_dir_prefix/branch-good &&
279+
test_must_be_empty err || return 1
280+
done &&
281+
282+
for bad_referent in "refs/heads/.branch" "refs/heads/~branch" "refs/heads/?branch"
283+
do
284+
printf "ref: %s\n" $bad_referent >$branch_dir_prefix/branch-bad &&
285+
test_must_fail git refs verify 2>err &&
286+
cat >expect <<-EOF &&
287+
error: refs/heads/branch-bad: badReferentName: points to invalid refname '\''$bad_referent'\''
288+
EOF
289+
rm $branch_dir_prefix/branch-bad &&
290+
test_cmp expect err || return 1
291+
done &&
292+
293+
printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline &&
294+
git refs verify 2>err &&
295+
cat >expect <<-EOF &&
296+
warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end
297+
EOF
298+
rm $branch_dir_prefix/branch-no-newline &&
299+
test_cmp expect err &&
300+
301+
printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 &&
302+
git refs verify 2>err &&
303+
cat >expect <<-EOF &&
304+
warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end
305+
warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines
306+
EOF
307+
rm $branch_dir_prefix/a/b/branch-trailing-1 &&
308+
test_cmp expect err &&
309+
310+
printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 &&
311+
git refs verify 2>err &&
312+
cat >expect <<-EOF &&
313+
warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines
314+
EOF
315+
rm $branch_dir_prefix/a/b/branch-trailing-2 &&
316+
test_cmp expect err &&
317+
318+
printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 &&
319+
git refs verify 2>err &&
320+
cat >expect <<-EOF &&
321+
warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines
322+
EOF
323+
rm $branch_dir_prefix/a/b/branch-trailing-3 &&
324+
test_cmp expect err &&
325+
326+
printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated &&
327+
git refs verify 2>err &&
328+
cat >expect <<-EOF &&
329+
warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end
330+
warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines
331+
EOF
332+
rm $branch_dir_prefix/a/b/branch-complicated &&
333+
test_cmp expect err
334+
'
335+
336+
test_expect_success 'textual symref content should be checked (aggregate)' '
337+
test_when_finished "rm -rf repo" &&
338+
git init repo &&
339+
branch_dir_prefix=.git/refs/heads &&
340+
tag_dir_prefix=.git/refs/tags &&
341+
cd repo &&
342+
test_commit default &&
343+
mkdir -p "$branch_dir_prefix/a/b" &&
344+
345+
printf "ref: refs/heads/branch\n" >$branch_dir_prefix/branch-good &&
346+
printf "ref: HEAD\n" >$branch_dir_prefix/branch-head &&
347+
printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline-1 &&
348+
printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 &&
349+
printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 &&
350+
printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 &&
351+
printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated &&
352+
printf "ref: refs/heads/.branch\n" >$branch_dir_prefix/branch-bad-1 &&
353+
354+
test_must_fail git refs verify 2>err &&
355+
cat >expect <<-EOF &&
356+
error: refs/heads/branch-bad-1: badReferentName: points to invalid refname '\''refs/heads/.branch'\''
357+
warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end
358+
warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines
359+
warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end
360+
warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines
361+
warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines
362+
warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines
363+
warning: refs/heads/branch-no-newline-1: refMissingNewline: misses LF at the end
364+
EOF
365+
sort err >sorted_err &&
366+
test_cmp expect sorted_err
367+
'
368+
266369
test_expect_success 'ref content checks should work with worktrees' '
267370
test_when_finished "rm -rf repo" &&
268371
git init repo &&
@@ -313,6 +416,14 @@ test_expect_success 'ref content checks should work with worktrees' '
313416
warning: worktrees/worktree-1/refs/worktree/branch-no-newline: refMissingNewline: misses LF at the end
314417
EOF
315418
rm $worktree1_refdir_prefix/branch-no-newline &&
419+
test_cmp expect err &&
420+
421+
printf "%s garbage" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-garbage &&
422+
git refs verify 2>err &&
423+
cat >expect <<-EOF &&
424+
warning: worktrees/worktree-1/refs/worktree/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\''
425+
EOF
426+
rm $worktree1_refdir_prefix/branch-garbage &&
316427
test_cmp expect err
317428
'
318429

0 commit comments

Comments
 (0)