Merge branch 'jk/getwholeline-getdelim-empty'
authorJunio C Hamano <gitster@pobox.com>
Sun, 3 Apr 2016 17:29:34 +0000 (10:29 -0700)
committerJunio C Hamano <gitster@pobox.com>
Sun, 3 Apr 2016 17:29:34 +0000 (10:29 -0700)
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

1  2 
strbuf.c
t/t9300-fast-import.sh
diff --combined strbuf.c
index f60e2ee72ba86cbd6c66622366af4a7faf25e1a0,258650ccb35df257a7ddd37d07cfa3adc25cee3e..2c08dbb15381351ede663c1d41ebf3b6838a0c12
+++ 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 4c5f3c9d418bf6f85f1067b2cb1481bf941ff954,95c70ed43ee5daf8ba50f7c76d19be5d6c5c0745..25bb60b2814320b628d3d12af88ee1d394a94217
@@@ -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
  
        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' '
  
  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
  
  ###
  
  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' '
        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