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
next prev parent 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).