All of lore.kernel.org
 help / color / mirror / Atom feed
From: shejialuo <shejialuo@gmail.com>
To: git@vger.kernel.org
Cc: Patrick Steinhardt <ps@pks.im>,
	Karthik Nayak <karthik.188@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH 01/10] files-backend: add object check for regular ref
Date: Sun, 5 Jan 2025 21:49:09 +0800	[thread overview]
Message-ID: <Z3qN1T3lJoj82ckl@ArchLinux> (raw)
In-Reply-To: <Z3qNUizvHJLgMx1y@ArchLinux>

Although we use "parse_loose_ref_content" to check whether the object id
is correct, we never parse it into the "struct object" structure thus we
ignore checking whether there is a real object existing in the repo and
whether the object type is correct.

Use "parse_object" to parse the oid for the regular ref content. If the
object does not exist, report the error to the user by reusing the fsck
message "BAD_REF_CONTENT".

Then, we need to check the type of the object. Just like "git-fsck(1)",
we only report "not a commit" error when the ref is a branch. Last,
update the test to exercise the code.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: shejialuo <shejialuo@gmail.com>
---
 refs/files-backend.c     | 50 ++++++++++++++++++++++++++++++++--------
 t/t0602-reffiles-fsck.sh | 30 ++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 10 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 64f51f0da9..0a4912c009 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -20,6 +20,7 @@
 #include "../lockfile.h"
 #include "../object.h"
 #include "../object-file.h"
+#include "../packfile.h"
 #include "../path.h"
 #include "../dir.h"
 #include "../chdir-notify.h"
@@ -3589,6 +3590,34 @@ static int files_fsck_symref_target(struct fsck_options *o,
 	return ret;
 }
 
+static int files_fsck_refs_oid(struct fsck_options *o,
+			       struct ref_store *ref_store,
+			       struct fsck_ref_report report,
+			       const char *target_name,
+			       struct object_id *oid)
+{
+	struct object *obj;
+	int ret = 0;
+
+	if (is_promisor_object(ref_store->repo, oid))
+		return 0;
+
+	obj = parse_object(ref_store->repo, oid);
+	if (!obj) {
+		ret |= fsck_report_ref(o, &report,
+				       FSCK_MSG_BAD_REF_CONTENT,
+				       "points to non-existing object %s",
+				       oid_to_hex(oid));
+	} else if (obj->type != OBJ_COMMIT && is_branch(target_name)) {
+		ret |= fsck_report_ref(o, &report,
+				       FSCK_MSG_BAD_REF_CONTENT,
+				       "points to non-commit object %s",
+				       oid_to_hex(oid));
+	}
+
+	return ret;
+}
+
 static int files_fsck_refs_content(struct ref_store *ref_store,
 				   struct fsck_options *o,
 				   const char *target_name,
@@ -3654,18 +3683,19 @@ static int files_fsck_refs_content(struct ref_store *ref_store,
 	}
 
 	if (!(type & REF_ISSYMREF)) {
+		ret |= files_fsck_refs_oid(o, ref_store, report, target_name, &oid);
+
 		if (!*trailing) {
-			ret = fsck_report_ref(o, &report,
-					      FSCK_MSG_REF_MISSING_NEWLINE,
-					      "misses LF at the end");
-			goto cleanup;
-		}
-		if (*trailing != '\n' || *(trailing + 1)) {
-			ret = fsck_report_ref(o, &report,
-					      FSCK_MSG_TRAILING_REF_CONTENT,
-					      "has trailing garbage: '%s'", trailing);
-			goto cleanup;
+			ret |= fsck_report_ref(o, &report,
+					       FSCK_MSG_REF_MISSING_NEWLINE,
+					       "misses LF at the end");
+		} else if (*trailing != '\n' || *(trailing + 1)) {
+			ret |= fsck_report_ref(o, &report,
+					       FSCK_MSG_TRAILING_REF_CONTENT,
+					       "has trailing garbage: '%s'", trailing);
 		}
+
+		goto cleanup;
 	} else {
 		ret = files_fsck_symref_target(o, &report, &referent, 0);
 		goto cleanup;
diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh
index d4a08b823b..75f234a94a 100755
--- a/t/t0602-reffiles-fsck.sh
+++ b/t/t0602-reffiles-fsck.sh
@@ -161,8 +161,10 @@ test_expect_success 'regular ref content should be checked (individual)' '
 	test_when_finished "rm -rf repo" &&
 	git init repo &&
 	branch_dir_prefix=.git/refs/heads &&
+	tag_dir_prefix=.git/refs/tags &&
 	cd repo &&
 	test_commit default &&
+	git branch branch-1 &&
 	mkdir -p "$branch_dir_prefix/a/b" &&
 
 	git refs verify 2>err &&
@@ -198,6 +200,28 @@ test_expect_success 'regular ref content should be checked (individual)' '
 	rm $branch_dir_prefix/branch-no-newline &&
 	test_cmp expect err &&
 
+	for non_existing_oid in "$(test_oid 001)" "$(test_oid 002)"
+	do
+		printf "%s\n" $non_existing_oid >$branch_dir_prefix/invalid-commit &&
+		test_must_fail git refs verify 2>err &&
+		cat >expect <<-EOF &&
+		error: refs/heads/invalid-commit: badRefContent: points to non-existing object $non_existing_oid
+		EOF
+		rm $branch_dir_prefix/invalid-commit &&
+		test_cmp expect err || return 1
+	done &&
+
+	for tree_oid in "$(git rev-parse main^{tree})" "$(git rev-parse branch-1^{tree})"
+	do
+		printf "%s\n" $tree_oid >$branch_dir_prefix/branch-tree &&
+		test_must_fail git refs verify 2>err &&
+		cat >expect <<-EOF &&
+		error: refs/heads/branch-tree: badRefContent: points to non-commit object $tree_oid
+		EOF
+		rm $branch_dir_prefix/branch-tree &&
+		test_cmp expect err || return 1
+	done &&
+
 	for trailing_content in " garbage" "    more garbage"
 	do
 		printf "%s" "$(git rev-parse main)$trailing_content" >$branch_dir_prefix/branch-garbage &&
@@ -244,15 +268,21 @@ test_expect_success 'regular ref content should be checked (aggregate)' '
 	bad_content_1=$(git rev-parse main)x &&
 	bad_content_2=xfsazqfxcadas &&
 	bad_content_3=Xfsazqfxcadas &&
+	non_existing_oid=$(test_oid 001) &&
+	tree_oid=$(git rev-parse main^{tree}) &&
 	printf "%s" $bad_content_1 >$tag_dir_prefix/tag-bad-1 &&
 	printf "%s" $bad_content_2 >$tag_dir_prefix/tag-bad-2 &&
 	printf "%s" $bad_content_3 >$branch_dir_prefix/a/b/branch-bad &&
 	printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline &&
 	printf "%s garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage &&
+	printf "%s\n" $non_existing_oid >$branch_dir_prefix/branch-non-existing-oid &&
+	printf "%s\n" $tree_oid >$branch_dir_prefix/branch-tree &&
 
 	test_must_fail git refs verify 2>err &&
 	cat >expect <<-EOF &&
 	error: refs/heads/a/b/branch-bad: badRefContent: $bad_content_3
+	error: refs/heads/branch-non-existing-oid: badRefContent: points to non-existing object $non_existing_oid
+	error: refs/heads/branch-tree: badRefContent: points to non-commit object $tree_oid
 	error: refs/tags/tag-bad-1: badRefContent: $bad_content_1
 	error: refs/tags/tag-bad-2: badRefContent: $bad_content_2
 	warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\''
-- 
2.47.1


  reply	other threads:[~2025-01-05 13:49 UTC|newest]

Thread overview: 168+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-05 13:46 [PATCH 00/10] add more ref consistency checks shejialuo
2025-01-05 13:49 ` shejialuo [this message]
2025-01-07 14:17   ` [PATCH 01/10] files-backend: add object check for regular ref Karthik Nayak
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-17 13:40     ` shejialuo
2025-01-24  7:54       ` Patrick Steinhardt
2025-01-05 13:49 ` [PATCH 02/10] builtin/refs.h: get worktrees without reading head info shejialuo
2025-01-07 14:57   ` Karthik Nayak
2025-01-07 16:34     ` shejialuo
2025-01-08  8:40       ` Karthik Nayak
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-05 13:49 ` [PATCH 03/10] packed-backend: check whether the "packed-refs" is regular shejialuo
2025-01-07 16:33   ` Karthik Nayak
2025-01-17 14:00     ` shejialuo
2025-01-17 22:01       ` Eric Sunshine
2025-01-18  3:05         ` shejialuo
2025-01-19  8:03         ` Karthik Nayak
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-05 13:49 ` [PATCH 04/10] packed-backend: add "packed-refs" header consistency check shejialuo
2025-01-08  0:54   ` shejialuo
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-17 14:23     ` shejialuo
2025-01-24  7:51       ` Patrick Steinhardt
2025-02-17 13:16     ` shejialuo
2025-01-05 13:49 ` [PATCH 05/10] packed-backend: check whether the refname contains NULL binaries shejialuo
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-17 14:33     ` shejialuo
2025-01-05 13:49 ` [PATCH 06/10] packed-backend: add "packed-refs" entry consistency check shejialuo
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-17 14:35     ` shejialuo
2025-01-05 13:50 ` [PATCH 07/10] packed-backend: create "fsck_packed_ref_entry" to store parsing info shejialuo
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-05 13:50 ` [PATCH 08/10] packed-backend: add check for object consistency shejialuo
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-05 13:50 ` [PATCH 09/10] packed-backend: check whether the "packed-refs" is sorted shejialuo
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-05 13:50 ` [PATCH 10/10] builtin/fsck: add `git refs verify` child process shejialuo
2025-01-06 22:16   ` Junio C Hamano
2025-01-07 12:00     ` shejialuo
2025-01-07 15:52       ` Junio C Hamano
2025-01-30  4:04 ` [PATCH v2 0/8] add more ref consistency checks shejialuo
2025-01-30  4:06   ` [PATCH v2 1/8] t0602: use subshell to ensure working directory unchanged shejialuo
2025-01-30 17:53     ` Junio C Hamano
2025-01-30  4:07   ` [PATCH v2 2/8] builtin/refs: get worktrees without reading head info shejialuo
2025-01-30 18:04     ` Junio C Hamano
2025-01-31 13:29       ` shejialuo
2025-01-31 16:16         ` Junio C Hamano
2025-01-30  4:07   ` [PATCH v2 3/8] packed-backend: check whether the "packed-refs" is regular shejialuo
2025-01-30 18:23     ` Junio C Hamano
2025-01-31 13:54       ` shejialuo
2025-01-31 16:20         ` Junio C Hamano
2025-02-01  9:47           ` shejialuo
2025-02-03 20:15             ` Junio C Hamano
2025-02-04  3:58               ` shejialuo
2025-02-03  8:40     ` Patrick Steinhardt
2025-01-30  4:07   ` [PATCH v2 4/8] packed-backend: add "packed-refs" header consistency check shejialuo
2025-01-30 18:58     ` Junio C Hamano
2025-01-31 14:23       ` shejialuo
2025-01-30  4:07   ` [PATCH v2 5/8] packed-backend: check whether the refname contains NUL characters shejialuo
2025-02-03  8:40     ` Patrick Steinhardt
2025-02-05 10:09       ` shejialuo
2025-01-30  4:07   ` [PATCH v2 6/8] packed-backend: add "packed-refs" entry consistency check shejialuo
2025-02-03  8:40     ` Patrick Steinhardt
2025-02-04  4:28       ` shejialuo
2025-01-30  4:08   ` [PATCH v2 7/8] packed-backend: check whether the "packed-refs" is sorted shejialuo
2025-01-30 19:02     ` Junio C Hamano
2025-01-31 14:35       ` shejialuo
2025-01-31 16:23         ` Junio C Hamano
2025-02-01  9:50           ` shejialuo
2025-02-03  8:40         ` Patrick Steinhardt
2025-02-03  8:40     ` Patrick Steinhardt
2025-01-30  4:08   ` [PATCH v2 8/8] builtin/fsck: add `git refs verify` child process shejialuo
2025-01-30 19:03     ` Junio C Hamano
2025-01-31 14:37       ` shejialuo
2025-02-03  8:40     ` Patrick Steinhardt
2025-02-04  5:32       ` shejialuo
2025-02-06  5:56   ` [PATCH v3 0/8] add more ref consistency checks shejialuo
2025-02-06  5:58     ` [PATCH v3 1/8] t0602: use subshell to ensure working directory unchanged shejialuo
2025-02-06  5:58     ` [PATCH v3 2/8] builtin/refs: get worktrees without reading head information shejialuo
2025-02-06  5:58     ` [PATCH v3 3/8] packed-backend: check whether the "packed-refs" is regular file shejialuo
2025-02-06  5:59     ` [PATCH v3 4/8] packed-backend: add "packed-refs" header consistency check shejialuo
2025-02-12  9:56       ` Patrick Steinhardt
2025-02-12 10:12         ` shejialuo
2025-02-12 17:48         ` Junio C Hamano
2025-02-14  3:53           ` shejialuo
2025-02-06  5:59     ` [PATCH v3 5/8] packed-backend: check whether the refname contains NUL characters shejialuo
2025-02-06  5:59     ` [PATCH v3 6/8] packed-backend: add "packed-refs" entry consistency check shejialuo
2025-02-12  9:56       ` Patrick Steinhardt
2025-02-12 10:18         ` shejialuo
2025-02-06  5:59     ` [PATCH v3 7/8] packed-backend: check whether the "packed-refs" is sorted shejialuo
2025-02-12  9:56       ` Patrick Steinhardt
2025-02-12 10:20         ` shejialuo
2025-02-12 10:42           ` Patrick Steinhardt
2025-02-12 10:56         ` shejialuo
2025-02-06  6:00     ` [PATCH v3 8/8] builtin/fsck: add `git refs verify` child process shejialuo
2025-02-12  9:56       ` Patrick Steinhardt
2025-02-12 10:21         ` shejialuo
2025-02-14  4:50     ` [PATCH v4 0/8] add more ref consistency checks shejialuo
2025-02-14  4:51       ` [PATCH v4 1/8] t0602: use subshell to ensure working directory unchanged shejialuo
2025-02-14  4:52       ` [PATCH v4 2/8] builtin/refs: get worktrees without reading head information shejialuo
2025-02-14  9:19         ` Karthik Nayak
2025-02-14 12:20           ` shejialuo
2025-02-14  4:52       ` [PATCH v4 3/8] packed-backend: check whether the "packed-refs" is regular file shejialuo
2025-02-14  9:50         ` Karthik Nayak
2025-02-14 12:37           ` shejialuo
2025-02-14  4:52       ` [PATCH v4 4/8] packed-backend: add "packed-refs" header consistency check shejialuo
2025-02-14 10:30         ` Karthik Nayak
2025-02-14 12:43           ` shejialuo
2025-02-14 14:01         ` Junio C Hamano
2025-02-14  4:52       ` [PATCH v4 5/8] packed-backend: check whether the refname contains NUL characters shejialuo
2025-02-14  4:53       ` [PATCH v4 6/8] packed-backend: add "packed-refs" entry consistency check shejialuo
2025-02-14  4:59       ` [PATCH v4 7/8] packed-backend: check whether the "packed-refs" is sorted shejialuo
2025-02-14  4:59       ` [PATCH v4 8/8] builtin/fsck: add `git refs verify` child process shejialuo
2025-02-14  9:04       ` [PATCH v4 0/8] add more ref consistency checks Karthik Nayak
2025-02-14 12:16         ` shejialuo
2025-02-17 15:25       ` [PATCH v5 " shejialuo
2025-02-17 15:27         ` [PATCH v5 1/8] t0602: use subshell to ensure working directory unchanged shejialuo
2025-02-17 15:27         ` [PATCH v5 2/8] builtin/refs: get worktrees without reading head information shejialuo
2025-02-25  8:26           ` Patrick Steinhardt
2025-02-17 15:27         ` [PATCH v5 3/8] packed-backend: check whether the "packed-refs" is regular file shejialuo
2025-02-25  8:27           ` Patrick Steinhardt
2025-02-17 15:27         ` [PATCH v5 4/8] packed-backend: add "packed-refs" header consistency check shejialuo
2025-02-25  8:27           ` Patrick Steinhardt
2025-02-25 12:34             ` shejialuo
2025-02-17 15:27         ` [PATCH v5 5/8] packed-backend: check whether the refname contains NUL characters shejialuo
2025-02-17 15:28         ` [PATCH v5 6/8] packed-backend: add "packed-refs" entry consistency check shejialuo
2025-02-17 15:28         ` [PATCH v5 7/8] packed-backend: check whether the "packed-refs" is sorted shejialuo
2025-02-17 15:28         ` [PATCH v5 8/8] builtin/fsck: add `git refs verify` child process shejialuo
2025-02-25  8:27         ` [PATCH v5 0/8] add more ref consistency checks Patrick Steinhardt
2025-02-25 13:19         ` [PATCH v6 0/9] " shejialuo
2025-02-25 13:21           ` [PATCH v6 1/9] t0602: use subshell to ensure working directory unchanged shejialuo
2025-02-25 13:21           ` [PATCH v6 2/9] builtin/refs: get worktrees without reading head information shejialuo
2025-02-25 13:21           ` [PATCH v6 3/9] packed-backend: check whether the "packed-refs" is regular file shejialuo
2025-02-25 17:44             ` Junio C Hamano
2025-02-26 12:05               ` shejialuo
2025-02-25 13:21           ` [PATCH v6 4/9] packed-backend: check if header starts with "# pack-refs with: " shejialuo
2025-02-26  8:08             ` Patrick Steinhardt
2025-02-26 12:28               ` shejialuo
2025-02-25 13:21           ` [PATCH v6 5/9] packed-backend: add "packed-refs" header consistency check shejialuo
2025-02-25 13:21           ` [PATCH v6 6/9] packed-backend: check whether the refname contains NUL characters shejialuo
2025-02-25 13:22           ` [PATCH v6 7/9] packed-backend: add "packed-refs" entry consistency check shejialuo
2025-02-25 13:22           ` [PATCH v6 8/9] packed-backend: check whether the "packed-refs" is sorted shejialuo
2025-02-25 13:22           ` [PATCH v6 9/9] builtin/fsck: add `git refs verify` child process shejialuo
2025-02-26 13:48           ` [PATCH v7 0/9] add more ref consistency checks shejialuo
2025-02-26 13:49             ` [PATCH v7 1/9] t0602: use subshell to ensure working directory unchanged shejialuo
2025-02-26 13:49             ` [PATCH v7 2/9] builtin/refs: get worktrees without reading head information shejialuo
2025-02-26 13:49             ` [PATCH v7 3/9] packed-backend: check whether the "packed-refs" is regular file shejialuo
2025-02-26 18:36               ` Junio C Hamano
2025-02-27  0:57                 ` shejialuo
2025-02-27 14:10                   ` Patrick Steinhardt
2025-02-27 16:57                   ` Junio C Hamano
2025-02-28  5:02                     ` shejialuo
2025-02-26 13:50             ` [PATCH v7 4/9] packed-backend: check if header starts with "# pack-refs with: " shejialuo
2025-02-26 13:50             ` [PATCH v7 5/9] packed-backend: add "packed-refs" header consistency check shejialuo
2025-02-26 13:50             ` [PATCH v7 6/9] packed-backend: check whether the refname contains NUL characters shejialuo
2025-02-26 13:50             ` [PATCH v7 7/9] packed-backend: add "packed-refs" entry consistency check shejialuo
2025-02-26 13:50             ` [PATCH v7 8/9] packed-backend: check whether the "packed-refs" is sorted shejialuo
2025-02-26 13:51             ` [PATCH v7 9/9] builtin/fsck: add `git refs verify` child process shejialuo
2025-02-27 16:03             ` [PATCH v8 0/9] add more ref consistency checks shejialuo
2025-02-27 16:05               ` [PATCH v8 1/9] t0602: use subshell to ensure working directory unchanged shejialuo
2025-02-27 16:06               ` [PATCH v8 2/9] builtin/refs: get worktrees without reading head information shejialuo
2025-02-27 16:06               ` [PATCH v8 3/9] packed-backend: check whether the "packed-refs" is regular file shejialuo
2025-02-27 16:06               ` [PATCH v8 4/9] packed-backend: check if header starts with "# pack-refs with: " shejialuo
2025-02-27 16:06               ` [PATCH v8 5/9] packed-backend: add "packed-refs" header consistency check shejialuo
2025-02-27 16:07               ` [PATCH v8 6/9] packed-backend: check whether the refname contains NUL characters shejialuo
2025-02-27 16:07               ` [PATCH v8 7/9] packed-backend: add "packed-refs" entry consistency check shejialuo
2025-02-27 16:07               ` [PATCH v8 8/9] packed-backend: check whether the "packed-refs" is sorted shejialuo
2025-02-27 16:07               ` [PATCH v8 9/9] builtin/fsck: add `git refs verify` child process shejialuo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z3qN1T3lJoj82ckl@ArchLinux \
    --to=shejialuo@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=karthik.188@gmail.com \
    --cc=mhagger@alum.mit.edu \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.