summaryrefslogtreecommitdiff
path: root/t/t9300-fast-import.sh
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2016-03-05 18:43:30 (GMT)
committerJunio C Hamano <gitster@pobox.com>2016-03-05 18:57:37 (GMT)
commitb70904306fc30857fa3638d2bce7ae0ad1251e23 (patch)
tree5ffc4cd6567e5d35a00c94508f8b79102ccc578e /t/t9300-fast-import.sh
parent326e5bc91eecf73234ead29636207bc516573e79 (diff)
downloadgit-b70904306fc30857fa3638d2bce7ae0ad1251e23.zip
git-b70904306fc30857fa3638d2bce7ae0ad1251e23.tar.gz
git-b70904306fc30857fa3638d2bce7ae0ad1251e23.tar.bz2
strbuf_getwholeline: NUL-terminate getdelim buffer on error
Commit 0cc30e0 (strbuf_getwholeline: use getdelim if it is available, 2015-04-16) tries to clean up after getdelim() returns EOF, but gets one case wrong, which can lead in some obscure cases to us reading uninitialized memory. After getdelim() returns -1, we re-initialize the strbuf only if sb->buf is NULL. The thinking was that either: 1. We fed an existing allocated buffer to getdelim(), and at most it would have realloc'd, leaving our NUL in place. 2. We didn't have a buffer to feed, so we gave getdelim() NULL; sb->buf will remain NULL, and we just want to restore the empty slopbuf. But that second case isn't quite right. getdelim() may allocate a buffer, write nothing into it, and then return EOF. The resulting strbuf rightfully has sb->len set to "0", but is missing the NUL terminator in the first byte. Most call-sites are fine with this. They see the EOF and don't bother looking at the strbuf. Or they notice that sb->len is empty, and don't look at the contents. But there's at least one case that does neither, and relies on parsing the resulting (possibly zero-length) string: fast-import. You can see this in action with the new test (though we probably only notice failure there when run with --valgrind or ASAN). We can fix this by unconditionally resetting the strbuf when we have a buffer after getdelim(). That fixes case 2 above. Case 1 is probably already fine in practice, but it does not hurt for us to re-assert our invariants (especially because we are relying on whatever getdelim() happens to do, which may vary from platform to platform). Our fix covers that case, too. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 't/t9300-fast-import.sh')
-rwxr-xr-xt/t9300-fast-import.sh4
1 files changed, 4 insertions, 0 deletions
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 14a9384..95c70ed 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -55,6 +55,10 @@ test_expect_success 'empty stream succeeds' '
git fast-import </dev/null
'
+test_expect_success 'truncated stream complains' '
+ echo "tag foo" | test_must_fail git fast-import
+'
+
test_expect_success 'A: create pack from stdin' '
test_tick &&
cat >input <<-INPUT_END &&