Skip to content

Commit 3b910d0

Browse files
peffgitster
authored andcommitted
add tests for indexing packs with delta cycles
If we receive a broken or malicious pack from a remote, we will feed it to index-pack. As index-pack processes the objects as a stream, reconstructing and hashing each object to get its name, it is not very susceptible to doing the wrong with bad data (it simply notices that the data is bogus and aborts). However, one question raised on the list is whether it could be susceptible to problems during the delta-resolution phase. In particular, can a cycle in the packfile deltas cause us to go into an infinite loop or cause any other problem? The answer is no. We cannot have a cycle of delta-base offsets, because they go only in one direction (the OFS_DELTA object mentions its base by an offset towards the beginning of the file, and we explicitly reject negative offsets). We can have a cycle of REF_DELTA objects, which refer to base objects by sha1 name. However, index-pack does not know these sha1 names ahead of time; it has to reconstruct the objects to get their names, and it cannot do so if there is a delta cycle (in other words, it does not even realize there is a cycle, but only that there are items that cannot be resolved). Even though we can reason out that index-pack should handle this fine, let's add a few tests to make sure it behaves correctly. Signed-off-by: Jeff King <[email protected]> Acked-by: Nicolas Pitre <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 171bdac commit 3b910d0

File tree

2 files changed

+81
-0
lines changed

2 files changed

+81
-0
lines changed

t/lib-pack.sh

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,28 @@ pack_obj () {
5555
printf '\062\170\234\143\267\3\0\0\116\0\106'
5656
return
5757
;;
58+
01d7713666f4de822776c7622c10f1b07de280dc)
59+
printf '\165\1\327\161\66\146\364\336\202\47\166' &&
60+
printf '\307\142\54\20\361\260\175\342\200\334\170' &&
61+
printf '\234\143\142\142\142\267\003\0\0\151\0\114'
62+
return
63+
;;
64+
esac
65+
;;
66+
67+
# blob containing "\7\0"
68+
01d7713666f4de822776c7622c10f1b07de280dc)
69+
case "$2" in
70+
'')
71+
printf '\062\170\234\143\147\0\0\0\20\0\10'
72+
return
73+
;;
74+
e68fe8129b546b101aee9510c5328e7f21ca1d18)
75+
printf '\165\346\217\350\22\233\124\153\20\32\356' &&
76+
printf '\225\20\305\62\216\177\41\312\35\30\170\234' &&
77+
printf '\143\142\142\142\147\0\0\0\53\0\16'
78+
return
79+
;;
5880
esac
5981
;;
6082
esac

t/t5309-pack-delta-cycles.sh

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
#!/bin/sh
2+
3+
test_description='test index-pack handling of delta cycles in packfiles'
4+
. ./test-lib.sh
5+
. "$TEST_DIRECTORY"/lib-pack.sh
6+
7+
# Two similar-ish objects that we have computed deltas between.
8+
A=01d7713666f4de822776c7622c10f1b07de280dc
9+
B=e68fe8129b546b101aee9510c5328e7f21ca1d18
10+
11+
# double-check our hand-constucted packs
12+
test_expect_success 'index-pack works with a single delta (A->B)' '
13+
clear_packs &&
14+
{
15+
pack_header 2 &&
16+
pack_obj $A $B &&
17+
pack_obj $B
18+
} >ab.pack &&
19+
pack_trailer ab.pack &&
20+
git index-pack --stdin <ab.pack &&
21+
git cat-file -t $A &&
22+
git cat-file -t $B
23+
'
24+
25+
test_expect_success 'index-pack works with a single delta (B->A)' '
26+
clear_packs &&
27+
{
28+
pack_header 2 &&
29+
pack_obj $A &&
30+
pack_obj $B $A
31+
} >ba.pack &&
32+
pack_trailer ba.pack &&
33+
git index-pack --stdin <ba.pack &&
34+
git cat-file -t $A &&
35+
git cat-file -t $B
36+
'
37+
38+
test_expect_success 'index-pack detects missing base objects' '
39+
clear_packs &&
40+
{
41+
pack_header 1 &&
42+
pack_obj $A $B
43+
} >missing.pack &&
44+
pack_trailer missing.pack &&
45+
test_must_fail git index-pack --fix-thin --stdin <missing.pack
46+
'
47+
48+
test_expect_success 'index-pack detects REF_DELTA cycles' '
49+
clear_packs &&
50+
{
51+
pack_header 2 &&
52+
pack_obj $A $B &&
53+
pack_obj $B $A
54+
} >cycle.pack &&
55+
pack_trailer cycle.pack &&
56+
test_must_fail git index-pack --fix-thin --stdin <cycle.pack
57+
'
58+
59+
test_done

0 commit comments

Comments
 (0)