Skip to content

Commit 488bdf2

Browse files
jherlandgitster
authored andcommitted
Fix crasher on encountering SHA1-like non-note in notes tree
When loading a notes tree, the code primarily looks for SHA1-like paths whose total length (discounting directory separators) are 40 chars (interpreted as valid note entries) or less (interpreted as subtree entries that may in turn contain note entries when unpacked). However, there is an additional condition that must hold for valid subtree entries: They must be _tree_ objects (duh). This patch adds an appropriate test for this condition, thereby fixing the crash that occured when passing a non-tree object to the tree-walk API. The patch also adds another selftest verifying correct behaviour of non-notes in note trees. Signed-off-by: Johan Herland <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 907a0b1 commit 488bdf2

File tree

2 files changed

+174
-0
lines changed

2 files changed

+174
-0
lines changed

notes.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,8 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
331331
hashcpy(l->key_sha1, commit_sha1);
332332
hashcpy(l->val_sha1, entry.sha1);
333333
if (len < 20) {
334+
if (!S_ISDIR(entry.mode))
335+
continue; /* entry cannot be subtree */
334336
l->key_sha1[19] = (unsigned char) len;
335337
type = PTR_TYPE_SUBTREE;
336338
}

t/t3304-notes-mixed.sh

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
#!/bin/sh
2+
3+
test_description='Test notes trees that also contain non-notes'
4+
5+
. ./test-lib.sh
6+
7+
number_of_commits=100
8+
9+
start_note_commit () {
10+
test_tick &&
11+
cat <<INPUT_END
12+
commit refs/notes/commits
13+
committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
14+
data <<COMMIT
15+
notes
16+
COMMIT
17+
18+
from refs/notes/commits^0
19+
deleteall
20+
INPUT_END
21+
22+
}
23+
24+
verify_notes () {
25+
git log | grep "^ " > output &&
26+
i=$number_of_commits &&
27+
while [ $i -gt 0 ]; do
28+
echo " commit #$i" &&
29+
echo " note for commit #$i" &&
30+
i=$(($i-1));
31+
done > expect &&
32+
test_cmp expect output
33+
}
34+
35+
test_expect_success "setup: create a couple of commits" '
36+
37+
test_tick &&
38+
cat <<INPUT_END >input &&
39+
commit refs/heads/master
40+
committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
41+
data <<COMMIT
42+
commit #1
43+
COMMIT
44+
45+
M 644 inline file
46+
data <<EOF
47+
file in commit #1
48+
EOF
49+
50+
INPUT_END
51+
52+
test_tick &&
53+
cat <<INPUT_END >>input &&
54+
commit refs/heads/master
55+
committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
56+
data <<COMMIT
57+
commit #2
58+
COMMIT
59+
60+
M 644 inline file
61+
data <<EOF
62+
file in commit #2
63+
EOF
64+
65+
INPUT_END
66+
git fast-import --quiet <input
67+
'
68+
69+
test_expect_success "create a notes tree with both notes and non-notes" '
70+
71+
commit1=$(git rev-parse refs/heads/master^) &&
72+
commit2=$(git rev-parse refs/heads/master) &&
73+
test_tick &&
74+
cat <<INPUT_END >input &&
75+
commit refs/notes/commits
76+
committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
77+
data <<COMMIT
78+
notes commit #1
79+
COMMIT
80+
81+
N inline $commit1
82+
data <<EOF
83+
note for commit #1
84+
EOF
85+
86+
N inline $commit2
87+
data <<EOF
88+
note for commit #2
89+
EOF
90+
91+
INPUT_END
92+
test_tick &&
93+
cat <<INPUT_END >>input &&
94+
commit refs/notes/commits
95+
committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
96+
data <<COMMIT
97+
notes commit #2
98+
COMMIT
99+
100+
M 644 inline foobar/non-note.txt
101+
data <<EOF
102+
A non-note in a notes tree
103+
EOF
104+
105+
N inline $commit2
106+
data <<EOF
107+
edited note for commit #2
108+
EOF
109+
110+
INPUT_END
111+
test_tick &&
112+
cat <<INPUT_END >>input &&
113+
commit refs/notes/commits
114+
committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
115+
data <<COMMIT
116+
notes commit #3
117+
COMMIT
118+
119+
N inline $commit1
120+
data <<EOF
121+
edited note for commit #1
122+
EOF
123+
124+
M 644 inline deadbeef
125+
data <<EOF
126+
non-note with SHA1-like name
127+
EOF
128+
129+
M 644 inline de/adbeef
130+
data <<EOF
131+
another non-note with SHA1-like name
132+
EOF
133+
134+
INPUT_END
135+
git fast-import --quiet <input &&
136+
git config core.notesRef refs/notes/commits
137+
'
138+
139+
cat >expect <<EXPECT_END
140+
commit #2
141+
edited note for commit #2
142+
commit #1
143+
edited note for commit #1
144+
EXPECT_END
145+
146+
test_expect_success "verify contents of notes" '
147+
148+
git log | grep "^ " > actual &&
149+
test_cmp expect actual
150+
'
151+
152+
cat >expect_nn1 <<EXPECT_END
153+
A non-note in a notes tree
154+
EXPECT_END
155+
cat >expect_nn2 <<EXPECT_END
156+
non-note with SHA1-like name
157+
EXPECT_END
158+
cat >expect_nn3 <<EXPECT_END
159+
another non-note with SHA1-like name
160+
EXPECT_END
161+
162+
test_expect_success "verify contents of non-notes" '
163+
164+
git cat-file -p refs/notes/commits:foobar/non-note.txt > actual_nn1 &&
165+
test_cmp expect_nn1 actual_nn1 &&
166+
git cat-file -p refs/notes/commits:deadbeef > actual_nn2 &&
167+
test_cmp expect_nn2 actual_nn2 &&
168+
git cat-file -p refs/notes/commits:de/adbeef > actual_nn3 &&
169+
test_cmp expect_nn3 actual_nn3
170+
'
171+
172+
test_done

0 commit comments

Comments
 (0)