Skip to content

Commit 4e34e20

Browse files
gitstergitster
committed
Merge branch 'dt/tree-fsck'
The codepath in "git fsck" to detect malformed tree objects has been updated not to die but keep going after detecting them. * dt/tree-fsck: fsck: handle bad trees like other errors tree-walk: be more specific about corrupt tree errors
2 parents fe252ef + 8354fa3 commit 4e34e20

File tree

5 files changed

+130
-20
lines changed

5 files changed

+130
-20
lines changed

‎fsck.c

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -347,8 +347,9 @@ static int fsck_walk_tree(struct tree *tree, void *data, struct fsck_options *op
347347
return -1;
348348

349349
name = get_object_name(options, &tree->object);
350-
init_tree_desc(&desc, tree->buffer, tree->size);
351-
while (tree_entry(&desc, &entry)) {
350+
if (init_tree_desc_gently(&desc, tree->buffer, tree->size))
351+
return -1;
352+
while (tree_entry_gently(&desc, &entry)) {
352353
struct object *obj;
353354
int result;
354355

@@ -520,7 +521,7 @@ static int verify_ordered(unsigned mode1, const char *name1, unsigned mode2, con
520521

521522
static int fsck_tree(struct tree *item, struct fsck_options *options)
522523
{
523-
int retval;
524+
int retval = 0;
524525
int has_null_sha1 = 0;
525526
int has_full_path = 0;
526527
int has_empty_name = 0;
@@ -535,7 +536,10 @@ static int fsck_tree(struct tree *item, struct fsck_options *options)
535536
unsigned o_mode;
536537
const char *o_name;
537538

538-
init_tree_desc(&desc, item->buffer, item->size);
539+
if (init_tree_desc_gently(&desc, item->buffer, item->size)) {
540+
retval += report(options, &item->object, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree");
541+
return retval;
542+
}
539543

540544
o_mode = 0;
541545
o_name = NULL;
@@ -556,7 +560,10 @@ static int fsck_tree(struct tree *item, struct fsck_options *options)
556560
is_hfs_dotgit(name) ||
557561
is_ntfs_dotgit(name));
558562
has_zero_pad |= *(char *)desc.buffer == '0';
559-
update_tree_entry(&desc);
563+
if (update_tree_entry_gently(&desc)) {
564+
retval += report(options, &item->object, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree");
565+
break;
566+
}
560567

561568
switch (mode) {
562569
/*
@@ -597,7 +604,6 @@ static int fsck_tree(struct tree *item, struct fsck_options *options)
597604
o_name = name;
598605
}
599606

600-
retval = 0;
601607
if (has_null_sha1)
602608
retval += report(options, &item->object, FSCK_MSG_NULL_SHA1, "contains entries pointing to null sha1");
603609
if (has_full_path)

‎t/t1007-hash-object.sh

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,9 +183,30 @@ for args in "-w --stdin-paths" "--stdin-paths -w"; do
183183
pop_repo
184184
done
185185

186-
test_expect_success 'corrupt tree' '
186+
test_expect_success 'too-short tree' '
187187
echo abc >malformed-tree &&
188-
test_must_fail git hash-object -t tree malformed-tree
188+
test_must_fail git hash-object -t tree malformed-tree 2>err &&
189+
test_i18ngrep "too-short tree object" err
190+
'
191+
192+
hex2oct() {
193+
perl -ne 'printf "\\%03o", hex for /../g'
194+
}
195+
196+
test_expect_success 'malformed mode in tree' '
197+
hex_sha1=$(echo foo | git hash-object --stdin -w) &&
198+
bin_sha1=$(echo $hex_sha1 | hex2oct) &&
199+
printf "9100644 \0$bin_sha1" >tree-with-malformed-mode &&
200+
test_must_fail git hash-object -t tree tree-with-malformed-mode 2>err &&
201+
test_i18ngrep "malformed mode in tree entry" err
202+
'
203+
204+
test_expect_success 'empty filename in tree' '
205+
hex_sha1=$(echo foo | git hash-object --stdin -w) &&
206+
bin_sha1=$(echo $hex_sha1 | hex2oct) &&
207+
printf "100644 \0$bin_sha1" >tree-with-empty-filename &&
208+
test_must_fail git hash-object -t tree tree-with-empty-filename 2>err &&
209+
test_i18ngrep "empty filename in tree entry" err
189210
'
190211

191212
test_expect_success 'corrupt commit' '

‎t/t1450-fsck.sh

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,7 @@ test_expect_success 'commit with NUL in header' '
188188
grep "error in commit $new.*unterminated header: NUL at offset" out
189189
'
190190

191-
test_expect_success 'malformatted tree object' '
192-
test_when_finished "git update-ref -d refs/tags/wrong" &&
191+
test_expect_success 'tree object with duplicate entries' '
193192
test_when_finished "remove_object \$T" &&
194193
T=$(
195194
GIT_INDEX_FILE=test-index &&
@@ -208,6 +207,19 @@ test_expect_success 'malformatted tree object' '
208207
grep "error in tree .*contains duplicate file entries" out
209208
'
210209

210+
test_expect_success 'unparseable tree object' '
211+
test_when_finished "git update-ref -d refs/heads/wrong" &&
212+
test_when_finished "remove_object \$tree_sha1" &&
213+
test_when_finished "remove_object \$commit_sha1" &&
214+
tree_sha1=$(printf "100644 \0twenty-bytes-of-junk" | git hash-object -t tree --stdin -w --literally) &&
215+
commit_sha1=$(git commit-tree $tree_sha1) &&
216+
git update-ref refs/heads/wrong $commit_sha1 &&
217+
test_must_fail git fsck 2>out &&
218+
test_i18ngrep "error: empty filename in tree entry" out &&
219+
test_i18ngrep "$tree_sha1" out &&
220+
test_i18ngrep ! "fatal: empty filename in tree entry" out
221+
'
222+
211223
test_expect_success 'tag pointing to nonexistent' '
212224
cat >invalid-tag <<-\EOF &&
213225
object ffffffffffffffffffffffffffffffffffffffff

‎tree-walk.c

Lines changed: 73 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,31 +22,60 @@ static const char *get_mode(const char *str, unsigned int *modep)
2222
return str;
2323
}
2424

25-
static void decode_tree_entry(struct tree_desc *desc, const char *buf, unsigned long size)
25+
static int decode_tree_entry(struct tree_desc *desc, const char *buf, unsigned long size, struct strbuf *err)
2626
{
2727
const char *path;
2828
unsigned int mode, len;
2929

30-
if (size < 24 || buf[size - 21])
31-
die("corrupt tree file");
30+
if (size < 23 || buf[size - 21]) {
31+
strbuf_addstr(err, _("too-short tree object"));
32+
return -1;
33+
}
3234

3335
path = get_mode(buf, &mode);
34-
if (!path || !*path)
35-
die("corrupt tree file");
36+
if (!path) {
37+
strbuf_addstr(err, _("malformed mode in tree entry"));
38+
return -1;
39+
}
40+
if (!*path) {
41+
strbuf_addstr(err, _("empty filename in tree entry"));
42+
return -1;
43+
}
3644
len = strlen(path) + 1;
3745

3846
/* Initialize the descriptor entry */
3947
desc->entry.path = path;
4048
desc->entry.mode = canon_mode(mode);
4149
desc->entry.oid = (const struct object_id *)(path + len);
50+
51+
return 0;
4252
}
4353

44-
void init_tree_desc(struct tree_desc *desc, const void *buffer, unsigned long size)
54+
static int init_tree_desc_internal(struct tree_desc *desc, const void *buffer, unsigned long size, struct strbuf *err)
4555
{
4656
desc->buffer = buffer;
4757
desc->size = size;
4858
if (size)
49-
decode_tree_entry(desc, buffer, size);
59+
return decode_tree_entry(desc, buffer, size, err);
60+
return 0;
61+
}
62+
63+
void init_tree_desc(struct tree_desc *desc, const void *buffer, unsigned long size)
64+
{
65+
struct strbuf err = STRBUF_INIT;
66+
if (init_tree_desc_internal(desc, buffer, size, &err))
67+
die("%s", err.buf);
68+
strbuf_release(&err);
69+
}
70+
71+
int init_tree_desc_gently(struct tree_desc *desc, const void *buffer, unsigned long size)
72+
{
73+
struct strbuf err = STRBUF_INIT;
74+
int result = init_tree_desc_internal(desc, buffer, size, &err);
75+
if (result)
76+
error("%s", err.buf);
77+
strbuf_release(&err);
78+
return result;
5079
}
5180

5281
void *fill_tree_descriptor(struct tree_desc *desc, const unsigned char *sha1)
@@ -73,21 +102,44 @@ static void entry_extract(struct tree_desc *t, struct name_entry *a)
73102
*a = t->entry;
74103
}
75104

76-
void update_tree_entry(struct tree_desc *desc)
105+
static int update_tree_entry_internal(struct tree_desc *desc, struct strbuf *err)
77106
{
78107
const void *buf = desc->buffer;
79108
const unsigned char *end = desc->entry.oid->hash + 20;
80109
unsigned long size = desc->size;
81110
unsigned long len = end - (const unsigned char *)buf;
82111

83112
if (size < len)
84-
die("corrupt tree file");
113+
die(_("too-short tree file"));
85114
buf = end;
86115
size -= len;
87116
desc->buffer = buf;
88117
desc->size = size;
89118
if (size)
90-
decode_tree_entry(desc, buf, size);
119+
return decode_tree_entry(desc, buf, size, err);
120+
return 0;
121+
}
122+
123+
void update_tree_entry(struct tree_desc *desc)
124+
{
125+
struct strbuf err = STRBUF_INIT;
126+
if (update_tree_entry_internal(desc, &err))
127+
die("%s", err.buf);
128+
strbuf_release(&err);
129+
}
130+
131+
int update_tree_entry_gently(struct tree_desc *desc)
132+
{
133+
struct strbuf err = STRBUF_INIT;
134+
if (update_tree_entry_internal(desc, &err)) {
135+
error("%s", err.buf);
136+
strbuf_release(&err);
137+
/* Stop processing this tree after error */
138+
desc->size = 0;
139+
return -1;
140+
}
141+
strbuf_release(&err);
142+
return 0;
91143
}
92144

93145
int tree_entry(struct tree_desc *desc, struct name_entry *entry)
@@ -100,6 +152,17 @@ int tree_entry(struct tree_desc *desc, struct name_entry *entry)
100152
return 1;
101153
}
102154

155+
int tree_entry_gently(struct tree_desc *desc, struct name_entry *entry)
156+
{
157+
if (!desc->size)
158+
return 0;
159+
160+
*entry = desc->entry;
161+
if (update_tree_entry_gently(desc))
162+
return 0;
163+
return 1;
164+
}
165+
103166
void setup_traverse_info(struct traverse_info *info, const char *base)
104167
{
105168
int pathlen = strlen(base);

‎tree-walk.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,22 @@ static inline int tree_entry_len(const struct name_entry *ne)
2525
return (const char *)ne->oid - ne->path - 1;
2626
}
2727

28+
/*
29+
* The _gently versions of these functions warn and return false on a
30+
* corrupt tree entry rather than dying,
31+
*/
32+
2833
void update_tree_entry(struct tree_desc *);
34+
int update_tree_entry_gently(struct tree_desc *);
2935
void init_tree_desc(struct tree_desc *desc, const void *buf, unsigned long size);
36+
int init_tree_desc_gently(struct tree_desc *desc, const void *buf, unsigned long size);
3037

3138
/*
3239
* Helper function that does both tree_entry_extract() and update_tree_entry()
3340
* and returns true for success
3441
*/
3542
int tree_entry(struct tree_desc *, struct name_entry *);
43+
int tree_entry_gently(struct tree_desc *, struct name_entry *);
3644

3745
void *fill_tree_descriptor(struct tree_desc *desc, const unsigned char *sha1);
3846

0 commit comments

Comments
 (0)