* [PATCH] promisor-remote: prevent lazy-fetch recursion in child fetch
@ 2026-03-04 16:57 Paul Tarjan via GitGitGadget
2026-03-04 17:41 ` Junio C Hamano
2026-03-04 18:27 ` [PATCH v2] " Paul Tarjan via GitGitGadget
0 siblings, 2 replies; 11+ messages in thread
From: Paul Tarjan via GitGitGadget @ 2026-03-04 16:57 UTC (permalink / raw)
To: git; +Cc: Paul Tarjan, Paul Tarjan
From: Paul Tarjan <github@paulisageek.com>
fetch_objects() spawns a child `git fetch` to lazily fill in missing
objects. That child's index-pack, when it receives a thin pack
containing a REF_DELTA against a still-missing base, explicitly
calls promisor_remote_get_direct() — which is fetch_objects() again.
If the base is truly unavailable (e.g. because many refs in the
local store point at objects that have been garbage-collected on the
server), each recursive lazy-fetch can trigger another, leading to
unbounded recursion with runaway disk and process consumption.
The GIT_NO_LAZY_FETCH guard (introduced by e6d5479e7a (git: add
--no-lazy-fetch option, 2021-08-31)) already exists at the top of
fetch_objects(); the missing piece is propagating it into the child
fetch's environment. Add that propagation so the child's
index-pack, if it encounters a REF_DELTA against a missing base,
hits the guard and fails fast instead of recursing.
Depth-1 lazy fetch (the whole point of fetch_objects()) is
unaffected: only the child and its descendants see the variable.
With negotiationAlgorithm=noop the client advertises no "have"
lines, so a well-behaved server sends requested objects
un-deltified or deltified only against objects in the same pack;
the child's index-pack should never need a depth-2 fetch. If it
does, the server response was broken or the local store is already
corrupt, and further fetching would not help.
This is the same bug shape that 3a1ea94a49 (commit-graph.c: no lazy
fetch in lookup_commit_in_graph(), 2022-07-01) addressed at a
different entry point.
Add a test that verifies the child fetch environment contains
GIT_NO_LAZY_FETCH=1 via a reference-transaction hook, and that
only one fetch subprocess is spawned.
Cc: Jonathan Tan <jonathantanmy@google.com>
Cc: Han Xin <hanxin.hx@bytedance.com>
Cc: Jeff Hostetler <jeffhostetler@github.com>
Cc: Christian Couder <christian.couder@gmail.com>
Signed-off-by: Paul Tarjan <github@paulisageek.com>
---
promisor-remote: prevent recursive lazy-fetch during index-pack
fetch_objects() in promisor-remote.c spawns a child git fetch to lazily
fill missing objects. That child's index-pack --fix-thin, when it hits a
REF_DELTA against a still-missing base, calls
promisor_remote_get_direct() — which is fetch_objects() again. Unbounded
recursion.
We hit this in production: 276 GB of promisor packs written in 90
minutes against a 100 GB monorepo with ~61K stale prefetch refs pointing
at GC'd commits. Every thin pack picked a bad delta base, and the
recursion fanned out until the mount filled.
The fix is one line: propagate GIT_NO_LAZY_FETCH=1 into the child
fetch's environment. The guard already exists at the top of
fetch_objects() (added by e6d5479e7a, 2021); nothing was setting it in
the child. This is the same bug shape that Han Xin's 3a1ea94a49 (2022)
closed at lookup_commit_in_graph().
Depth-1 lazy fetch (the whole point of fetch_objects()) is unaffected —
only the child and its descendants see the variable. With
negotiationAlgorithm=noop the client advertises no "have" lines, so a
well-behaved server sends objects un-deltified or deltified only against
objects in the same pack. A depth-2 fetch would not help; if the server
sends broken thin packs, recursing just makes it worse.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2224%2Fptarjan%2Fclaude%2Ffix-lazy-fetch-recursion-KP9Hl-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2224/ptarjan/claude/fix-lazy-fetch-recursion-KP9Hl-v1
Pull-Request: https://github.com/git/git/pull/2224
promisor-remote.c | 7 +++
t/meson.build | 1 +
t/t0412-promisor-no-lazy-fetch-recursion.sh | 49 +++++++++++++++++++++
3 files changed, 57 insertions(+)
create mode 100755 t/t0412-promisor-no-lazy-fetch-recursion.sh
diff --git a/promisor-remote.c b/promisor-remote.c
index 96fa215b06..35c7aab93d 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -42,6 +42,13 @@ static int fetch_objects(struct repository *repo,
child.in = -1;
if (repo != the_repository)
prepare_other_repo_env(&child.env, repo->gitdir);
+ /*
+ * Prevent the child's index-pack from recursing back into
+ * fetch_objects() when resolving REF_DELTA bases it does not
+ * have. With noop negotiation the server should never need
+ * to send such deltas, so a depth-2 fetch would not help.
+ */
+ strvec_pushf(&child.env, "%s=1", NO_LAZY_FETCH_ENVIRONMENT);
strvec_pushl(&child.args, "-c", "fetch.negotiationAlgorithm=noop",
"fetch", remote_name, "--no-tags",
"--no-write-fetch-head", "--recurse-submodules=no",
diff --git a/t/meson.build b/t/meson.build
index e5174ee575..0499533dff 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -141,6 +141,7 @@ integration_tests = [
't0303-credential-external.sh',
't0410-partial-clone.sh',
't0411-clone-from-partial.sh',
+ 't0412-promisor-no-lazy-fetch-recursion.sh',
't0450-txt-doc-vs-help.sh',
't0500-progress-display.sh',
't0600-reffiles-backend.sh',
diff --git a/t/t0412-promisor-no-lazy-fetch-recursion.sh b/t/t0412-promisor-no-lazy-fetch-recursion.sh
new file mode 100755
index 0000000000..ec203543d4
--- /dev/null
+++ b/t/t0412-promisor-no-lazy-fetch-recursion.sh
@@ -0,0 +1,49 @@
+#!/bin/sh
+
+test_description='promisor-remote: no recursive lazy-fetch
+
+Verify that fetch_objects() sets GIT_NO_LAZY_FETCH=1 in the child
+fetch environment, so that index-pack cannot recurse back into
+fetch_objects() when resolving REF_DELTA bases.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ test_create_repo server &&
+ test_commit -C server foo &&
+ git -C server repack -a -d --write-bitmap-index &&
+
+ git clone "file://$(pwd)/server" client &&
+ HASH=$(git -C client rev-parse foo) &&
+ rm -rf client/.git/objects/* &&
+
+ git -C client config core.repositoryformatversion 1 &&
+ git -C client config extensions.partialclone "origin"
+'
+
+test_expect_success 'lazy-fetch spawns only one fetch subprocess' '
+ GIT_TRACE="$(pwd)/trace" git -C client cat-file -p "$HASH" &&
+
+ grep "git fetch" trace >fetches &&
+ test_line_count = 1 fetches
+'
+
+test_expect_success 'child of lazy-fetch has GIT_NO_LAZY_FETCH=1' '
+ rm -rf client/.git/objects/* &&
+
+ # Install a reference-transaction hook to record the env var
+ # as seen by processes inside the child fetch.
+ test_hook -C client reference-transaction <<-\EOF &&
+ echo "$GIT_NO_LAZY_FETCH" >>../env-in-child
+ EOF
+
+ rm -f env-in-child &&
+ git -C client cat-file -p "$HASH" &&
+
+ # The hook runs inside the child fetch, which should have
+ # GIT_NO_LAZY_FETCH=1 in its environment.
+ grep "^1$" env-in-child
+'
+
+test_done
base-commit: 7b2bccb0d58d4f24705bf985de1f4612e4cf06e5
--
gitgitgadget
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] promisor-remote: prevent lazy-fetch recursion in child fetch 2026-03-04 16:57 [PATCH] promisor-remote: prevent lazy-fetch recursion in child fetch Paul Tarjan via GitGitGadget @ 2026-03-04 17:41 ` Junio C Hamano 2026-03-04 18:20 ` Paul Tarjan 2026-03-04 18:27 ` [PATCH v2] " Paul Tarjan via GitGitGadget 1 sibling, 1 reply; 11+ messages in thread From: Junio C Hamano @ 2026-03-04 17:41 UTC (permalink / raw) To: Paul Tarjan via GitGitGadget Cc: git, Paul Tarjan, Han Xin, Jeff Hostetler, Christian Couder "Paul Tarjan via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Paul Tarjan <github@paulisageek.com> > > fetch_objects() spawns a child `git fetch` to lazily fill in missing > objects. That child's index-pack, when it receives a thin pack > containing a REF_DELTA against a still-missing base, explicitly > calls promisor_remote_get_direct() — which is fetch_objects() again. > If the base is truly unavailable (e.g. because many refs in the > local store point at objects that have been garbage-collected on the > server), each recursive lazy-fetch can trigger another, leading to > unbounded recursion with runaway disk and process consumption. > > The GIT_NO_LAZY_FETCH guard (introduced by e6d5479e7a (git: add > --no-lazy-fetch option, 2021-08-31)) already exists at the top of > fetch_objects(); the missing piece is propagating it into the child > fetch's environment. Add that propagation so the child's > index-pack, if it encounters a REF_DELTA against a missing base, > hits the guard and fails fast instead of recursing. > > Depth-1 lazy fetch (the whole point of fetch_objects()) is > unaffected: only the child and its descendants see the variable. > With negotiationAlgorithm=noop the client advertises no "have" > lines, so a well-behaved server sends requested objects > un-deltified or deltified only against objects in the same pack; > the child's index-pack should never need a depth-2 fetch. If it > does, the server response was broken or the local store is already > corrupt, and further fetching would not help. > > This is the same bug shape that 3a1ea94a49 (commit-graph.c: no lazy > fetch in lookup_commit_in_graph(), 2022-07-01) addressed at a > different entry point. > > Add a test that verifies the child fetch environment contains > GIT_NO_LAZY_FETCH=1 via a reference-transaction hook, and that > only one fetch subprocess is spawned. > > Cc: Jonathan Tan <jonathantanmy@google.com> > Cc: Han Xin <hanxin.hx@bytedance.com> > Cc: Jeff Hostetler <jeffhostetler@github.com> > Cc: Christian Couder <christian.couder@gmail.com> I would suggest dropping these CC: lines from the proposed log message. As far as I can see, they do not have their intended effect; [*1*] does not show any of these folks listed on Cc: *1* https://lore.kernel.org/git/pull.2224.git.git.1772643468305.gitgitgadget@gmail.com/ I am not a GitGitGadget user, but I think ... > Signed-off-by: Paul Tarjan <github@paulisageek.com> > --- > promisor-remote: prevent recursive lazy-fetch during index-pack > > fetch_objects() in promisor-remote.c spawns a child git fetch to lazily > fill missing objects. That child's index-pack --fix-thin, when it hits a > REF_DELTA against a still-missing base, calls > promisor_remote_get_direct() — which is fetch_objects() again. Unbounded > recursion. > > We hit this in production: 276 GB of promisor packs written in 90 > minutes against a 100 GB monorepo with ~61K stale prefetch refs pointing > at GC'd commits. Every thin pack picked a bad delta base, and the > recursion fanned out until the mount filled. > > The fix is one line: propagate GIT_NO_LAZY_FETCH=1 into the child > fetch's environment. The guard already exists at the top of > fetch_objects() (added by e6d5479e7a, 2021); nothing was setting it in > the child. This is the same bug shape that Han Xin's 3a1ea94a49 (2022) > closed at lookup_commit_in_graph(). > > Depth-1 lazy fetch (the whole point of fetch_objects()) is unaffected — > only the child and its descendants see the variable. With > negotiationAlgorithm=noop the client advertises no "have" lines, so a > well-behaved server sends objects un-deltified or deltified only against > objects in the same pack. A depth-2 fetch would not help; if the server > sends broken thin packs, recursing just makes it worse. ... once I heard that the tool expects list of folks to CC: on this side, i.e., not in the proposed commit log message, but in the pull request description. I also do not see much point in duplicating most of what appears in the proposed log message here after the three dash line, but that is a separate story. This is totally an unrelated tangent, but perhaps we'd need a best-practice document/guide for GitGitGadget users, that covers at least the following two things? * The pull-request message appear under three-dash in the e-mailed patch, where additional information that are not meant to become part of the log message goes. You do not want to duplicate your commit log message there. * Do not write Cc: trailers in your commit log message, as GitGitGadget does not pay attention to them. If you want to specify whom to Cc: your patches, write these in your pull-request message instead, which GitGitGadget does pay attention to. > diff --git a/promisor-remote.c b/promisor-remote.c > index 96fa215b06..35c7aab93d 100644 > --- a/promisor-remote.c > +++ b/promisor-remote.c > @@ -42,6 +42,13 @@ static int fetch_objects(struct repository *repo, > child.in = -1; > if (repo != the_repository) > prepare_other_repo_env(&child.env, repo->gitdir); > + /* > + * Prevent the child's index-pack from recursing back into > + * fetch_objects() when resolving REF_DELTA bases it does not > + * have. With noop negotiation the server should never need > + * to send such deltas, so a depth-2 fetch would not help. > + */ > + strvec_pushf(&child.env, "%s=1", NO_LAZY_FETCH_ENVIRONMENT); > strvec_pushl(&child.args, "-c", "fetch.negotiationAlgorithm=noop", > "fetch", remote_name, "--no-tags", > "--no-write-fetch-head", "--recurse-submodules=no", Looks good. > diff --git a/t/meson.build b/t/meson.build > index e5174ee575..0499533dff 100644 > --- a/t/meson.build > +++ b/t/meson.build > @@ -141,6 +141,7 @@ integration_tests = [ > 't0303-credential-external.sh', > 't0410-partial-clone.sh', > 't0411-clone-from-partial.sh', > + 't0412-promisor-no-lazy-fetch-recursion.sh', Hmph, do we really need an entirely new test script file dedicated for this single liner change, instead of adding to some existing test script that already covers related topics (like promisors and lazy fetches from them)? > 't0450-txt-doc-vs-help.sh', > 't0500-progress-display.sh', > 't0600-reffiles-backend.sh', > diff --git a/t/t0412-promisor-no-lazy-fetch-recursion.sh b/t/t0412-promisor-no-lazy-fetch-recursion.sh > new file mode 100755 > index 0000000000..ec203543d4 > --- /dev/null > +++ b/t/t0412-promisor-no-lazy-fetch-recursion.sh > @@ -0,0 +1,49 @@ > +#!/bin/sh > + > +test_description='promisor-remote: no recursive lazy-fetch > + > +Verify that fetch_objects() sets GIT_NO_LAZY_FETCH=1 in the child > +fetch environment, so that index-pack cannot recurse back into > +fetch_objects() when resolving REF_DELTA bases. > +' > + > +. ./test-lib.sh > + > +test_expect_success 'setup' ' > + test_create_repo server && > + test_commit -C server foo && > + git -C server repack -a -d --write-bitmap-index && > + > + git clone "file://$(pwd)/server" client && > + HASH=$(git -C client rev-parse foo) && > + rm -rf client/.git/objects/* && > + > + git -C client config core.repositoryformatversion 1 && > + git -C client config extensions.partialclone "origin" > +' > + > +test_expect_success 'lazy-fetch spawns only one fetch subprocess' ' > + GIT_TRACE="$(pwd)/trace" git -C client cat-file -p "$HASH" && > + > + grep "git fetch" trace >fetches && > + test_line_count = 1 fetches > +' > + > +test_expect_success 'child of lazy-fetch has GIT_NO_LAZY_FETCH=1' ' > + rm -rf client/.git/objects/* && > + > + # Install a reference-transaction hook to record the env var > + # as seen by processes inside the child fetch. > + test_hook -C client reference-transaction <<-\EOF && > + echo "$GIT_NO_LAZY_FETCH" >>../env-in-child > + EOF > + > + rm -f env-in-child && > + git -C client cat-file -p "$HASH" && > + > + # The hook runs inside the child fetch, which should have > + # GIT_NO_LAZY_FETCH=1 in its environment. > + grep "^1$" env-in-child > +' > + > +test_done > > base-commit: 7b2bccb0d58d4f24705bf985de1f4612e4cf06e5 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] promisor-remote: prevent lazy-fetch recursion in child fetch 2026-03-04 17:41 ` Junio C Hamano @ 2026-03-04 18:20 ` Paul Tarjan 0 siblings, 0 replies; 11+ messages in thread From: Paul Tarjan @ 2026-03-04 18:20 UTC (permalink / raw) To: git Cc: gitster, paul, jonathantanmy, hanxin.hx, jeffhostetler, christian.couder On Tue, Mar 4, 2026, Junio C Hamano wrote: > I would suggest dropping these CC: lines from the proposed log > message. As far as I can see, they do not have their intended > effect; [*1*] does not show any of these folks listed on Cc: Done, moved them to the PR description for GitGitGadget to pick up. > I also do not see much point in duplicating > most of what appears in the proposed log message here after the > three dash line, but that is a separate story. Cleaned up the PR description to avoid the duplication. > Hmph, do we really need an entirely new test script file dedicated > for this single liner change, instead of adding to some existing > test script that already covers related topics (like promisors and > lazy fetches from them)? Moved the test into t0411-clone-from-partial.sh, which already has the other lazy-fetch tests. Dropped the separate t0412 file and the meson.build entry. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] promisor-remote: prevent lazy-fetch recursion in child fetch 2026-03-04 16:57 [PATCH] promisor-remote: prevent lazy-fetch recursion in child fetch Paul Tarjan via GitGitGadget 2026-03-04 17:41 ` Junio C Hamano @ 2026-03-04 18:27 ` Paul Tarjan via GitGitGadget 2026-03-11 10:52 ` Patrick Steinhardt 2026-03-11 14:19 ` Paul Tarjan via GitGitGadget 1 sibling, 2 replies; 11+ messages in thread From: Paul Tarjan via GitGitGadget @ 2026-03-04 18:27 UTC (permalink / raw) To: git; +Cc: Christian Couder, Han Xin, Paul Tarjan, Paul Tarjan From: Paul Tarjan <github@paulisageek.com> fetch_objects() spawns a child `git fetch` to lazily fill in missing objects. That child's index-pack, when it receives a thin pack containing a REF_DELTA against a still-missing base, explicitly calls promisor_remote_get_direct() — which is fetch_objects() again. If the base is truly unavailable (e.g. because many refs in the local store point at objects that have been garbage-collected on the server), each recursive lazy-fetch can trigger another, leading to unbounded recursion with runaway disk and process consumption. The GIT_NO_LAZY_FETCH guard (introduced by e6d5479e7a (git: add --no-lazy-fetch option, 2021-08-31)) already exists at the top of fetch_objects(); the missing piece is propagating it into the child fetch's environment. Add that propagation so the child's index-pack, if it encounters a REF_DELTA against a missing base, hits the guard and fails fast instead of recursing. Depth-1 lazy fetch (the whole point of fetch_objects()) is unaffected: only the child and its descendants see the variable. With negotiationAlgorithm=noop the client advertises no "have" lines, so a well-behaved server sends requested objects un-deltified or deltified only against objects in the same pack; the child's index-pack should never need a depth-2 fetch. If it does, the server response was broken or the local store is already corrupt, and further fetching would not help. This is the same bug shape that 3a1ea94a49 (commit-graph.c: no lazy fetch in lookup_commit_in_graph(), 2022-07-01) addressed at a different entry point. Add a test that verifies the child fetch environment contains GIT_NO_LAZY_FETCH=1 via a reference-transaction hook. Signed-off-by: Paul Tarjan <github@paulisageek.com> --- promisor-remote: prevent recursive lazy-fetch during index-pack Propagate GIT_NO_LAZY_FETCH=1 into the child fetch spawned by fetch_objects() so that index-pack cannot recurse back into lazy-fetch when resolving REF_DELTA bases. We hit this in production: 276 GB of promisor packs written in 90 minutes against a 100 GB monorepo with ~61K stale prefetch refs pointing at GC'd commits. Changes since v1: * Dropped CC: trailers from commit message (moved here for GitGitGadget) * Moved test into t0411-clone-from-partial.sh instead of a new file * Removed duplicate commit-message summary from PR description Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2224%2Fptarjan%2Fclaude%2Ffix-lazy-fetch-recursion-KP9Hl-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2224/ptarjan/claude/fix-lazy-fetch-recursion-KP9Hl-v2 Pull-Request: https://github.com/git/git/pull/2224 Range-diff vs v1: 1: 7b723441f4 ! 1: 907ca7a0ac promisor-remote: prevent lazy-fetch recursion in child fetch @@ Commit message different entry point. Add a test that verifies the child fetch environment contains - GIT_NO_LAZY_FETCH=1 via a reference-transaction hook, and that - only one fetch subprocess is spawned. + GIT_NO_LAZY_FETCH=1 via a reference-transaction hook. - Cc: Jonathan Tan <jonathantanmy@google.com> - Cc: Han Xin <hanxin.hx@bytedance.com> - Cc: Jeff Hostetler <jeffhostetler@github.com> - Cc: Christian Couder <christian.couder@gmail.com> Signed-off-by: Paul Tarjan <github@paulisageek.com> ## promisor-remote.c ## @@ promisor-remote.c: static int fetch_objects(struct repository *repo, "fetch", remote_name, "--no-tags", "--no-write-fetch-head", "--recurse-submodules=no", - ## t/meson.build ## -@@ t/meson.build: integration_tests = [ - 't0303-credential-external.sh', - 't0410-partial-clone.sh', - 't0411-clone-from-partial.sh', -+ 't0412-promisor-no-lazy-fetch-recursion.sh', - 't0450-txt-doc-vs-help.sh', - 't0500-progress-display.sh', - 't0600-reffiles-backend.sh', - - ## t/t0412-promisor-no-lazy-fetch-recursion.sh (new) ## -@@ -+#!/bin/sh -+ -+test_description='promisor-remote: no recursive lazy-fetch -+ -+Verify that fetch_objects() sets GIT_NO_LAZY_FETCH=1 in the child -+fetch environment, so that index-pack cannot recurse back into -+fetch_objects() when resolving REF_DELTA bases. -+' -+ -+. ./test-lib.sh -+ -+test_expect_success 'setup' ' -+ test_create_repo server && -+ test_commit -C server foo && -+ git -C server repack -a -d --write-bitmap-index && + ## t/t0411-clone-from-partial.sh ## +@@ t/t0411-clone-from-partial.sh: test_expect_success 'promisor lazy-fetching can be re-enabled' ' + test_path_is_file script-executed + ' + ++test_expect_success 'lazy-fetch child has GIT_NO_LAZY_FETCH=1' ' ++ test_create_repo nolazy-server && ++ test_commit -C nolazy-server foo && ++ git -C nolazy-server repack -a -d --write-bitmap-index && + -+ git clone "file://$(pwd)/server" client && -+ HASH=$(git -C client rev-parse foo) && -+ rm -rf client/.git/objects/* && -+ -+ git -C client config core.repositoryformatversion 1 && -+ git -C client config extensions.partialclone "origin" -+' -+ -+test_expect_success 'lazy-fetch spawns only one fetch subprocess' ' -+ GIT_TRACE="$(pwd)/trace" git -C client cat-file -p "$HASH" && -+ -+ grep "git fetch" trace >fetches && -+ test_line_count = 1 fetches -+' ++ git clone "file://$(pwd)/nolazy-server" nolazy-client && ++ HASH=$(git -C nolazy-client rev-parse foo) && ++ rm -rf nolazy-client/.git/objects/* && + -+test_expect_success 'child of lazy-fetch has GIT_NO_LAZY_FETCH=1' ' -+ rm -rf client/.git/objects/* && ++ git -C nolazy-client config core.repositoryformatversion 1 && ++ git -C nolazy-client config extensions.partialclone "origin" && + + # Install a reference-transaction hook to record the env var + # as seen by processes inside the child fetch. -+ test_hook -C client reference-transaction <<-\EOF && ++ test_hook -C nolazy-client reference-transaction <<-\EOF && + echo "$GIT_NO_LAZY_FETCH" >>../env-in-child + EOF + + rm -f env-in-child && -+ git -C client cat-file -p "$HASH" && ++ git -C nolazy-client cat-file -p "$HASH" && + + # The hook runs inside the child fetch, which should have + # GIT_NO_LAZY_FETCH=1 in its environment. + grep "^1$" env-in-child +' + -+test_done + test_done promisor-remote.c | 7 +++++++ t/t0411-clone-from-partial.sh | 26 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/promisor-remote.c b/promisor-remote.c index 96fa215b06..35c7aab93d 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -42,6 +42,13 @@ static int fetch_objects(struct repository *repo, child.in = -1; if (repo != the_repository) prepare_other_repo_env(&child.env, repo->gitdir); + /* + * Prevent the child's index-pack from recursing back into + * fetch_objects() when resolving REF_DELTA bases it does not + * have. With noop negotiation the server should never need + * to send such deltas, so a depth-2 fetch would not help. + */ + strvec_pushf(&child.env, "%s=1", NO_LAZY_FETCH_ENVIRONMENT); strvec_pushl(&child.args, "-c", "fetch.negotiationAlgorithm=noop", "fetch", remote_name, "--no-tags", "--no-write-fetch-head", "--recurse-submodules=no", diff --git a/t/t0411-clone-from-partial.sh b/t/t0411-clone-from-partial.sh index 9e6bca5625..10a829fb80 100755 --- a/t/t0411-clone-from-partial.sh +++ b/t/t0411-clone-from-partial.sh @@ -78,4 +78,30 @@ test_expect_success 'promisor lazy-fetching can be re-enabled' ' test_path_is_file script-executed ' +test_expect_success 'lazy-fetch child has GIT_NO_LAZY_FETCH=1' ' + test_create_repo nolazy-server && + test_commit -C nolazy-server foo && + git -C nolazy-server repack -a -d --write-bitmap-index && + + git clone "file://$(pwd)/nolazy-server" nolazy-client && + HASH=$(git -C nolazy-client rev-parse foo) && + rm -rf nolazy-client/.git/objects/* && + + git -C nolazy-client config core.repositoryformatversion 1 && + git -C nolazy-client config extensions.partialclone "origin" && + + # Install a reference-transaction hook to record the env var + # as seen by processes inside the child fetch. + test_hook -C nolazy-client reference-transaction <<-\EOF && + echo "$GIT_NO_LAZY_FETCH" >>../env-in-child + EOF + + rm -f env-in-child && + git -C nolazy-client cat-file -p "$HASH" && + + # The hook runs inside the child fetch, which should have + # GIT_NO_LAZY_FETCH=1 in its environment. + grep "^1$" env-in-child +' + test_done base-commit: 7b2bccb0d58d4f24705bf985de1f4612e4cf06e5 -- gitgitgadget ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] promisor-remote: prevent lazy-fetch recursion in child fetch 2026-03-04 18:27 ` [PATCH v2] " Paul Tarjan via GitGitGadget @ 2026-03-11 10:52 ` Patrick Steinhardt 2026-03-11 14:18 ` Paul Tarjan 2026-03-11 14:19 ` Paul Tarjan via GitGitGadget 1 sibling, 1 reply; 11+ messages in thread From: Patrick Steinhardt @ 2026-03-11 10:52 UTC (permalink / raw) To: Paul Tarjan via GitGitGadget; +Cc: git, Christian Couder, Han Xin, Paul Tarjan On Wed, Mar 04, 2026 at 06:27:25PM +0000, Paul Tarjan via GitGitGadget wrote: > From: Paul Tarjan <github@paulisageek.com> > > fetch_objects() spawns a child `git fetch` to lazily fill in missing > objects. That child's index-pack, when it receives a thin pack > containing a REF_DELTA against a still-missing base, explicitly > calls promisor_remote_get_direct() — which is fetch_objects() again. > If the base is truly unavailable (e.g. because many refs in the > local store point at objects that have been garbage-collected on the > server), each recursive lazy-fetch can trigger another, leading to > unbounded recursion with runaway disk and process consumption. Is this a theoretical concern or a practical one? I would expect that backfill fetches never cause the server side to send a pack with REF_DELTA objects to nonexistent objects. And if they did they are broken. > The GIT_NO_LAZY_FETCH guard (introduced by e6d5479e7a (git: add > --no-lazy-fetch option, 2021-08-31)) already exists at the top of > fetch_objects(); the missing piece is propagating it into the child > fetch's environment. Add that propagation so the child's > index-pack, if it encounters a REF_DELTA against a missing base, > hits the guard and fails fast instead of recursing. > > Depth-1 lazy fetch (the whole point of fetch_objects()) is > unaffected: only the child and its descendants see the variable. > With negotiationAlgorithm=noop the client advertises no "have" > lines, so a well-behaved server sends requested objects > un-deltified or deltified only against objects in the same pack; > the child's index-pack should never need a depth-2 fetch. If it > does, the server response was broken or the local store is already > corrupt, and further fetching would not help. Exactly, this here matches my understanding. The backfill fetches don't perform negotiation, so we shouldn't ever see a thin pack in the first place. What I don't yet understand is your comment about the depth-2 fetch -- when would we ever do that? > This is the same bug shape that 3a1ea94a49 (commit-graph.c: no lazy > fetch in lookup_commit_in_graph(), 2022-07-01) addressed at a > different entry point. I dunno, I think it's quite different overall. In the mentioned commit we protect against a stale commit-graph, which is something that is quite plausible to happen on the client side. But here we protect us against a remote side that sends a packfile that violates specs, as far as I understand. > Add a test that verifies the child fetch environment contains > GIT_NO_LAZY_FETCH=1 via a reference-transaction hook. Hm. Can we craft a test that shows us the resulting failure in practice? Testing for the environment variable feels like a bad proxy to me, as I'd rather want to learn how Git would fail now. > Signed-off-by: Paul Tarjan <github@paulisageek.com> > --- > promisor-remote: prevent recursive lazy-fetch during index-pack > > Propagate GIT_NO_LAZY_FETCH=1 into the child fetch spawned by > fetch_objects() so that index-pack cannot recurse back into lazy-fetch > when resolving REF_DELTA bases. > > We hit this in production: 276 GB of promisor packs written in 90 > minutes against a 100 GB monorepo with ~61K stale prefetch refs pointing > at GC'd commits. Okay, so this seems to be an issue that can be hit in the wild. But I have to wonder whether this really is a bug on the client-side, or whether this is a bug that actually sits on your server. So ultimately: why does the server send REF_DELTA objects in the first place? Is it using git-upload-pack(1), or is it using a different implementation of Git to serve data? Note that I'm not arguing that we shouldn't have protection on the client, too. But I'd first like to understand whether there is a bug lurking somewhere that causes us to send invalid packfiles. Patrick ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] promisor-remote: prevent lazy-fetch recursion in child fetch 2026-03-11 10:52 ` Patrick Steinhardt @ 2026-03-11 14:18 ` Paul Tarjan 2026-03-12 7:27 ` Patrick Steinhardt 0 siblings, 1 reply; 11+ messages in thread From: Paul Tarjan @ 2026-03-11 14:18 UTC (permalink / raw) To: git; +Cc: ps, gitgitgadget, christian.couder, hanxin.hx Patrick Steinhardt <ps@pks.im> writes: > Is this a theoretical concern or a practical one? I would expect that > backfill fetches never cause the server side to send a pack with > REF_DELTA objects to nonexistent objects. And if they did they are > broken. Practical. We hit this at Anthropic: 276 GB of promisor packs written by `git maintenance --task=prefetch` in 90 minutes against a ~10 GB monorepo with ~61K stale prefetch refs pointing at GC'd commits. > Exactly, this here matches my understanding. The backfill fetches don't > perform negotiation, so we shouldn't ever see a thin pack in the first > place. What I don't yet understand is your comment about the depth-2 > fetch -- when would we ever do that? The code path already exists and is tested: t5616 line 832 ("tolerate server sending REF_DELTA against missing promisor objects") creates exactly this scenario. index-pack's fix_unresolved_deltas() calls promisor_remote_get_direct() when it encounters a REF_DELTA against a missing base (builtin/index-pack.c:1508). That's the depth-2 fetch. With noop negotiation a well-behaved server shouldn't send REF_DELTA against objects the client doesn't have. But partial clones with blob:none mean the client is missing most blobs, and if the server sends a thin pack deltified against one of those filtered-out blobs, index-pack will try to fetch the base. > I dunno, I think it's quite different overall. In the mentioned commit > we protect against a stale commit-graph, which is something that is > quite plausible to happen on the client side. But here we protect us > against a remote side that sends a packfile that violates specs, as far > as I understand. Fair point. The commit-graph case is purely client-side corruption, while this requires a misbehaving server. The bug shape is the same (unbounded recursion through fetch_objects()) but the trigger is different. I'll drop the comparison in the next version. > Hm. Can we craft a test that shows us the resulting failure in practice? > Testing for the environment variable feels like a bad proxy to me, as > I'd rather want to learn how Git would fail now. Good point. Reworked the test in v3. It now injects a thin pack containing a REF_DELTA against a missing base via HTTP (using the replace_packfile pattern from t5616). This triggers the actual recursion path: index-pack encounters the missing base, calls promisor_remote_get_direct(), which hits the GIT_NO_LAZY_FETCH=1 guard and fails with "lazy fetching disabled". Without the fix, the depth-2 fetch would proceed and potentially recurse. > Okay, so this seems to be an issue that can be hit in the wild. But I > have to wonder whether this really is a bug on the client-side, or > whether this is a bug that actually sits on your server. So ultimately: > why does the server send REF_DELTA objects in the first place? Is it > using git-upload-pack(1), or is it using a different implementation of > Git to serve data? The server is GitHub. I did a blob:none partial clone and after some further git operations ended up in this state. I don't have server-side data on why it sent REF_DELTAs against missing bases. > Note that I'm not arguing that we shouldn't have protection on the > client, too. But I'd first like to understand whether there is a bug > lurking somewhere that causes us to send invalid packfiles. Agreed, there may well be a server-side bug here. Regardless, the client should fail fast rather than consume unbounded resources. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] promisor-remote: prevent lazy-fetch recursion in child fetch 2026-03-11 14:18 ` Paul Tarjan @ 2026-03-12 7:27 ` Patrick Steinhardt 2026-03-13 1:43 ` Jeff King 2026-03-13 12:43 ` Paul Tarjan 0 siblings, 2 replies; 11+ messages in thread From: Patrick Steinhardt @ 2026-03-12 7:27 UTC (permalink / raw) To: Paul Tarjan; +Cc: git, gitgitgadget, christian.couder, hanxin.hx, Jeff King On Wed, Mar 11, 2026 at 08:18:46AM -0600, Paul Tarjan wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > Is this a theoretical concern or a practical one? I would expect that > > backfill fetches never cause the server side to send a pack with > > REF_DELTA objects to nonexistent objects. And if they did they are > > broken. > > Practical. We hit this at Anthropic: 276 GB of promisor packs written > by `git maintenance --task=prefetch` in 90 minutes against a ~10 GB > monorepo with ~61K stale prefetch refs pointing at GC'd commits. I must be misunderstanding something here, but how is it that a commit can be garbage collected if a ref points to it? That shouldn't ever happen, as reachable commits should not be pruned. Or do you mean to say that the commits don't exist on the server side anymore? > > Hm. Can we craft a test that shows us the resulting failure in practice? > > Testing for the environment variable feels like a bad proxy to me, as > > I'd rather want to learn how Git would fail now. > > Good point. Reworked the test in v3. It now injects a thin pack > containing a REF_DELTA against a missing base via HTTP (using the > replace_packfile pattern from t5616). This triggers the actual > recursion path: index-pack encounters the missing base, calls > promisor_remote_get_direct(), which hits the GIT_NO_LAZY_FETCH=1 > guard and fails with "lazy fetching disabled". Without the fix, > the depth-2 fetch would proceed and potentially recurse. Great, thanks. > > Okay, so this seems to be an issue that can be hit in the wild. But I > > have to wonder whether this really is a bug on the client-side, or > > whether this is a bug that actually sits on your server. So ultimately: > > why does the server send REF_DELTA objects in the first place? Is it > > using git-upload-pack(1), or is it using a different implementation of > > Git to serve data? > > The server is GitHub. I did a blob:none partial clone and after some > further git operations ended up in this state. I don't have > server-side data on why it sent REF_DELTAs against missing bases. That's certainly curious. Do you maybe have multiple remotes attached to the repository, or are you dropping/modifying the object filter at some point? All subsequent fetches need to use the same object filter as you've used during the initial clone, otherwise you may run into a situation as you have described. But in theory, Git knows to continue using the filter. > > Note that I'm not arguing that we shouldn't have protection on the > > client, too. But I'd first like to understand whether there is a bug > > lurking somewhere that causes us to send invalid packfiles. > > Agreed, there may well be a server-side bug here. Regardless, the > client should fail fast rather than consume unbounded resources. Probably, yes. What I'm trying to figure out is whether there are edge cases here where it's _valid_ for the server to send a thin pack with a REF_DELTA. Because if so, unconditionally disabling the lazy fetches would break such edge cases. I don't think there are such cases, but I wouldn't consider myself an expert with partial clones. Cc'd Peff, as he's implemented a couple fixes in this area a couple years ago. Patrick ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] promisor-remote: prevent lazy-fetch recursion in child fetch 2026-03-12 7:27 ` Patrick Steinhardt @ 2026-03-13 1:43 ` Jeff King 2026-03-13 12:43 ` [PATCH v3] " Paul Tarjan 2026-03-13 12:43 ` Paul Tarjan 1 sibling, 1 reply; 11+ messages in thread From: Jeff King @ 2026-03-13 1:43 UTC (permalink / raw) To: Patrick Steinhardt Cc: Paul Tarjan, git, gitgitgadget, christian.couder, hanxin.hx On Thu, Mar 12, 2026 at 08:27:05AM +0100, Patrick Steinhardt wrote: > > > Note that I'm not arguing that we shouldn't have protection on the > > > client, too. But I'd first like to understand whether there is a bug > > > lurking somewhere that causes us to send invalid packfiles. > > > > Agreed, there may well be a server-side bug here. Regardless, the > > client should fail fast rather than consume unbounded resources. > > Probably, yes. What I'm trying to figure out is whether there are edge > cases here where it's _valid_ for the server to send a thin pack with a > REF_DELTA. Because if so, unconditionally disabling the lazy fetches > would break such edge cases. > > I don't think there are such cases, but I wouldn't consider myself an > expert with partial clones. > > Cc'd Peff, as he's implemented a couple fixes in this area a couple > years ago. Hmm, I'm not sure I have much wisdom. Here's the most plausible scenario I could come up with. A backfill fetch like this is going to have a noop negotiation algorithm. So the server does not have any idea what the client has, and therefore shouldn't be sending any thin deltas against it. But it _can_ send deltas against objects which are part of the backfill itself. Normally we'd send those as OFS_DELTA, because they're both in the same output pack. But there are cases where we might not: - if the client did not tell us it understands ofs-deltas; this would not be true for any version of Git in the last 15+ years, but maybe there is a bug in sending or parsing the capability? Or an alternate Git implementation on the client side which forgets to send it? - the verbatim pack-reuse code will sometimes rewrite ofs-delta into ref-delta. I don't remember all of the cases where this might happen. Certainly if the client hasn't claimed to support ofs-deltas, but I think maybe some other cases? I'd have to dig into it. Now there's a catch: the pack is not really thin, and so index-pack should not need to do an extra backfill request in order to get the base object. But depending how index-pack is written, it might try to do so anyway. If X is a delta against base Y, but Y is itself a delta, we might not have resolved it yet. And so when we try to resolve X, we think "aha, let us see if we have Y", and then eagerly attempt a backfill fetch (probably triggered from odb_has_object() or similar). When in fact the right thing to do is to queue X, resolve everything we can, and see if we ended up with Y (actually index-pack works from the bases up in the final resolution phase, but the effect is the same). If that's what is happening, then I _think_ Paul's patch will do the right thing. We'd say "no, we don't have that object" without doing the backfill, and then eventually find it as part of the final resolution. It would be nice to confirm that's what's going on, though (and it isn't really a thin pack). If the problem can be reproduced, I don't suppose we have a GIT_TRACE_PACKET output from a failing instance? That would confirm that we're correctly using the noop negotiation. -Peff ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] promisor-remote: prevent lazy-fetch recursion in child fetch 2026-03-13 1:43 ` Jeff King @ 2026-03-13 12:43 ` Paul Tarjan 0 siblings, 0 replies; 11+ messages in thread From: Paul Tarjan @ 2026-03-13 12:43 UTC (permalink / raw) To: git; +Cc: peff, ps, gitgitgadget, christian.couder, hanxin.hx Jeff King <peff@peff.net> writes: > It would be nice to confirm that's what's going on, though (and it isn't > really a thin pack). If the problem can be reproduced, I don't suppose > we have a GIT_TRACE_PACKET output from a failing instance? That would > confirm that we're correctly using the noop negotiation. Yeah. I couldn't capture one live during the incident, but I did a clean-room reproduction afterward: empty bare repo, git remote add origin pointing at the same GitHub monorepo, then the exact argv from fetch_objects() with a tree SHA pulled from one of the .promisor sidecars written during the incident: $ git -c fetch.negotiationAlgorithm=noop fetch origin \ --no-tags --no-write-fetch-head --recurse-submodules=no \ --filter=blob:none --stdin <<< "$TREE_OID" packet: fetch< version 2 packet: fetch< agent=git/github-94ec3a8682aa-Linux packet: fetch< ls-refs=unborn packet: fetch< fetch=shallow wait-for-done filter packet: fetch< server-option packet: fetch< object-format=sha1 packet: fetch< 0000 packet: fetch> command=fetch packet: fetch> agent=git/2.51.2-Linux packet: fetch> object-format=sha1 packet: fetch> 0001 packet: fetch> thin-pack packet: fetch> no-progress packet: fetch> ofs-delta packet: fetch> filter blob:none packet: fetch> want 086faa42a038f2e4a1b5e395cde544f8f3c6a9a5 packet: fetch> done packet: fetch> 0000 packet: fetch< packfile packet: sideband< PACK ... packet: sideband< 0000 Zero have lines, noop negotiation doing its thing. Two interesting bits: 1. The client sends thin-pack unconditionally (fetch-pack.c doesn't check what negotiation algorithm is in use). So the server is allowed to send a thin pack. With zero haves it has nothing to thin against, but nothing on the wire actually forbids it. 2. filter blob:none is on the wire -- hardcoded by fetch_objects(). Combined with thin-pack, this gives the server a way to end up omitting an object that happens to be a delta base. For this particular tree GitHub responded cleanly (5 trees, no deltas, 2701 bytes), so whatever triggers the REF_DELTA depends on the specific objects being fetched and how they're stored server-side. > But it _can_ send deltas against objects which are part of the backfill > itself. Normally we'd send those as OFS_DELTA, because they're both in > the same output pack. But there are cases where we might not: > > - the verbatim pack-reuse code will sometimes rewrite ofs-delta into > ref-delta. I don't remember all of the cases where this might > happen. Certainly if the client hasn't claimed to support > ofs-deltas, but I think maybe some other cases? I'd have to dig into > it. I think it's (b). What's interesting is that Jonathan Tan flagged exactly this risk when he introduced the prefetch call. 8a30a1efd1 ("index-pack: prefetch missing REF_DELTA bases", 2019-05-14): Support for lazy fetching should still generally be turned off in index-pack because it is used as part of the lazy fetching process itself (if not, infinite loops may occur), but we do need to fetch the REF_DELTA bases. (When fetching REF_DELTA bases, it is unlikely that those are REF_DELTA themselves, because we do not send "have" when making such fetches.) So the "infinite loops may occur" door was left open deliberately, on the argument that it's unlikely. My patch just closes it. Here's what I think went wrong in practice: fetch_objects() hardcodes --filter=blob:none. When the want is a tree, the server walks tree reachability and assembles potentially millions of tree objects with no blobs. If pack-reuse passes through an object stored server-side as OFS_DELTA against a blob, it comes out as REF_DELTA and the blob base gets filtered out by blob:none. I have a live ps snapshot from the incident showing the chain: 3719679 git -c fetch.negotiationAlgorithm=noop fetch origin \ --no-tags --no-write-fetch-head \ --recurse-submodules=no --filter=blob:none --stdin 3725911 git index-pack --stdin --fix-thin --keep=fetch-pack \ 3719679 on <host> --promisor --pack_header=2,7535361 3726936 git -c fetch.negotiationAlgorithm=noop fetch origin \ --no-tags --no-write-fetch-head \ --recurse-submodules=no --filter=blob:none --stdin --keep embeds the parent PID, so the chain is clear: lazy-fetch (3719679) -> index-pack (3725911, 7,535,361 objects) -> depth-2 lazy-fetch (3726936). 7.5M objects is about right for wanting a single tree with no haves and filter=blob:none -- you get the entire commit+tree closure minus blobs. One REF_DELTA against a filtered blob in 7.5M objects and you're recursing. > Now there's a catch: the pack is not really thin, and so index-pack > should not need to do an extra backfill request in order to get the base > object. Actually, the pack IS thin in this case -- the REF_DELTA's base is a blob that the server filtered out because of blob:none. The base isn't in the pack at all. fix_unresolved_deltas() correctly sees it as unresolved and calls promisor_remote_get_direct(). > If that's what is happening, then I _think_ Paul's patch will do the > right thing. We'd say "no, we don't have that object" without doing the > backfill, and then eventually find it as part of the final resolution. Right, that's what happens. The child hits the guard at the top of fetch_objects() and bails with "lazy fetching disabled". The depth-1 index-pack can't resolve the delta, which is the correct outcome -- retrying the same noop fetch would just get the same response. I also wrote a deterministic local reproducer (no network needed) that shows the recursion directly. Two scripts, needs python3 + git, works on Linux and macOS. Ran it just now: repro-depth1.sh sets up a local bare server with two similar blobs, a blob:none partial-clone client, and a hand-crafted 68-byte thin pack with one REF_DELTA against a promised-but-absent blob: === without GIT_NO_LAZY_FETCH (current git) === index-pack invocations: 2 lazy-fetch spawns: 1 === with GIT_NO_LAZY_FETCH=1 (what the patch sets) === index-pack invocations: 1 lazy-fetch spawns: 0 repro-unbounded.sh goes further: uploadpack.packObjectsHook always returns the same thin pack, so every lazy-fetch spawns another. Watchdog kills it at depth 50: === without patch === recursion depth reached: 55 index-pack invocations: 55 tmp_pack_* on disk: 55 === with GIT_NO_LAZY_FETCH=1 (the patch) === recursion depth: 0 index-pack invocations: 1 tmp_pack_* on disk: 0 Those tmp_pack_* files are the disk growth -- each index-pack streams to a tmpfile, blocks in fix_unresolved_deltas() to lazy-fetch the base, and never gets to rename it. In the real incident each was ~1.4 GB. Scripts below. --- >8 --- repro-depth1.sh --- >8 --- #!/bin/bash # Deterministic depth-1 recursion proof for promisor-remote lazy-fetch bug. # No network. Runs in ~1 second. # # Mechanism: # - Local bare server with two similar blobs; client is a blob:none # partial clone (both blobs promised, neither local). # - Craft a 68-byte thin pack by hand: one REF_DELTA object whose base # SHA is one of the promised-but-absent blobs. # - Feed to `git -C client index-pack --stdin --fix-thin`. # - fix_unresolved_deltas() sees the unresolved REF_DELTA, calls # promisor_remote_get_direct() for the base, which spawns # `git -c fetch.negotiationAlgorithm=noop fetch ... --stdin`. # - That child fetch spawns its own index-pack --fix-thin. # - GIT_TRACE shows the nested spawn. set -euo pipefail W="${TMPDIR:-/tmp}/promisor-recursion-demo" rm -rf "$W"; mkdir -p "$W"; cd "$W" git -c init.defaultBranch=main init -q --bare server.git git -C server.git config uploadpack.allowFilter true git -C server.git config uploadpack.allowAnySHA1InWant true git -c init.defaultBranch=main init -q work ( cd work python3 -c "print('\n'.join('shared line %d ' * 4 % (i,i,i,i) for i in range(200)))" > f git add f; git commit -qm base python3 -c "print('\n'.join('shared line %d ' * 4 % (i,i,i,i) for i in range(200))); print('extra')" > f git add f; git commit -qm delta git push -q ../server.git main ) printf '%s\n%s\n' "$(git -C work rev-parse HEAD~1:f)" "$(git -C work rev-parse HEAD:f)" | \ git -C server.git pack-objects --stdout --delta-base-offset > both.pack python3 << 'PYEOF' import struct, zlib, hashlib with open('both.pack', 'rb') as f: data = f.read() assert data[:4] == b'PACK' and struct.unpack('>I', data[8:12])[0] == 2 def parse_hdr(data, pos): b = data[pos]; otype = (b >> 4) & 7; sz = b & 0xf; sh = 4; p = pos while b & 0x80: p += 1; b = data[p]; sz |= (b & 0x7f) << sh; sh += 7 return otype, sz, p + 1 def zlib_end(data, pos): d = zlib.decompressobj(); p = pos while p < len(data): chunk = data[p:p+512]; d.decompress(chunk) if d.unused_data: return p + len(chunk) - len(d.unused_data) if d.eof: return p + len(chunk) p += len(chunk) return p t1, s1, d1 = parse_hdr(data, 12) if t1 == 6: p = d1; b = data[p]; p += 1 while b & 0x80: b = data[p]; p += 1 e1 = zlib_end(data, p) else: e1 = zlib_end(data, d1) t2, s2, d2 = parse_hdr(data, e1) if t1 == 3 and t2 == 6: base_raw = zlib.decompress(data[d1:e1]) delta_sz, delta_hdr_end = s2, d2 elif t1 == 6 and t2 == 3: base_raw = zlib.decompress(data[d2:zlib_end(data, d2)]) delta_sz, delta_hdr_end = s1, d1 else: raise SystemExit(f"expected one blob + one OFS_DELTA, got types {t1},{t2}") base_sha = hashlib.sha1(f"blob {len(base_raw)}\0".encode() + base_raw).digest() p = delta_hdr_end; b = data[p]; p += 1 while b & 0x80: b = data[p]; p += 1 delta_zlib = data[p:zlib_end(data, p)] sz = delta_sz hb = [(7 << 4) | (sz & 0xf)]; sz >>= 4 while sz: hb[-1] |= 0x80; hb.append(sz & 0x7f); sz >>= 7 thin = b'PACK' + struct.pack('>II', 2, 1) + bytes(hb) + base_sha + delta_zlib thin += hashlib.sha1(thin).digest() with open('thin.pack', 'wb') as f: f.write(thin) print(f"thin.pack: {len(thin)} bytes, REF_DELTA base={base_sha.hex()}") PYEOF git clone -q --no-local --filter=blob:none --no-checkout "file://$W/server.git" client echo "=== without GIT_NO_LAZY_FETCH (current git) ===" GIT_TRACE="$W/trace.txt" \ git -C client index-pack --stdin --fix-thin < thin.pack >/dev/null 2>&1 || true grep -E 'built-in: git (index-pack|fetch)|run_command:.*negotiationAlgorithm' trace.txt echo "index-pack invocations: $(grep -c 'built-in: git index-pack' trace.txt)" echo "lazy-fetch spawns: $(grep -c 'run_command:.*negotiationAlgorithm=noop' trace.txt)" echo "" echo "=== with GIT_NO_LAZY_FETCH=1 (what the patch sets) ===" rm -rf client git clone -q --no-local --filter=blob:none --no-checkout "file://$W/server.git" client GIT_NO_LAZY_FETCH=1 GIT_TRACE="$W/trace2.txt" \ git -C client index-pack --stdin --fix-thin < thin.pack 2>&1 || true echo "index-pack invocations: $(grep -c 'built-in: git index-pack' trace2.txt)" echo "lazy-fetch spawns: $(grep -c 'run_command:.*negotiationAlgorithm=noop' trace2.txt || echo 0)" --- >8 --- repro-unbounded.sh --- >8 --- #!/bin/bash # Unbounded recursion proof. Run repro-depth1.sh first. # uploadpack.packObjectsHook always returns the same thin pack, # so every lazy-fetch spawns another. Watchdog kills at depth 50. set -uo pipefail W="${TMPDIR:-/tmp}/promisor-recursion-demo" cd "$W" test -f thin.pack || { echo "run repro-depth1.sh first"; exit 1; } cat > evil-pack-objects.sh << EOF #!/bin/sh cat > /dev/null cat "$W/thin.pack" EOF chmod +x evil-pack-objects.sh cat > fake-global.gitconfig << EOF [uploadpack] packObjectsHook = $W/evil-pack-objects.sh EOF rm -rf client GIT_CONFIG_GLOBAL= git clone -q --no-local --filter=blob:none --no-checkout \ "file://$W/server.git" client TRACE="$W/unbounded.trace" rm -f "$TRACE" echo "=== spawning (watchdog kills at depth 50) ===" GIT_TRACE="$TRACE" GIT_CONFIG_GLOBAL="$W/fake-global.gitconfig" \ git -C client index-pack --stdin --fix-thin < thin.pack >/dev/null 2>&1 & ROOT=$! LIMIT=50 while kill -0 "$ROOT" 2>/dev/null; do N=$(grep -c 'run_command:.*negotiationAlgorithm=noop' "$TRACE" 2>/dev/null || true) N=${N:-0} if (( N >= LIMIT )); then echo ">>> depth $N reached, killing process tree $ROOT <<<" pkill -KILL -f "promisor-recursion-demo/client" 2>/dev/null || true kill -KILL "$ROOT" 2>/dev/null || true sleep 0.2 pkill -KILL -f "promisor-recursion-demo/client" 2>/dev/null || true break fi sleep 0.02 done wait "$ROOT" 2>/dev/null EC=$? DEPTH=$(grep -c 'run_command:.*negotiationAlgorithm=noop' "$TRACE" 2>/dev/null || true) DEPTH=${DEPTH:-0} IPACKS=$(grep -c 'built-in: git index-pack' "$TRACE" 2>/dev/null || true) IPACKS=${IPACKS:-0} TMPS=$(ls client/.git/objects/pack/tmp_pack_* 2>/dev/null | wc -l | tr -d ' ') echo "" echo "=== without patch (root exit $EC) ===" echo "recursion depth reached: $DEPTH" echo "index-pack invocations: $IPACKS" echo "tmp_pack_* on disk: $TMPS" echo "" echo "first 4 + last 2 spawns (identical line, no termination condition):" grep -E 'built-in: git index-pack|run_command:.*negotiationAlgorithm=noop' "$TRACE" | head -4 echo "..." grep -E 'built-in: git index-pack|run_command:.*negotiationAlgorithm=noop' "$TRACE" | tail -2 echo "" echo "=== with GIT_NO_LAZY_FETCH=1 (the patch) ===" rm -rf client; rm -f "$TRACE" GIT_CONFIG_GLOBAL= git clone -q --no-local --filter=blob:none --no-checkout \ "file://$W/server.git" client GIT_NO_LAZY_FETCH=1 GIT_TRACE="$TRACE" GIT_CONFIG_GLOBAL="$W/fake-global.gitconfig" \ git -C client index-pack --stdin --fix-thin < thin.pack 2>&1 || true CTRL_DEPTH=$(grep -c 'run_command:.*negotiationAlgorithm=noop' "$TRACE" 2>/dev/null || true) CTRL_IPACKS=$(grep -c 'built-in: git index-pack' "$TRACE" 2>/dev/null || true) CTRL_TMPS=$(ls client/.git/objects/pack/tmp_pack_* 2>/dev/null | wc -l | tr -d ' ') echo "recursion depth: ${CTRL_DEPTH:-0}" echo "index-pack invocations: ${CTRL_IPACKS:-0}" echo "tmp_pack_* on disk: $CTRL_TMPS" rm -rf "$W/client" Paul ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] promisor-remote: prevent lazy-fetch recursion in child fetch 2026-03-12 7:27 ` Patrick Steinhardt 2026-03-13 1:43 ` Jeff King @ 2026-03-13 12:43 ` Paul Tarjan 1 sibling, 0 replies; 11+ messages in thread From: Paul Tarjan @ 2026-03-13 12:43 UTC (permalink / raw) To: git; +Cc: ps, peff, gitgitgadget, christian.couder, hanxin.hx Patrick Steinhardt <ps@pks.im> writes: > I must be misunderstanding something here, but how is it that a commit > can be garbage collected if a ref points to it? That shouldn't ever > happen, as reachable commits should not be pruned. > > Or do you mean to say that the commits don't exist on the server side > anymore? Sloppy wording on my part — "GC'd" is wrong. These refs pointed at commits that were promised but never materialized on the partial clone. The ~77K broken refs looked like: error: refs/prefetch/remotes/origin/claude/add-azure-dependencies-EaaDn \ does not point to a valid object! They were created by git maintenance --task=prefetch, which runs git fetch --prefetch --prune and writes refs/prefetch/remotes/origin/<branch> pointing at the remote tip. On a blob:none partial clone it fetches commit/tree metadata, but some referenced commits were never actually downloaded before the upstream branches (ephemeral CI/automation branches, force-pushed and deleted within days) disappeared. This is a red herring for the patch though. The stale prefetch refs explain why the outer fetch got a thin pack — the client advertised haves from promised-but-absent commits. But the recursion (depth-1 to depth-2+) is entirely inside fetch_objects() with noop negotiation, independent of any refs. I'll fix the commit message wording in a v4 if you'd like. > That's certainly curious. Do you maybe have multiple remotes attached to > the repository, or are you dropping/modifying the object filter at some > point? > > All subsequent fetches need to use the same object filter as you've used > during the initial clone, otherwise you may run into a situation as you > have described. But in theory, Git knows to continue using the filter. Nobody intentionally changed the filter. What happened is the lazy-fetch child kept re-writing it. fetch_objects() hardcodes --filter=blob:none on the child argv, and the child's builtin/fetch.c writes the active filter to config. 23547c40 ("fetch: do not override partial clone filter", 2020) guards this write behind a check for an already-set filter. But I was unsetting remote.origin.partialclonefilter manually trying to stop the storm, so the guard passed and the next lazy-fetch child wrote it right back: 21:45 (unset) manual git config --unset 22:05 blob:none re-written by a lazy-fetch child 22:11 (unset) manual unset again 23:11 blob:none re-written again 23:13 (unset) manual unset 23:28 blob:none re-written (caught live by a config-mtime trap) The actual mitigation was unsetting remote.origin.promisor too — with no promisor remote, fix_unresolved_deltas() skips the prefetch entirely. This is arguably a separate bug: fetch_objects() should probably pass -c remote.<name>.partialclonefilter=blob:none to override for the single invocation, rather than --filter=blob:none which persists to config. Not in scope for this patch, but I could follow up separately if there's interest. Paul ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3] promisor-remote: prevent lazy-fetch recursion in child fetch 2026-03-04 18:27 ` [PATCH v2] " Paul Tarjan via GitGitGadget 2026-03-11 10:52 ` Patrick Steinhardt @ 2026-03-11 14:19 ` Paul Tarjan via GitGitGadget 1 sibling, 0 replies; 11+ messages in thread From: Paul Tarjan via GitGitGadget @ 2026-03-11 14:19 UTC (permalink / raw) To: git Cc: Christian Couder, Han Xin, Paul Tarjan, Patrick Steinhardt, Paul Tarjan, Paul Tarjan From: Paul Tarjan <github@paulisageek.com> fetch_objects() spawns a child `git fetch` to lazily fill in missing objects. That child's index-pack, when it receives a thin pack containing a REF_DELTA against a still-missing base, calls promisor_remote_get_direct() -- which is fetch_objects() again. With negotiationAlgorithm=noop the client advertises no "have" lines, so a well-behaved server sends requested objects un-deltified or deltified only against objects in the same pack. A server that nevertheless sends REF_DELTA against a base the client does not have is misbehaving; however the client should not recurse unboundedly in response. Propagate GIT_NO_LAZY_FETCH=1 into the child fetch's environment so that if the child's index-pack encounters such a REF_DELTA, it hits the existing guard at the top of fetch_objects() and fails fast instead of recursing. Depth-1 lazy fetch (the whole point of fetch_objects()) is unaffected: only the child and its descendants see the variable. Add a test that injects a thin pack containing a REF_DELTA against a missing base via HTTP, triggering the recursion path through index-pack's promisor_remote_get_direct() call. With the fix, the child's fetch_objects() sees GIT_NO_LAZY_FETCH=1 and blocks the depth-2 fetch with a "lazy fetching disabled" warning. Signed-off-by: Paul Tarjan <github@paulisageek.com> --- promisor-remote: prevent recursive lazy-fetch during index-pack Propagate GIT_NO_LAZY_FETCH=1 into the child fetch spawned by fetch_objects() so that index-pack cannot recurse back into lazy-fetch when resolving REF_DELTA bases. We hit this in production: 276 GB of promisor packs written in 90 minutes against a ~10 GB monorepo with ~61K stale prefetch refs pointing at GC'd commits. Changes since v2: * Replaced env-var-proxy test with behavioral test that injects a thin pack containing a REF_DELTA against a missing base via HTTP, triggering the actual recursion path through index-pack's promisor_remote_get_direct() call * Moved test from t0411-clone-from-partial.sh to t5616-partial-clone.sh (requires HTTP infrastructure) * Dropped commit-graph comparison from commit message per review feedback Changes since v1: * Dropped CC: trailers from commit message (moved here for GitGitGadget) * Moved test into t0411-clone-from-partial.sh instead of a new file * Removed duplicate commit-message summary from PR description Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2224%2Fptarjan%2Fclaude%2Ffix-lazy-fetch-recursion-KP9Hl-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2224/ptarjan/claude/fix-lazy-fetch-recursion-KP9Hl-v3 Pull-Request: https://github.com/git/git/pull/2224 Range-diff vs v2: 1: 907ca7a0ac ! 1: d58ccb858f promisor-remote: prevent lazy-fetch recursion in child fetch @@ Commit message fetch_objects() spawns a child `git fetch` to lazily fill in missing objects. That child's index-pack, when it receives a thin pack - containing a REF_DELTA against a still-missing base, explicitly - calls promisor_remote_get_direct() — which is fetch_objects() again. - If the base is truly unavailable (e.g. because many refs in the - local store point at objects that have been garbage-collected on the - server), each recursive lazy-fetch can trigger another, leading to - unbounded recursion with runaway disk and process consumption. + containing a REF_DELTA against a still-missing base, calls + promisor_remote_get_direct() -- which is fetch_objects() again. - The GIT_NO_LAZY_FETCH guard (introduced by e6d5479e7a (git: add - --no-lazy-fetch option, 2021-08-31)) already exists at the top of - fetch_objects(); the missing piece is propagating it into the child - fetch's environment. Add that propagation so the child's - index-pack, if it encounters a REF_DELTA against a missing base, - hits the guard and fails fast instead of recursing. - - Depth-1 lazy fetch (the whole point of fetch_objects()) is - unaffected: only the child and its descendants see the variable. With negotiationAlgorithm=noop the client advertises no "have" lines, so a well-behaved server sends requested objects - un-deltified or deltified only against objects in the same pack; - the child's index-pack should never need a depth-2 fetch. If it - does, the server response was broken or the local store is already - corrupt, and further fetching would not help. + un-deltified or deltified only against objects in the same pack. + A server that nevertheless sends REF_DELTA against a base the + client does not have is misbehaving; however the client should + not recurse unboundedly in response. - This is the same bug shape that 3a1ea94a49 (commit-graph.c: no lazy - fetch in lookup_commit_in_graph(), 2022-07-01) addressed at a - different entry point. + Propagate GIT_NO_LAZY_FETCH=1 into the child fetch's environment + so that if the child's index-pack encounters such a REF_DELTA, it + hits the existing guard at the top of fetch_objects() and fails + fast instead of recursing. Depth-1 lazy fetch (the whole point + of fetch_objects()) is unaffected: only the child and its + descendants see the variable. - Add a test that verifies the child fetch environment contains - GIT_NO_LAZY_FETCH=1 via a reference-transaction hook. + Add a test that injects a thin pack containing a REF_DELTA against + a missing base via HTTP, triggering the recursion path through + index-pack's promisor_remote_get_direct() call. With the fix, the + child's fetch_objects() sees GIT_NO_LAZY_FETCH=1 and blocks the + depth-2 fetch with a "lazy fetching disabled" warning. Signed-off-by: Paul Tarjan <github@paulisageek.com> @@ promisor-remote.c: static int fetch_objects(struct repository *repo, "fetch", remote_name, "--no-tags", "--no-write-fetch-head", "--recurse-submodules=no", - ## t/t0411-clone-from-partial.sh ## -@@ t/t0411-clone-from-partial.sh: test_expect_success 'promisor lazy-fetching can be re-enabled' ' - test_path_is_file script-executed + ## t/t5616-partial-clone.sh ## +@@ t/t5616-partial-clone.sh: test_expect_success PERL_TEST_HELPERS 'tolerate server sending REF_DELTA against + ! test -e "$HTTPD_ROOT_PATH/one-time-script" ' -+test_expect_success 'lazy-fetch child has GIT_NO_LAZY_FETCH=1' ' -+ test_create_repo nolazy-server && -+ test_commit -C nolazy-server foo && -+ git -C nolazy-server repack -a -d --write-bitmap-index && ++test_expect_success PERL_TEST_HELPERS 'lazy-fetch of REF_DELTA with missing base does not recurse' ' ++ SERVER="$HTTPD_DOCUMENT_ROOT_PATH/server" && ++ rm -rf "$SERVER" repo && ++ test_create_repo "$SERVER" && ++ test_config -C "$SERVER" uploadpack.allowfilter 1 && ++ test_config -C "$SERVER" uploadpack.allowanysha1inwant 1 && ++ ++ # Create a commit with 2 blobs to be used as delta base and content. ++ for i in $(test_seq 10) ++ do ++ echo "this is a line" >>"$SERVER/foo.txt" && ++ echo "this is another line" >>"$SERVER/bar.txt" || return 1 ++ done && ++ git -C "$SERVER" add foo.txt bar.txt && ++ git -C "$SERVER" commit -m initial && ++ BLOB_FOO=$(git -C "$SERVER" rev-parse HEAD:foo.txt) && ++ BLOB_BAR=$(git -C "$SERVER" rev-parse HEAD:bar.txt) && + -+ git clone "file://$(pwd)/nolazy-server" nolazy-client && -+ HASH=$(git -C nolazy-client rev-parse foo) && -+ rm -rf nolazy-client/.git/objects/* && ++ # Partial clone with blob:none. The client has commits and ++ # trees but no blobs. ++ test_config -C "$SERVER" protocol.version 2 && ++ git -c protocol.version=2 clone --no-checkout \ ++ --filter=blob:none $HTTPD_URL/one_time_script/server repo && + -+ git -C nolazy-client config core.repositoryformatversion 1 && -+ git -C nolazy-client config extensions.partialclone "origin" && ++ # Sanity check: client does not have either blob locally. ++ git -C repo rev-list --objects --ignore-missing \ ++ -- $BLOB_FOO >objlist && ++ test_line_count = 0 objlist && + -+ # Install a reference-transaction hook to record the env var -+ # as seen by processes inside the child fetch. -+ test_hook -C nolazy-client reference-transaction <<-\EOF && -+ echo "$GIT_NO_LAZY_FETCH" >>../env-in-child ++ # Craft a thin pack where BLOB_FOO is a REF_DELTA against ++ # BLOB_BAR. Since the client has neither blob (blob:none ++ # filter), the delta base will be missing. This simulates a ++ # misbehaving server that sends REF_DELTA against an object ++ # the client does not have. ++ test-tool -C "$SERVER" pack-deltas --num-objects=1 >thin.pack <<-EOF && ++ REF_DELTA $BLOB_FOO $BLOB_BAR + EOF + -+ rm -f env-in-child && -+ git -C nolazy-client cat-file -p "$HASH" && ++ replace_packfile thin.pack && + -+ # The hook runs inside the child fetch, which should have -+ # GIT_NO_LAZY_FETCH=1 in its environment. -+ grep "^1$" env-in-child ++ # Trigger a lazy fetch for BLOB_FOO. The child fetch spawned ++ # by fetch_objects() receives our crafted thin pack. Its ++ # index-pack encounters the missing delta base (BLOB_BAR) and ++ # tries to lazy-fetch it via promisor_remote_get_direct(). ++ # ++ # With the fix: fetch_objects() propagates GIT_NO_LAZY_FETCH=1 ++ # to the child, so the depth-2 fetch is blocked and we see the ++ # "lazy fetching disabled" warning. The object cannot be ++ # resolved, so cat-file fails. ++ # ++ # Without the fix: the depth-2 fetch would proceed, potentially ++ # recursing unboundedly with a persistently misbehaving server. ++ test_must_fail git -C repo -c protocol.version=2 \ ++ cat-file -p $BLOB_FOO 2>err && ++ test_grep "lazy fetching disabled" err && ++ ++ # Ensure that the one-time-script was used. ++ ! test -e "$HTTPD_ROOT_PATH/one-time-script" +' + - test_done + # DO NOT add non-httpd-specific tests here, because the last part of this + # test script is only executed when httpd is available and enabled. + promisor-remote.c | 7 +++++ t/t5616-partial-clone.sh | 60 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/promisor-remote.c b/promisor-remote.c index 96fa215b06..35c7aab93d 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -42,6 +42,13 @@ static int fetch_objects(struct repository *repo, child.in = -1; if (repo != the_repository) prepare_other_repo_env(&child.env, repo->gitdir); + /* + * Prevent the child's index-pack from recursing back into + * fetch_objects() when resolving REF_DELTA bases it does not + * have. With noop negotiation the server should never need + * to send such deltas, so a depth-2 fetch would not help. + */ + strvec_pushf(&child.env, "%s=1", NO_LAZY_FETCH_ENVIRONMENT); strvec_pushl(&child.args, "-c", "fetch.negotiationAlgorithm=noop", "fetch", remote_name, "--no-tags", "--no-write-fetch-head", "--recurse-submodules=no", diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh index 1e354e057f..27f131c8d9 100755 --- a/t/t5616-partial-clone.sh +++ b/t/t5616-partial-clone.sh @@ -907,6 +907,66 @@ test_expect_success PERL_TEST_HELPERS 'tolerate server sending REF_DELTA against ! test -e "$HTTPD_ROOT_PATH/one-time-script" ' +test_expect_success PERL_TEST_HELPERS 'lazy-fetch of REF_DELTA with missing base does not recurse' ' + SERVER="$HTTPD_DOCUMENT_ROOT_PATH/server" && + rm -rf "$SERVER" repo && + test_create_repo "$SERVER" && + test_config -C "$SERVER" uploadpack.allowfilter 1 && + test_config -C "$SERVER" uploadpack.allowanysha1inwant 1 && + + # Create a commit with 2 blobs to be used as delta base and content. + for i in $(test_seq 10) + do + echo "this is a line" >>"$SERVER/foo.txt" && + echo "this is another line" >>"$SERVER/bar.txt" || return 1 + done && + git -C "$SERVER" add foo.txt bar.txt && + git -C "$SERVER" commit -m initial && + BLOB_FOO=$(git -C "$SERVER" rev-parse HEAD:foo.txt) && + BLOB_BAR=$(git -C "$SERVER" rev-parse HEAD:bar.txt) && + + # Partial clone with blob:none. The client has commits and + # trees but no blobs. + test_config -C "$SERVER" protocol.version 2 && + git -c protocol.version=2 clone --no-checkout \ + --filter=blob:none $HTTPD_URL/one_time_script/server repo && + + # Sanity check: client does not have either blob locally. + git -C repo rev-list --objects --ignore-missing \ + -- $BLOB_FOO >objlist && + test_line_count = 0 objlist && + + # Craft a thin pack where BLOB_FOO is a REF_DELTA against + # BLOB_BAR. Since the client has neither blob (blob:none + # filter), the delta base will be missing. This simulates a + # misbehaving server that sends REF_DELTA against an object + # the client does not have. + test-tool -C "$SERVER" pack-deltas --num-objects=1 >thin.pack <<-EOF && + REF_DELTA $BLOB_FOO $BLOB_BAR + EOF + + replace_packfile thin.pack && + + # Trigger a lazy fetch for BLOB_FOO. The child fetch spawned + # by fetch_objects() receives our crafted thin pack. Its + # index-pack encounters the missing delta base (BLOB_BAR) and + # tries to lazy-fetch it via promisor_remote_get_direct(). + # + # With the fix: fetch_objects() propagates GIT_NO_LAZY_FETCH=1 + # to the child, so the depth-2 fetch is blocked and we see the + # "lazy fetching disabled" warning. The object cannot be + # resolved, so cat-file fails. + # + # Without the fix: the depth-2 fetch would proceed, potentially + # recursing unboundedly with a persistently misbehaving server. + test_must_fail git -C repo -c protocol.version=2 \ + cat-file -p $BLOB_FOO 2>err && + test_grep "lazy fetching disabled" err && + + # Ensure that the one-time-script was used. + ! test -e "$HTTPD_ROOT_PATH/one-time-script" +' + # DO NOT add non-httpd-specific tests here, because the last part of this # test script is only executed when httpd is available and enabled. base-commit: 7b2bccb0d58d4f24705bf985de1f4612e4cf06e5 -- gitgitgadget ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-03-13 12:43 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-04 16:57 [PATCH] promisor-remote: prevent lazy-fetch recursion in child fetch Paul Tarjan via GitGitGadget 2026-03-04 17:41 ` Junio C Hamano 2026-03-04 18:20 ` Paul Tarjan 2026-03-04 18:27 ` [PATCH v2] " Paul Tarjan via GitGitGadget 2026-03-11 10:52 ` Patrick Steinhardt 2026-03-11 14:18 ` Paul Tarjan 2026-03-12 7:27 ` Patrick Steinhardt 2026-03-13 1:43 ` Jeff King 2026-03-13 12:43 ` [PATCH v3] " Paul Tarjan 2026-03-13 12:43 ` Paul Tarjan 2026-03-11 14:19 ` Paul Tarjan via GitGitGadget
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox