From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 15 Apr 2016 01:57:46 +0000 (-0700)
Subject: Merge branch 'jk/getwholeline-getdelim-empty' into maint
X-Git-Tag: v2.8.2~16
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/f55f97cb3307f49e6b15d9f0145b6d3f00b22ff7?hp=-c

Merge branch 'jk/getwholeline-getdelim-empty' into maint

strbuf_getwholeline() did not NUL-terminate the buffer on certain
corner cases in its error codepath.

* jk/getwholeline-getdelim-empty:
strbuf_getwholeline: NUL-terminate getdelim buffer on error
---

f55f97cb3307f49e6b15d9f0145b6d3f00b22ff7
diff --combined strbuf.c
index f60e2ee72b,258650ccb3..2c08dbb153
--- a/strbuf.c
+++ b/strbuf.c
@@@ -384,17 -384,6 +384,17 @@@ ssize_t strbuf_read(struct strbuf *sb, 
  	return sb->len - oldlen;
  }
  
 +ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint)
 +{
 +	ssize_t cnt;
 +
 +	strbuf_grow(sb, hint ? hint : 8192);
 +	cnt = xread(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);
 +	if (cnt > 0)
 +		strbuf_setlen(sb, sb->len + cnt);
 +	return cnt;
 +}
 +
  #define STRBUF_MAXLINK (2*PATH_MAX)
  
  int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)
@@@ -481,9 -470,15 +481,15 @@@ int strbuf_getwholeline(struct strbuf *
  	if (errno == ENOMEM)
  		die("Out of memory, getdelim failed");
  
- 	/* Restore slopbuf that we moved out of the way before */
+ 	/*
+ 	 * Restore strbuf invariants; if getdelim left us with a NULL pointer,
+ 	 * we can just re-init, but otherwise we should make sure that our
+ 	 * length is empty, and that the result is NUL-terminated.
+ 	 */
  	if (!sb->buf)
  		strbuf_init(sb, 0);
+ 	else
+ 		strbuf_reset(sb);
  	return EOF;
  }
  #else
@@@ -512,37 -507,15 +518,37 @@@ int strbuf_getwholeline(struct strbuf *
  }
  #endif
  
 -int strbuf_getline(struct strbuf *sb, FILE *fp, int term)
 +static int strbuf_getdelim(struct strbuf *sb, FILE *fp, int term)
  {
  	if (strbuf_getwholeline(sb, fp, term))
  		return EOF;
 -	if (sb->buf[sb->len-1] == term)
 -		strbuf_setlen(sb, sb->len-1);
 +	if (sb->buf[sb->len - 1] == term)
 +		strbuf_setlen(sb, sb->len - 1);
  	return 0;
  }
  
 +int strbuf_getline(struct strbuf *sb, FILE *fp)
 +{
 +	if (strbuf_getwholeline(sb, fp, '\n'))
 +		return EOF;
 +	if (sb->buf[sb->len - 1] == '\n') {
 +		strbuf_setlen(sb, sb->len - 1);
 +		if (sb->len && sb->buf[sb->len - 1] == '\r')
 +			strbuf_setlen(sb, sb->len - 1);
 +	}
 +	return 0;
 +}
 +
 +int strbuf_getline_lf(struct strbuf *sb, FILE *fp)
 +{
 +	return strbuf_getdelim(sb, fp, '\n');
 +}
 +
 +int strbuf_getline_nul(struct strbuf *sb, FILE *fp)
 +{
 +	return strbuf_getdelim(sb, fp, '\0');
 +}
 +
  int strbuf_getwholeline_fd(struct strbuf *sb, int fd, int term)
  {
  	strbuf_reset(sb);
@@@ -718,7 -691,7 +724,7 @@@ char *xstrdup_tolower(const char *strin
  	size_t len, i;
  
  	len = strlen(string);
 -	result = xmalloc(len + 1);
 +	result = xmallocz(len);
  	for (i = 0; i < len; i++)
  		result[i] = tolower(string[i]);
  	result[i] = '\0';
diff --combined t/t9300-fast-import.sh
index 4c5f3c9d41,95c70ed43e..25bb60b281
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@@ -55,6 -55,10 +55,10 @@@ test_expect_success 'empty stream succe
  	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 &&
@@@ -171,10 -175,10 +175,10 @@@ test_expect_success 'A: verify tag/seri
  
  test_expect_success 'A: verify marks output' '
  	cat >expect <<-EOF &&
 -	:2 `git rev-parse --verify master:file2`
 -	:3 `git rev-parse --verify master:file3`
 -	:4 `git rev-parse --verify master:file4`
 -	:5 `git rev-parse --verify master^0`
 +	:2 $(git rev-parse --verify master:file2)
 +	:3 $(git rev-parse --verify master:file3)
 +	:4 $(git rev-parse --verify master:file4)
 +	:5 $(git rev-parse --verify master^0)
  	EOF
  	test_cmp expect marks.out
  '
@@@ -264,8 -268,8 +268,8 @@@ test_expect_success 'A: verify diff' 
  	EOF
  	git diff-tree -M -r master verify--import-marks >actual &&
  	compare_diff_raw expect actual &&
 -	test `git rev-parse --verify master:file2` \
 -	    = `git rev-parse --verify verify--import-marks:copy-of-file2`
 +	test $(git rev-parse --verify master:file2) \
 +	    = $(git rev-parse --verify verify--import-marks:copy-of-file2)
  '
  
  test_expect_success 'A: export marks with large values' '
@@@ -364,7 -368,7 +368,7 @@@ test_expect_success 'B: accept branch n
  		git prune" &&
  	git fast-import <input &&
  	test -f .git/TEMP_TAG &&
 -	test `git rev-parse master` = `git rev-parse TEMP_TAG^`
 +	test $(git rev-parse master) = $(git rev-parse TEMP_TAG^)
  '
  
  test_expect_success 'B: accept empty committer' '
@@@ -473,8 -477,8 +477,8 @@@ test_expect_success 'B: fail on invali
  ###
  
  test_expect_success 'C: incremental import create pack from stdin' '
 -	newf=`echo hi newf | git hash-object -w --stdin` &&
 -	oldf=`git rev-parse --verify master:file2` &&
 +	newf=$(echo hi newf | git hash-object -w --stdin) &&
 +	oldf=$(git rev-parse --verify master:file2) &&
  	test_tick &&
  	cat >input <<-INPUT_END &&
  	commit refs/heads/branch
@@@ -499,13 -503,13 +503,13 @@@ test_expect_success 'C: verify pack' 
  '
  
  test_expect_success 'C: validate reuse existing blob' '
 -	test $newf = `git rev-parse --verify branch:file2/newf` &&
 -	test $oldf = `git rev-parse --verify branch:file2/oldf`
 +	test $newf = $(git rev-parse --verify branch:file2/newf) &&
 +	test $oldf = $(git rev-parse --verify branch:file2/oldf)
  '
  
  test_expect_success 'C: verify commit' '
  	cat >expect <<-EOF &&
 -	parent `git rev-parse --verify master^0`
 +	parent $(git rev-parse --verify master^0)
  	author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
  	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
  
@@@ -624,7 -628,7 +628,7 @@@ test_expect_success 'E: verify commit' 
  ###
  
  test_expect_success 'F: non-fast-forward update skips' '
 -	old_branch=`git rev-parse --verify branch^0` &&
 +	old_branch=$(git rev-parse --verify branch^0) &&
  	test_tick &&
  	cat >input <<-INPUT_END &&
  	commit refs/heads/branch
@@@ -642,7 -646,7 +646,7 @@@
  
  	test_must_fail git fast-import <input &&
  	# branch must remain unaffected
 -	test $old_branch = `git rev-parse --verify branch^0`
 +	test $old_branch = $(git rev-parse --verify branch^0)
  '
  
  test_expect_success 'F: verify pack' '
@@@ -651,8 -655,8 +655,8 @@@
  
  test_expect_success 'F: verify other commit' '
  	cat >expect <<-EOF &&
 -	tree `git rev-parse branch~1^{tree}`
 -	parent `git rev-parse branch~1`
 +	tree $(git rev-parse branch~1^{tree})
 +	parent $(git rev-parse branch~1)
  	author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
  	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
  
@@@ -667,7 -671,7 +671,7 @@@
  ###
  
  test_expect_success 'G: non-fast-forward update forced' '
 -	old_branch=`git rev-parse --verify branch^0` &&
 +	old_branch=$(git rev-parse --verify branch^0) &&
  	test_tick &&
  	cat >input <<-INPUT_END &&
  	commit refs/heads/branch
@@@ -687,8 -691,8 +691,8 @@@ test_expect_success 'G: verify pack' 
  '
  
  test_expect_success 'G: branch changed, but logged' '
 -	test $old_branch != `git rev-parse --verify branch^0` &&
 -	test $old_branch = `git rev-parse --verify branch@{1}`
 +	test $old_branch != $(git rev-parse --verify branch^0) &&
 +	test $old_branch = $(git rev-parse --verify branch@{1})
  '
  
  ###
@@@ -763,7 -767,7 +767,7 @@@ test_expect_success 'I: export-pack-edg
  
  test_expect_success 'I: verify edge list' '
  	cat >expect <<-EOF &&
 -	.git/objects/pack/pack-.pack: `git rev-parse --verify export-boundary`
 +	.git/objects/pack/pack-.pack: $(git rev-parse --verify export-boundary)
  	EOF
  	sed -e s/pack-.*pack/pack-.pack/ edges.list >actual &&
  	test_cmp expect actual
@@@ -795,8 -799,8 +799,8 @@@ test_expect_success 'J: reset existing 
  	git fast-import <input
  '
  test_expect_success 'J: branch has 1 commit, empty tree' '
 -	test 1 = `git rev-list J | wc -l` &&
 -	test 0 = `git ls-tree J | wc -l`
 +	test 1 = $(git rev-list J | wc -l) &&
 +	test 0 = $(git ls-tree J | wc -l)
  '
  
  test_expect_success 'J: tag must fail on empty branch' '
@@@ -838,8 -842,8 +842,8 @@@ test_expect_success 'K: reinit branch w
  	git fast-import <input
  '
  test_expect_success 'K: verify K^1 = branch^1' '
 -	test `git rev-parse --verify branch^1` \
 -		= `git rev-parse --verify K^1`
 +	test $(git rev-parse --verify branch^1) \
 +		= $(git rev-parse --verify K^1)
  '
  
  ###
@@@ -929,7 -933,7 +933,7 @@@ test_expect_success 'L: nested tree cop
  	git ls-tree L2 g/b/ >tmp &&
  	cat tmp | cut -f 2 >actual &&
  	test_cmp expect actual &&
 -	git fsck `git rev-parse L2`
 +	git fsck $(git rev-parse L2)
  '
  
  ###
@@@ -1106,7 -1110,7 +1110,7 @@@ test_expect_success 'N: copy dirty subd
  	INPUT_END
  
  	git fast-import <input &&
 -	test `git rev-parse N2^{tree}` = `git rev-parse N3^{tree}`
 +	test $(git rev-parse N2^{tree}) = $(git rev-parse N3^{tree})
  '
  
  test_expect_success 'N: copy directory by id' '
@@@ -1503,7 -1507,7 +1507,7 @@@ test_expect_success 'O: comments are al
  	INPUT_END
  
  	git fast-import <input &&
 -	test `git rev-parse N3` = `git rev-parse O1`
 +	test $(git rev-parse N3) = $(git rev-parse O1)
  '
  
  test_expect_success 'O: blank lines not necessary after data commands' '
@@@ -1524,7 -1528,7 +1528,7 @@@
  	INPUT_END
  
  	git fast-import <input &&
 -	test `git rev-parse N3` = `git rev-parse O2`
 +	test $(git rev-parse N3) = $(git rev-parse O2)
  '
  
  test_expect_success 'O: repack before next test' '
@@@ -1570,8 -1574,8 +1574,8 @@@ test_expect_success 'O: blank lines no
  	INPUT_END
  
  	git fast-import <input &&
 -	test 8 = `find .git/objects/pack -type f | wc -l` &&
 -	test `git rev-parse refs/tags/O3-2nd` = `git rev-parse O3^` &&
 +	test 8 = $(find .git/objects/pack -type f | wc -l) &&
 +	test $(git rev-parse refs/tags/O3-2nd) = $(git rev-parse O3^) &&
  	git log --reverse --pretty=oneline O3 | sed s/^.*z// >actual &&
  	test_cmp expect actual
  '
@@@ -1631,7 -1635,7 +1635,7 @@@ test_expect_success 'P: superproject & 
  	data <<DATAEND
  	[submodule "sub"]
  		path = sub
 -		url = "`pwd`/sub"
 +		url = "$(pwd)/sub"
  	DATAEND
  
  	commit refs/heads/subuse1
@@@ -1691,7 -1695,7 +1695,7 @@@ test_expect_success 'P: verbatim SHA gi
  	data <<DATAEND
  	[submodule "sub"]
  		path = sub
 -		url = "`pwd`/sub"
 +		url = "$(pwd)/sub"
  	DATAEND
  
  	commit refs/heads/subuse2
@@@ -1978,7 -1982,7 +1982,7 @@@ test_expect_success 'Q: verify first no
  
  test_expect_success 'Q: verify second notes commit' '
  	cat >expect <<-EOF &&
 -	parent `git rev-parse --verify refs/notes/foobar~2`
 +	parent $(git rev-parse --verify refs/notes/foobar~2)
  	author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
  	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
  
@@@ -2045,7 -2049,7 +2049,7 @@@ test_expect_success 'Q: verify third no
  
  test_expect_success 'Q: verify fourth notes commit' '
  	cat >expect <<-EOF &&
 -	parent `git rev-parse --verify refs/notes/foobar^`
 +	parent $(git rev-parse --verify refs/notes/foobar^)
  	author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
  	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE