@matheusbsbs / matheus-bagliano / commits / bc2fed496b

bundle: use a strbuf to scan the log for boundary commits

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>

Thomas Rast committed Feb 22, 2012 at 20:34 UTC bc2fed496baa54ae99dede7da23dec938adbf0eb

2 files changed +24 -7

bundle.c

+8 -7

@@ -234,7 +234,7 @@ int create_bundle(struct bundle_header *header, const char *path,

234 const char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));

235 const char **argv_pack = xmalloc(6 * sizeof(const char *));

236 int i, ref_count = 0;

237 - char buffer[1024];

237 + struct strbuf buf = STRBUF_INIT;

238 struct rev_info revs;

239 struct child_process rls;

240 FILE *rls_fout;

@@ -266,20 +266,21 @@ int create_bundle(struct bundle_header *header, const char *path,

266 if (start_command(&rls))

267 return -1;

268 rls_fout = xfdopen(rls.out, "r");

269 - while (fgets(buffer, sizeof(buffer), rls_fout)) {

269 + while (strbuf_getwholeline(&buf, rls_fout, '\n') != EOF) {

270 unsigned char sha1[20];

271 - if (buffer[0] == '-') {

272 - write_or_die(bundle_fd, buffer, strlen(buffer));

273 - if (!get_sha1_hex(buffer + 1, sha1)) {

271 + if (buf.len > 0 && buf.buf[0] == '-') {

272 + write_or_die(bundle_fd, buf.buf, buf.len);

273 + if (!get_sha1_hex(buf.buf + 1, sha1)) {

274 struct object *object = parse_object(sha1);

275 object->flags |= UNINTERESTING;

276 - add_pending_object(&revs, object, buffer);

276 + add_pending_object(&revs, object, buf.buf);

277 }

278 - } else if (!get_sha1_hex(buffer, sha1)) {

278 + } else if (!get_sha1_hex(buf.buf, sha1)) {

279 struct object *object = parse_object(sha1);

280 object->flags |= SHOWN;

281 }

282 }

283 + strbuf_release(&buf);

284 fclose(rls_fout);

285 if (finish_command(&rls))

286 return error("rev-list died");

t/t5704-bundle.sh

+16

@@ -59,4 +59,20 @@ test_expect_success 'empty bundle file is rejected' '

60 '

62 +# This triggers a bug in older versions where the resulting line (with

63 +# --pretty=oneline) was longer than a 1024-char buffer.

64 +test_expect_success 'ridiculously long subject in boundary' '

65 + : >file4 &&

66 + test_tick &&

67 + git add file4 &&

68 + printf "%01200d\n" 0 | git commit -F - &&

69 + test_commit fifth &&

70 + git bundle create long-subject-bundle.bdl HEAD^..HEAD &&

71 + git bundle list-heads long-subject-bundle.bdl >heads &&

72 + test -s heads &&

73 + git fetch long-subject-bundle.bdl &&

74 + sed -n "/^-/{p;q}" long-subject-bundle.bdl >boundary &&

75 + grep "^-$_x40 " boundary

76 +'

77 +

78 test_done