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 <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:52 UTC a6354e6048269bf163c957c607a29db55917aaec
4 files changed +155
Documentation/fsck-msgids.txt
+3
@@ -28,6 +28,9 @@
28 `badRefName`::
29 (ERROR) A ref has an invalid format.
30
31 +`badReferentName`::
32 + (ERROR) The referent name of a symref is invalid.
33 +
34 `badTagName`::
35 (INFO) A tag has an invalid format.
36
fsck.h
+1
@@ -34,6 +34,7 @@ enum fsck_msg_type {
34 FUNC(BAD_REF_CONTENT, ERROR) \
35 FUNC(BAD_REF_FILETYPE, ERROR) \
36 FUNC(BAD_REF_NAME, ERROR) \
37 + FUNC(BAD_REFERENT_NAME, ERROR) \
38 FUNC(BAD_TIMEZONE, ERROR) \
39 FUNC(BAD_TREE, ERROR) \
40 FUNC(BAD_TREE_SHA1, ERROR) \
refs/files-backend.c
+40
@@ -3509,6 +3509,43 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store,
3509 const char *refname,
3510 struct dir_iterator *iter);
3511
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 +
3549 static int files_fsck_refs_content(struct ref_store *ref_store,
3550 struct fsck_options *o,
3551 const char *target_name,
@@ -3563,6 +3600,9 @@ static int files_fsck_refs_content(struct ref_store *ref_store,
3600 "has trailing garbage: '%s'", trailing);
3601 goto cleanup;
3602 }
3603 + } else {
3604 + ret = files_fsck_symref_target(o, &report, &referent);
3605 + goto cleanup;
3606 }
3607
3608 cleanup:
t/t0602-reffiles-fsck.sh
+111
@@ -263,6 +263,109 @@ test_expect_success 'regular ref content should be checked (aggregate)' '
263 test_cmp expect sorted_err
264 '
265
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 +
369 test_expect_success 'ref content checks should work with worktrees' '
370 test_when_finished "rm -rf repo" &&
371 git init repo &&
@@ -313,6 +416,14 @@ test_expect_success 'ref content checks should work with worktrees' '
416 warning: worktrees/worktree-1/refs/worktree/branch-no-newline: refMissingNewline: misses LF at the end
417 EOF
418 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 &&
427 test_cmp expect err
428 '
429