Skip to content

Commit

Permalink
bundle: use a strbuf to scan the log for boundary commits
Browse files Browse the repository at this point in the history
The first part of the bundle header contains the boundary commits, and
could be approximated by

  # v2 git bundle
  $(git rev-list --pretty=oneline --boundary <ARGS> | grep ^-)

git-bundle actually spawns exactly this rev-list invocation, and does
the grepping internally.

There was a subtle bug in the latter step: it used fgets() with a
1024-byte buffer.  If the user has sufficiently long subjects (e.g.,
by not adhering to the git oneline-subject convention in the first
place), the 'oneline' format can easily overflow the buffer.  fgets()
then returns the rest of the line in the next call(s).  If one of
these remaining parts started with '-', git-bundle would mistakenly
insert it into the bundle thinking it was a boundary commit.

Fix it by using strbuf_getwholeline() instead, which handles arbitrary
line lengths correctly.

Note that on the receiving side in parse_bundle_header() we were
already using strbuf_getwholeline_fd(), so that part is safe.

Reported-by: Jannis Pohlmann <jannis.pohlmann@codethink.co.uk>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Thomas Rast authored and Junio C Hamano committed Feb 23, 2012
1 parent 5e8617f commit bc2fed4
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 7 deletions.
15 changes: 8 additions & 7 deletions bundle.c
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ int create_bundle(struct bundle_header *header, const char *path,
const char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));
const char **argv_pack = xmalloc(6 * sizeof(const char *));
int i, ref_count = 0;
char buffer[1024];
struct strbuf buf = STRBUF_INIT;
struct rev_info revs;
struct child_process rls;
FILE *rls_fout;
Expand Down Expand Up @@ -266,20 +266,21 @@ int create_bundle(struct bundle_header *header, const char *path,
if (start_command(&rls))
return -1;
rls_fout = xfdopen(rls.out, "r");
while (fgets(buffer, sizeof(buffer), rls_fout)) {
while (strbuf_getwholeline(&buf, rls_fout, '\n') != EOF) {
unsigned char sha1[20];
if (buffer[0] == '-') {
write_or_die(bundle_fd, buffer, strlen(buffer));
if (!get_sha1_hex(buffer + 1, sha1)) {
if (buf.len > 0 && buf.buf[0] == '-') {
write_or_die(bundle_fd, buf.buf, buf.len);
if (!get_sha1_hex(buf.buf + 1, sha1)) {
struct object *object = parse_object(sha1);
object->flags |= UNINTERESTING;
add_pending_object(&revs, object, buffer);
add_pending_object(&revs, object, buf.buf);
}
} else if (!get_sha1_hex(buffer, sha1)) {
} else if (!get_sha1_hex(buf.buf, sha1)) {
struct object *object = parse_object(sha1);
object->flags |= SHOWN;
}
}
strbuf_release(&buf);
fclose(rls_fout);
if (finish_command(&rls))
return error("rev-list died");
Expand Down
16 changes: 16 additions & 0 deletions t/t5704-bundle.sh
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,20 @@ test_expect_success 'empty bundle file is rejected' '
'

# This triggers a bug in older versions where the resulting line (with
# --pretty=oneline) was longer than a 1024-char buffer.
test_expect_success 'ridiculously long subject in boundary' '
: >file4 &&
test_tick &&
git add file4 &&
printf "%01200d\n" 0 | git commit -F - &&
test_commit fifth &&
git bundle create long-subject-bundle.bdl HEAD^..HEAD &&
git bundle list-heads long-subject-bundle.bdl >heads &&
test -s heads &&
git fetch long-subject-bundle.bdl &&
sed -n "/^-/{p;q}" long-subject-bundle.bdl >boundary &&
grep "^-$_x40 " boundary
'

test_done

0 comments on commit bc2fed4

Please sign in to comment.