Skip to content

Commit 5c933f0

Browse files
gitstergitster
committed
Merge branch 'ab/pack-stdin-packs-fix'
Input validation of "git pack-objects --stdin-packs" has been corrected. * ab/pack-stdin-packs-fix: pack-objects: fix segfault in --stdin-packs option pack-objects tests: cover blindspots in stdin handling
2 parents e48a623 + 561fa03 commit 5c933f0

File tree

2 files changed

+124
-3
lines changed

2 files changed

+124
-3
lines changed

‎builtin/pack-objects.c

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3311,9 +3311,26 @@ static void read_packs_list_from_stdin(void)
33113311
}
33123312

33133313
/*
3314-
* First handle all of the excluded packs, marking them as kept in-core
3315-
* so that later calls to add_object_entry() discards any objects that
3316-
* are also found in excluded packs.
3314+
* Arguments we got on stdin may not even be packs. First
3315+
* check that to avoid segfaulting later on in
3316+
* e.g. pack_mtime_cmp(), excluded packs are handled below.
3317+
*
3318+
* Since we first parsed our STDIN and then sorted the input
3319+
* lines the pack we error on will be whatever line happens to
3320+
* sort first. This is lazy, it's enough that we report one
3321+
* bad case here, we don't need to report the first/last one,
3322+
* or all of them.
3323+
*/
3324+
for_each_string_list_item(item, &include_packs) {
3325+
struct packed_git *p = item->util;
3326+
if (!p)
3327+
die(_("could not find pack '%s'"), item->string);
3328+
}
3329+
3330+
/*
3331+
* Then, handle all of the excluded packs, marking them as
3332+
* kept in-core so that later calls to add_object_entry()
3333+
* discards any objects that are also found in excluded packs.
33173334
*/
33183335
for_each_string_list_item(item, &exclude_packs) {
33193336
struct packed_git *p = item->util;

‎t/t5300-pack-object.sh

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,110 @@ test_expect_success 'setup' '
3434
} >expect
3535
'
3636

37+
test_expect_success 'setup pack-object <stdin' '
38+
git init pack-object-stdin &&
39+
test_commit -C pack-object-stdin one &&
40+
test_commit -C pack-object-stdin two
41+
42+
'
43+
44+
test_expect_success 'pack-object <stdin parsing: basic [|--revs]' '
45+
cat >in <<-EOF &&
46+
$(git -C pack-object-stdin rev-parse one)
47+
EOF
48+
49+
git -C pack-object-stdin pack-objects basic-stdin <in &&
50+
idx=$(echo pack-object-stdin/basic-stdin-*.idx) &&
51+
git show-index <"$idx" >actual &&
52+
test_line_count = 1 actual &&
53+
54+
git -C pack-object-stdin pack-objects --revs basic-stdin-revs <in &&
55+
idx=$(echo pack-object-stdin/basic-stdin-revs-*.idx) &&
56+
git show-index <"$idx" >actual &&
57+
test_line_count = 3 actual
58+
'
59+
60+
test_expect_success 'pack-object <stdin parsing: [|--revs] bad line' '
61+
cat >in <<-EOF &&
62+
$(git -C pack-object-stdin rev-parse one)
63+
garbage
64+
$(git -C pack-object-stdin rev-parse two)
65+
EOF
66+
67+
sed "s/^> //g" >err.expect <<-EOF &&
68+
fatal: expected object ID, got garbage:
69+
> garbage
70+
71+
EOF
72+
test_must_fail git -C pack-object-stdin pack-objects bad-line-stdin <in 2>err.actual &&
73+
test_cmp err.expect err.actual &&
74+
75+
cat >err.expect <<-EOF &&
76+
fatal: bad revision '"'"'garbage'"'"'
77+
EOF
78+
test_must_fail git -C pack-object-stdin pack-objects --revs bad-line-stdin-revs <in 2>err.actual &&
79+
test_cmp err.expect err.actual
80+
'
81+
82+
test_expect_success 'pack-object <stdin parsing: [|--revs] empty line' '
83+
cat >in <<-EOF &&
84+
$(git -C pack-object-stdin rev-parse one)
85+
86+
$(git -C pack-object-stdin rev-parse two)
87+
EOF
88+
89+
sed -e "s/^> //g" -e "s/Z$//g" >err.expect <<-EOF &&
90+
fatal: expected object ID, got garbage:
91+
> Z
92+
93+
EOF
94+
test_must_fail git -C pack-object-stdin pack-objects empty-line-stdin <in 2>err.actual &&
95+
test_cmp err.expect err.actual &&
96+
97+
git -C pack-object-stdin pack-objects --revs empty-line-stdin-revs <in &&
98+
idx=$(echo pack-object-stdin/empty-line-stdin-revs-*.idx) &&
99+
git show-index <"$idx" >actual &&
100+
test_line_count = 3 actual
101+
'
102+
103+
test_expect_success 'pack-object <stdin parsing: [|--revs] with --stdin' '
104+
cat >in <<-EOF &&
105+
$(git -C pack-object-stdin rev-parse one)
106+
$(git -C pack-object-stdin rev-parse two)
107+
EOF
108+
109+
# There is the "--stdin-packs is incompatible with --revs"
110+
# test below, but we should make sure that the revision.c
111+
# --stdin is not picked up
112+
cat >err.expect <<-EOF &&
113+
fatal: disallowed abbreviated or ambiguous option '"'"'stdin'"'"'
114+
EOF
115+
test_must_fail git -C pack-object-stdin pack-objects stdin-with-stdin-option --stdin <in 2>err.actual &&
116+
test_cmp err.expect err.actual &&
117+
118+
test_must_fail git -C pack-object-stdin pack-objects --stdin --revs stdin-with-stdin-option-revs 2>err.actual <in &&
119+
test_cmp err.expect err.actual
120+
'
121+
122+
test_expect_success 'pack-object <stdin parsing: --stdin-packs handles garbage' '
123+
cat >in <<-EOF &&
124+
$(git -C pack-object-stdin rev-parse one)
125+
$(git -C pack-object-stdin rev-parse two)
126+
EOF
127+
128+
# That we get "two" and not "one" has to do with OID
129+
# ordering. It happens to be the same here under SHA-1 and
130+
# SHA-256. See commentary in pack-objects.c
131+
cat >err.expect <<-EOF &&
132+
fatal: could not find pack '"'"'$(git -C pack-object-stdin rev-parse two)'"'"'
133+
EOF
134+
test_must_fail git \
135+
-C pack-object-stdin \
136+
pack-objects stdin-with-stdin-option --stdin-packs \
137+
<in 2>err.actual &&
138+
test_cmp err.expect err.actual
139+
'
140+
37141
# usage: check_deltas <stderr_from_pack_objects> <cmp_op> <nr_deltas>
38142
# e.g.: check_deltas stderr -gt 0
39143
check_deltas() {

0 commit comments

Comments
 (0)