From: "blanet via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Patrick Steinhardt <ps@pks.im>,
Karthik Nayak <karthik.188@gmail.com>,
blanet <bupt_xingxin@163.com>
Subject: [PATCH v8 0/3] object checking related additions and fixes for bundles in fetches
Date: Wed, 19 Jun 2024 04:07:30 +0000 [thread overview]
Message-ID: <pull.1730.v8.git.1718770053.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1730.v7.git.1718632535.gitgitgadget@gmail.com>
While attempting to fix a reference negotiation bug in bundle-uri, we
identified that the fetch process lacks some crucial object validation
checks when processing bundles. The primary issues are:
1. In the bundle-uri scenario, object IDs were not validated before writing
bundle references. This was the root cause of the original negotiation
bug in bundle-uri and could lead to potential repository corruption.
2. The existing "fetch.fsckObjects" and "transfer.fsckObjects"
configurations were not applied when directly fetching bundles or
fetching with bundle-uri enabled. In fact, there were no object
validation supports for unbundle.
The first patch addresses the bundle-uri negotiation issue by removing the
REF_SKIP_OID_VERIFICATION flag when writing bundle references.
Patches 2 through 3 extend verify_bundle_flags for bundle.c:unbundle to add
support for object validation (fsck) in fetch scenarios, mainly following
the suggestions from Junio and Patrick on the mailing list.
Xing Xin (3):
bundle-uri: verify oid before writing refs
fetch-pack: expose fsckObjects configuration logic
unbundle: extend object verification for fetches
bundle-uri.c | 6 +-
bundle.c | 3 +
bundle.h | 1 +
fetch-pack.c | 17 ++--
fetch-pack.h | 5 +
t/t5558-clone-bundle-uri.sh | 187 +++++++++++++++++++++++++++++++++++-
t/t5607-clone-bundle.sh | 35 +++++++
transport.c | 3 +-
8 files changed, 243 insertions(+), 14 deletions(-)
base-commit: b9cfe4845cb2562584837bc0101c0ab76490a239
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1730%2Fblanet%2Fxx%2Fbundle-uri-bug-using-bundle-list-v8
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1730/blanet/xx/bundle-uri-bug-using-bundle-list-v8
Pull-Request: https://github.com/gitgitgadget/git/pull/1730
Range-diff vs v7:
1: fc9f44fda00 ! 1: d8fbde2dcd4 bundle-uri: verify oid before writing refs
@@ t/t5558-clone-bundle-uri.sh: test_expect_success 'fail to clone from non-bundle
+ git bundle create B.bundle topic &&
+
+ # Create a bundle with reference pointing to non-existent object.
-+ sed -e "/^$/q" -e "s/$(git rev-parse A) /$(git rev-parse B) /" \
++ commit_a=$(git rev-parse A) &&
++ commit_b=$(git rev-parse B) &&
++ sed -e "/^$/q" -e "s/$commit_a /$commit_b /" \
+ <A.bundle >bad-header.bundle &&
+ convert_bundle_to_pack \
+ <A.bundle >>bad-header.bundle
@@ t/t5558-clone-bundle-uri.sh: test_expect_success 'clone with path bundle' '
+ commit_b=$(git -C clone-from rev-parse B) &&
+ test_grep "trying to write ref '\''refs/bundles/topic'\'' with nonexistent object $commit_b" err &&
+ git -C clone-bad-header for-each-ref --format="%(refname)" >refs &&
-+ ! grep "refs/bundles/" refs
++ test_grep ! "refs/bundles/" refs
+'
+
test_expect_success 'clone with path bundle and non-default hash' '
@@ t/t5558-clone-bundle-uri.sh: test_expect_success 'clone bundle list (file, any m
+ test_write_lines refs/bundles/topic >expect &&
+ test_cmp expect actual &&
+ # Ensure that refs/bundles/topic are sent as "have".
-+ test_grep "clone> have $(git -C clone-from rev-parse A)" trace-packet.txt
++ tip=$(git -C clone-from rev-parse A) &&
++ test_grep "clone> have $tip" trace-packet.txt
+'
+
+test_expect_success 'negotiation: bundle with all wanted commits' '
@@ t/t5558-clone-bundle-uri.sh: test_expect_success 'clone bundle list (file, any m
+ test_write_lines refs/bundles/topic >expect &&
+ test_cmp expect actual &&
+ # We already have all needed commits so no "want" needed.
-+ ! grep "clone> want " trace-packet.txt
++ test_grep ! "clone> want " trace-packet.txt
+'
+
+test_expect_success 'negotiation: bundle list (no heuristic)' '
@@ t/t5558-clone-bundle-uri.sh: test_expect_success 'clone bundle list (file, any m
+ refs/bundles/left
+ EOF
+ test_cmp expect actual &&
-+ test_grep "clone> have $(git -C nego-bundle-list-no-heuristic rev-parse refs/bundles/left)" trace-packet.txt
++ tip=$(git -C nego-bundle-list-no-heuristic rev-parse refs/bundles/left) &&
++ test_grep "clone> have $tip" trace-packet.txt
+'
+
+test_expect_success 'negotiation: bundle list (creationToken)' '
@@ t/t5558-clone-bundle-uri.sh: test_expect_success 'clone bundle list (file, any m
+ refs/bundles/left
+ EOF
+ test_cmp expect actual &&
-+ test_grep "clone> have $(git -C nego-bundle-list-heuristic rev-parse refs/bundles/left)" trace-packet.txt
++ tip=$(git -C nego-bundle-list-heuristic rev-parse refs/bundles/left) &&
++ test_grep "clone> have $tip" trace-packet.txt
+'
+
+test_expect_success 'negotiation: bundle list with all wanted commits' '
@@ t/t5558-clone-bundle-uri.sh: test_expect_success 'clone bundle list (file, any m
+ EOF
+ test_cmp expect actual &&
+ # We already have all needed commits so no "want" needed.
-+ ! grep "clone> want " trace-packet.txt
++ test_grep ! "clone> want " trace-packet.txt
+'
+
#########################################################################
2: 3dc0d9dd22f ! 2: 518584c8698 fetch-pack: expose fsckObjects configuration logic
@@ Commit message
"fetch.fsckObjects" to control checks for broken objects in received
packs during fetches. However, these configurations were only
acknowledged by `fetch-pack.c:get_pack` and did not take effect in
- direct bundle fetches and fetches with _bundle-uri_ enabled.
+ direct bundle fetches or fetches with _bundle-uri_ enabled.
This commit exposes the fetch-then-transfer configuration logic by
adding a new function `fetch_pack_fsck_objects` in fetch-pack.h. This
new function is used to replace the assignment for `fsck_objects` in
- `fetch-pack.c:get_pack`. In the next commit, it will also be used by
- `bundle.c:unbundle` to better fit fetching scenarios.
+ `fetch-pack.c:get_pack`. In the next commit, this function will also be
+ used to extend fsck support for bundle-involved fetches.
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Patrick Steinhardt <ps@pks.im>
3: 2f15099bbb9 ! 3: 698dd6e49b7 unbundle: extend object verification for fetches
@@ bundle.h: int create_bundle(struct repository *r, const char *path,
## t/t5558-clone-bundle-uri.sh ##
@@ t/t5558-clone-bundle-uri.sh: test_expect_success 'create bundle' '
- sed -e "/^$/q" -e "s/$(git rev-parse A) /$(git rev-parse B) /" \
+ sed -e "/^$/q" -e "s/$commit_a /$commit_b /" \
<A.bundle >bad-header.bundle &&
convert_bundle_to_pack \
- <A.bundle >>bad-header.bundle
+ <A.bundle >>bad-header.bundle &&
+
++ tree_b=$(git rev-parse B^{tree}) &&
+ cat >data <<-EOF &&
-+ tree $(git rev-parse HEAD^{tree})
-+ parent $(git rev-parse HEAD)
++ tree $tree_b
++ parent $commit_b
+ author A U Thor
+ committer A U Thor
+
+ commit: this is a commit with bad emails
+
+ EOF
-+ git hash-object --literally -t commit -w --stdin <data >commit &&
-+ git branch bad $(cat commit) &&
++ bad_commit=$(git hash-object --literally -t commit -w --stdin <data) &&
++ git branch bad $bad_commit &&
+ git bundle create bad-object.bundle bad &&
+ git update-ref -d refs/heads/bad
)
'
@@ t/t5558-clone-bundle-uri.sh: test_expect_success 'clone with bundle that has bad header' '
- ! grep "refs/bundles/" refs
+ test_grep ! "refs/bundles/" refs
'
+test_expect_success 'clone with bundle that has bad object' '
-+ # Unbundle succeeds if no fsckObjects confugured.
++ # Unbundle succeeds if no fsckObjects configured.
+ git clone --bundle-uri="clone-from/bad-object.bundle" \
+ clone-from clone-bad-object-no-fsck &&
+ git -C clone-bad-object-no-fsck for-each-ref --format="%(refname)" >refs &&
@@ t/t5558-clone-bundle-uri.sh: test_expect_success 'clone with bundle that has bad
+ clone-from clone-bad-object-fsck 2>err &&
+ test_grep "missingEmail" err &&
+ git -C clone-bad-object-fsck for-each-ref --format="%(refname)" >refs &&
-+ ! grep "refs/bundles/" refs
++ test_grep ! "refs/bundles/" refs
+'
+
test_expect_success 'clone with path bundle and non-default hash' '
@@ t/t5607-clone-bundle.sh: test_expect_success 'fetch SHA-1 from bundle' '
+ test_create_repo bundle-fsck &&
+ (
+ cd bundle-fsck &&
-+ test_commit first &&
++ test_commit A &&
++ commit_a=$(git rev-parse A) &&
++ tree_a=$(git rev-parse A^{tree}) &&
+ cat >data <<-EOF &&
-+ tree $(git rev-parse HEAD^{tree})
-+ parent $(git rev-parse HEAD)
++ tree $tree_a
++ parent $commit_a
+ author A U Thor
+ committer A U Thor
+
+ commit: this is a commit with bad emails
+
+ EOF
-+ git hash-object --literally -t commit -w --stdin <data >commit &&
-+ git branch bad $(cat commit) &&
++ bad_commit=$(git hash-object --literally -t commit -w --stdin <data) &&
++ git branch bad $bad_commit &&
+ git bundle create bad.bundle bad
+ ) &&
+
--
gitgitgadget
next prev parent reply other threads:[~2024-06-19 4:07 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-15 3:01 [PATCH] bundle-uri: refresh packed_git if unbundle succeed blanet via GitGitGadget
2024-05-17 5:00 ` Patrick Steinhardt
2024-05-17 16:14 ` Junio C Hamano
2024-05-20 11:48 ` Xing Xin
2024-05-20 17:19 ` Junio C Hamano
2024-05-27 16:04 ` Xing Xin
2024-05-20 9:41 ` Xing Xin
2024-05-17 7:36 ` Karthik Nayak
2024-05-20 10:19 ` Xing Xin
2024-05-20 12:36 ` [PATCH v2] bundle-uri: verify oid before writing refs blanet via GitGitGadget
2024-05-21 15:41 ` Karthik Nayak
2024-05-27 15:41 ` [PATCH v3 0/4] object checking related additions and fixes for bundles in fetches blanet via GitGitGadget
2024-05-27 15:41 ` [PATCH v3 1/4] bundle-uri: verify oid before writing refs Xing Xin via GitGitGadget
2024-05-28 11:55 ` Patrick Steinhardt
2024-05-30 8:32 ` Xing Xin
2024-05-27 15:41 ` [PATCH v3 2/4] unbundle: introduce unbundle_fsck_flags for fsckobjects handling Xing Xin via GitGitGadget
2024-05-28 12:03 ` Patrick Steinhardt
2024-05-29 18:12 ` Xing Xin
2024-05-30 4:38 ` Patrick Steinhardt
2024-05-30 8:46 ` Xing Xin
2024-05-27 15:41 ` [PATCH v3 3/4] fetch-pack: expose fsckObjects configuration logic Xing Xin via GitGitGadget
2024-05-28 12:03 ` Patrick Steinhardt
2024-05-28 17:10 ` Junio C Hamano
2024-05-28 17:24 ` Junio C Hamano
2024-05-29 5:52 ` Patrick Steinhardt
2024-05-30 8:48 ` Xing Xin
2024-05-29 5:52 ` Patrick Steinhardt
2024-05-27 15:41 ` [PATCH v3 4/4] unbundle: introduce new option UNBUNDLE_FSCK_FOLLOW_FETCH Xing Xin via GitGitGadget
2024-05-28 12:05 ` Patrick Steinhardt
2024-05-30 8:54 ` Xing Xin
2024-05-30 8:21 ` [PATCH v4 0/4] object checking related additions and fixes for bundles in fetches blanet via GitGitGadget
2024-05-30 8:21 ` [PATCH v4 1/4] bundle-uri: verify oid before writing refs Xing Xin via GitGitGadget
2024-05-30 8:21 ` [PATCH v4 2/4] unbundle: extend verify_bundle_flags to support fsck-objects Xing Xin via GitGitGadget
2024-06-06 12:06 ` Patrick Steinhardt
2024-06-11 6:46 ` Xing Xin
2024-05-30 8:21 ` [PATCH v4 3/4] fetch-pack: expose fsckObjects configuration logic Xing Xin via GitGitGadget
2024-05-30 8:21 ` [PATCH v4 4/4] unbundle: introduce option VERIFY_BUNDLE_FSCK_FOLLOW_FETCH Xing Xin via GitGitGadget
2024-06-06 12:06 ` Patrick Steinhardt
2024-06-11 6:46 ` Xing Xin
2024-06-11 6:42 ` [PATCH v5 0/4] object checking related additions and fixes for bundles in fetches blanet via GitGitGadget
2024-06-11 6:42 ` [PATCH v5 1/4] bundle-uri: verify oid before writing refs Xing Xin via GitGitGadget
2024-06-11 6:42 ` [PATCH v5 2/4] fetch-pack: expose fsckObjects configuration logic Xing Xin via GitGitGadget
2024-06-11 6:42 ` [PATCH v5 3/4] unbundle: extend options to support object verification Xing Xin via GitGitGadget
2024-06-11 9:11 ` Patrick Steinhardt
2024-06-11 12:47 ` Xing Xin
2024-06-11 6:42 ` [PATCH v5 4/4] unbundle: use VERIFY_BUNDLE_FSCK_FOLLOW_FETCH for fetches Xing Xin via GitGitGadget
2024-06-11 12:45 ` [PATCH v6 0/3] object checking related additions and fixes for bundles in fetches blanet via GitGitGadget
2024-06-11 12:45 ` [PATCH v6 1/3] bundle-uri: verify oid before writing refs Xing Xin via GitGitGadget
2024-06-11 19:08 ` Junio C Hamano
2024-06-17 13:53 ` Xing Xin
2024-06-11 12:45 ` [PATCH v6 2/3] fetch-pack: expose fsckObjects configuration logic Xing Xin via GitGitGadget
2024-06-11 19:20 ` Junio C Hamano
2024-06-11 12:45 ` [PATCH v6 3/3] unbundle: support object verification for fetches Xing Xin via GitGitGadget
2024-06-11 20:05 ` Junio C Hamano
2024-06-12 18:33 ` Xing Xin
2024-06-11 13:14 ` [PATCH v6 0/3] object checking related additions and fixes for bundles in fetches Patrick Steinhardt
2024-06-17 13:55 ` [PATCH v7 " blanet via GitGitGadget
2024-06-17 13:55 ` [PATCH v7 1/3] bundle-uri: verify oid before writing refs Xing Xin via GitGitGadget
2024-06-18 17:37 ` Junio C Hamano
2024-06-19 6:30 ` Xing Xin
2024-06-17 13:55 ` [PATCH v7 2/3] fetch-pack: expose fsckObjects configuration logic Xing Xin via GitGitGadget
2024-06-17 13:55 ` [PATCH v7 3/3] unbundle: extend object verification for fetches Xing Xin via GitGitGadget
2024-06-19 4:07 ` blanet via GitGitGadget [this message]
2024-06-19 4:07 ` [PATCH v8 1/3] bundle-uri: verify oid before writing refs Xing Xin via GitGitGadget
2024-06-19 4:07 ` [PATCH v8 2/3] fetch-pack: expose fsckObjects configuration logic Xing Xin via GitGitGadget
2024-06-19 4:07 ` [PATCH v8 3/3] unbundle: extend object verification for fetches Xing Xin via GitGitGadget
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=pull.1730.v8.git.1718770053.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=bupt_xingxin@163.com \
--cc=git@vger.kernel.org \
--cc=karthik.188@gmail.com \
--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 read-only IMAP folder(s) and NNTP newsgroup(s).