[PATCH 1/1] ref: add object check for regular ref

[shejialuo <shejialuo@gmail.com>]

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. Unlike "git-fsck(1)"
which only report "not a commit" error when the ref is a branch. We will
check whether tags point to either annotated tag object or commit
object. For other refs, we check that they could only point to commit
object.

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 | 55 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+), 10 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 5cfb8b7ca8..816aced923 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -21,6 +21,7 @@
 #include "../lockfile.h"
 #include "../object.h"
 #include "../object-file.h"
+#include "../packfile.h"
 #include "../path.h"
 #include "../dir.h"
 #include "../chdir-notify.h"
@@ -3703,18 +3704,47 @@ static int files_fsck_refs_content(struct ref_store *ref_store,
 	}
 
 	if (!(type & REF_ISSYMREF)) {
-		if (!*trailing) {
-			ret = fsck_report_ref(o, &report,
-					      FSCK_MSG_REF_MISSING_NEWLINE,
-					      "misses LF at the end");
-			goto cleanup;
+		struct object *obj = parse_object(the_repository, &oid);
+
+		if (!obj) {
+			if (!is_promisor_object(the_repository, &oid)) {
+				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) {
+			const char *err_fmt = NULL;
+
+			/*
+			 * Only tag can point to annotated tag object. All other refs
+			 * should point to commit object.
+			 */
+			if (starts_with(target_name, "refs/tags/")) {
+				if (obj->type != OBJ_TAG)
+					err_fmt = "points to neither commit nor tag object %s";
+			} else {
+				err_fmt = "points to non-commit object %s";
+			}
+
+			if (err_fmt) {
+				ret |= fsck_report_ref(o, &report,
+						       FSCK_MSG_BAD_REF_CONTENT,
+						       err_fmt, oid_to_hex(&oid));
+			}
 		}
-		if (*trailing != '\n' || *(trailing + 1)) {
-			ret = fsck_report_ref(o, &report,
-					      FSCK_MSG_TRAILING_REF_CONTENT,
-					      "has trailing garbage: '%s'", trailing);
-			goto cleanup;
+
+		if (!*trailing) {
+			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..ff4ccd250a 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 tag -a annotated-tag -m "annotated tag" &&
 	mkdir -p "$branch_dir_prefix/a/b" &&
 
 	git refs verify 2>err &&
@@ -198,6 +200,47 @@ 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})"
+	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 &&
+
+		printf "%s\n" $tree_oid >$tag_dir_prefix/tag-tree &&
+		test_must_fail git refs verify 2>err &&
+		cat >expect <<-EOF &&
+		error: refs/tags/tag-tree: badRefContent: points to neither commit nor tag object $tree_oid
+		EOF
+		rm $tag_dir_prefix/tag-tree &&
+		test_cmp expect err || return 1
+	done &&
+
+	for tag_object_hash in "$(git rev-parse annotated-tag)"
+	do
+		printf "%s\n" $tag_object_hash >$branch_dir_prefix/branch-annotated-tag &&
+		test_must_fail git refs verify 2>err &&
+		cat >expect <<-EOF &&
+		error: refs/heads/branch-annotated-tag: badRefContent: points to non-commit object $tag_object_hash
+		EOF
+		rm $branch_dir_prefix/branch-annotated-tag &&
+		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 &&
@@ -239,22 +282,34 @@ test_expect_success 'regular ref content should be checked (aggregate)' '
 	tag_dir_prefix=.git/refs/tags &&
 	cd repo &&
 	test_commit default &&
+	git tag -a annotated-tag -m "annotated tag" &&
 	mkdir -p "$branch_dir_prefix/a/b" &&
 
 	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}) &&
+	tag_oid=$(git rev-parse annotated-tag) &&
 	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 &&
+	printf "%s\n" $tag_oid >$branch_dir_prefix/branch-tag &&
+	printf "%s\n" $tree_oid >$tag_dir_prefix/tag-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-tag: badRefContent: points to non-commit object $tag_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
+	error: refs/tags/tag-tree: badRefContent: points to neither commit nor tag object $tree_oid
 	warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\''
 	warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end
 	EOF
-- 
2.47.1