Merge branch 'ps/upload-pack-oom-protection'

A broken or malicious "git fetch" can say that it has the same
object for many many times, and the upload-pack serving it can
exhaust memory storing them redundantly, which has been corrected.

* ps/upload-pack-oom-protection:
  upload-pack: don't ACK non-commits repeatedly in protocol v2
  t5530: modernize tests
main
Junio C Hamano 2025-09-15 08:52:06 -07:00
commit a93ec6da42
2 changed files with 51 additions and 36 deletions

View File

@ -4,8 +4,6 @@ test_description='errors in upload-pack'

. ./test-lib.sh

D=$(pwd)

corrupt_repo () {
object_sha1=$(git rev-parse "$1") &&
ob=$(expr "$object_sha1" : "\(..\)") &&
@ -21,11 +19,7 @@ test_expect_success 'setup and corrupt repository' '
test_tick &&
echo changed >file &&
git commit -a -m changed &&
corrupt_repo HEAD:file

'

test_expect_success 'fsck fails' '
corrupt_repo HEAD:file &&
test_must_fail git fsck
'

@ -40,17 +34,12 @@ test_expect_success 'upload-pack fails due to error in pack-objects packing' '
'

test_expect_success 'corrupt repo differently' '

git hash-object -w file &&
corrupt_repo HEAD^^{tree}

'

test_expect_success 'fsck fails' '
corrupt_repo HEAD^^{tree} &&
test_must_fail git fsck
'
test_expect_success 'upload-pack fails due to error in rev-list' '

test_expect_success 'upload-pack fails due to error in rev-list' '
printf "%04xwant %s\n%04xshallow %s00000009done\n0000" \
$(($hexsz + 10)) $(git rev-parse HEAD) \
$(($hexsz + 12)) $(git rev-parse HEAD^) >input &&
@ -59,7 +48,6 @@ test_expect_success 'upload-pack fails due to error in rev-list' '
'

test_expect_success 'upload-pack fails due to bad want (no object)' '

printf "%04xwant %s multi_ack_detailed\n00000009done\n0000" \
$(($hexsz + 29)) $(test_oid deadbeef) >input &&
test_must_fail git upload-pack . <input >output 2>output.err &&
@ -69,7 +57,6 @@ test_expect_success 'upload-pack fails due to bad want (no object)' '
'

test_expect_success 'upload-pack fails due to bad want (not tip)' '

oid=$(echo an object we have | git hash-object -w --stdin) &&
printf "%04xwant %s multi_ack_detailed\n00000009done\n0000" \
$(($hexsz + 29)) "$oid" >input &&
@ -80,7 +67,6 @@ test_expect_success 'upload-pack fails due to bad want (not tip)' '
'

test_expect_success 'upload-pack fails due to error in pack-objects enumeration' '

printf "%04xwant %s\n00000009done\n0000" \
$((hexsz + 10)) $(git rev-parse HEAD) >input &&
test_must_fail git upload-pack . <input >/dev/null 2>output.err &&
@ -105,18 +91,48 @@ test_expect_success 'upload-pack tolerates EOF just after stateless client wants
test_cmp expect actual
'

test_expect_success 'create empty repository' '

mkdir foo &&
cd foo &&
git init

test_expect_success 'fetch fails' '
git init foo &&
test_must_fail git -C foo fetch .. main
'

test_expect_success 'fetch fails' '

test_must_fail git fetch .. main
test_expect_success 'upload-pack ACKs repeated non-commit objects repeatedly (protocol v0)' '
commit_id=$(git rev-parse HEAD) &&
tree_id=$(git rev-parse HEAD^{tree}) &&
test-tool pkt-line pack >request <<-EOF &&
want $commit_id
0000
have $tree_id
have $tree_id
0000
EOF
git upload-pack --stateless-rpc . <request >actual &&
depacketize <actual >actual.raw &&
grep ^ACK actual.raw >actual.acks &&
cat >expect <<-EOF &&
ACK $tree_id
ACK $tree_id
EOF
test_cmp expect actual.acks
'

test_expect_success 'upload-pack ACKs repeated non-commit objects once only (protocol v2)' '
commit_id=$(git rev-parse HEAD) &&
tree_id=$(git rev-parse HEAD^{tree}) &&
test-tool pkt-line pack >request <<-EOF &&
command=fetch
object-format=$(test_oid algo)
0001
want $commit_id
have $tree_id
have $tree_id
0000
EOF
GIT_PROTOCOL=version=2 git upload-pack . <request >actual &&
depacketize <actual >actual.raw &&
grep ^ACK actual.raw >actual.acks &&
echo "ACK $tree_id" >expect &&
test_cmp expect actual.acks
'

test_done

View File

@ -476,20 +476,17 @@ static void create_pack_file(struct upload_pack_data *pack_data,

static int do_got_oid(struct upload_pack_data *data, const struct object_id *oid)
{
int we_knew_they_have = 0;
struct object *o = parse_object_with_flags(the_repository, oid,
PARSE_OBJECT_SKIP_HASH_CHECK |
PARSE_OBJECT_DISCARD_TREE);

if (!o)
die("oops (%s)", oid_to_hex(oid));

if (o->type == OBJ_COMMIT) {
struct commit_list *parents;
struct commit *commit = (struct commit *)o;
if (o->flags & THEY_HAVE)
we_knew_they_have = 1;
else
o->flags |= THEY_HAVE;

if (!data->oldest_have || (commit->date < data->oldest_have))
data->oldest_have = commit->date;
for (parents = commit->parents;
@ -497,11 +494,13 @@ static int do_got_oid(struct upload_pack_data *data, const struct object_id *oid
parents = parents->next)
parents->item->object.flags |= THEY_HAVE;
}
if (!we_knew_they_have) {
add_object_array(o, NULL, &data->have_obj);
return 1;
}
return 0;

if (o->flags & THEY_HAVE)
return 0;
o->flags |= THEY_HAVE;

add_object_array(o, NULL, &data->have_obj);
return 1;
}

static int got_oid(struct upload_pack_data *data,