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@xxxxxx>
Mentored-by: Karthik Nayak <karthik.188@xxxxxxxxx>
Signed-off-by: shejialuo <shejialuo@xxxxxxxxx>
---
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
