Skip to content

Commit 53ad040

Browse files
derrickstoleegitster
authored andcommitted
multi-pack-index: verify bad header
When verifying if a multi-pack-index file is valid, we want the command to fail to signal an invalid file. Previously, we wrote an error to stderr and continued as if we had no multi-pack-index. Now, die() instead of error(). Add tests that check corrupted headers in a few ways: * Bad signature * Bad file version * Bad hash version * Truncated hash count * Extended hash count Signed-off-by: Derrick Stolee <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 56ee7ff commit 53ad040

File tree

2 files changed

+51
-13
lines changed

2 files changed

+51
-13
lines changed

midx.c

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -76,24 +76,18 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local
7676
m->local = local;
7777

7878
m->signature = get_be32(m->data);
79-
if (m->signature != MIDX_SIGNATURE) {
80-
error(_("multi-pack-index signature 0x%08x does not match signature 0x%08x"),
79+
if (m->signature != MIDX_SIGNATURE)
80+
die(_("multi-pack-index signature 0x%08x does not match signature 0x%08x"),
8181
m->signature, MIDX_SIGNATURE);
82-
goto cleanup_fail;
83-
}
8482

8583
m->version = m->data[MIDX_BYTE_FILE_VERSION];
86-
if (m->version != MIDX_VERSION) {
87-
error(_("multi-pack-index version %d not recognized"),
84+
if (m->version != MIDX_VERSION)
85+
die(_("multi-pack-index version %d not recognized"),
8886
m->version);
89-
goto cleanup_fail;
90-
}
9187

9288
hash_version = m->data[MIDX_BYTE_HASH_VERSION];
93-
if (hash_version != MIDX_HASH_VERSION) {
94-
error(_("hash version %u does not match"), hash_version);
95-
goto cleanup_fail;
96-
}
89+
if (hash_version != MIDX_HASH_VERSION)
90+
die(_("hash version %u does not match"), hash_version);
9791
m->hash_len = MIDX_HASH_LEN;
9892

9993
m->num_chunks = m->data[MIDX_BYTE_NUM_CHUNKS];

t/t5319-multi-pack-index.sh

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,51 @@ test_expect_success 'verify multi-pack-index success' '
154154
git multi-pack-index verify --object-dir=$objdir
155155
'
156156

157+
# usage: corrupt_midx_and_verify <pos> <data> <objdir> <string>
158+
corrupt_midx_and_verify() {
159+
POS=$1 &&
160+
DATA="${2:-\0}" &&
161+
OBJDIR=$3 &&
162+
GREPSTR="$4" &&
163+
FILE=$OBJDIR/pack/multi-pack-index &&
164+
chmod a+w $FILE &&
165+
test_when_finished mv midx-backup $FILE &&
166+
cp $FILE midx-backup &&
167+
printf "$DATA" | dd of="$FILE" bs=1 seek="$POS" conv=notrunc &&
168+
test_must_fail git multi-pack-index verify --object-dir=$OBJDIR 2>test_err &&
169+
grep -v "^+" test_err >err &&
170+
test_i18ngrep "$GREPSTR" err
171+
}
172+
173+
test_expect_success 'verify bad signature' '
174+
corrupt_midx_and_verify 0 "\00" $objdir \
175+
"multi-pack-index signature"
176+
'
177+
178+
MIDX_BYTE_VERSION=4
179+
MIDX_BYTE_OID_VERSION=5
180+
MIDX_BYTE_CHUNK_COUNT=6
181+
182+
test_expect_success 'verify bad version' '
183+
corrupt_midx_and_verify $MIDX_BYTE_VERSION "\00" $objdir \
184+
"multi-pack-index version"
185+
'
186+
187+
test_expect_success 'verify bad OID version' '
188+
corrupt_midx_and_verify $MIDX_BYTE_OID_VERSION "\02" $objdir \
189+
"hash version"
190+
'
191+
192+
test_expect_success 'verify truncated chunk count' '
193+
corrupt_midx_and_verify $MIDX_BYTE_CHUNK_COUNT "\01" $objdir \
194+
"missing required"
195+
'
196+
197+
test_expect_success 'verify extended chunk count' '
198+
corrupt_midx_and_verify $MIDX_BYTE_CHUNK_COUNT "\07" $objdir \
199+
"terminating multi-pack-index chunk id appears earlier than expected"
200+
'
201+
157202
test_expect_success 'repack removes multi-pack-index' '
158203
test_path_is_file $objdir/pack/multi-pack-index &&
159204
git repack -adf &&
@@ -191,7 +236,6 @@ test_expect_success 'multi-pack-index in an alternate' '
191236

192237
compare_results_with_midx "with alternate (remote midx)"
193238

194-
195239
# usage: corrupt_data <file> <pos> [<data>]
196240
corrupt_data () {
197241
file=$1

0 commit comments

Comments
 (0)