Merge branch 'js/fsck-tag-validation'
authorJunio C Hamano <gitster@pobox.com>
Mon, 22 Dec 2014 20:27:41 +0000 (12:27 -0800)
committerJunio C Hamano <gitster@pobox.com>
Mon, 22 Dec 2014 20:27:41 +0000 (12:27 -0800)
New tag object format validation added in 2.2 showed garbage
after a tagname it reported in its error message.

* js/fsck-tag-validation:
index-pack: terminate object buffers with NUL
fsck: properly bound "invalid tag name" error message

1  2 
builtin/index-pack.c
fsck.c
t/t1450-fsck.sh
diff --combined builtin/index-pack.c
index a369f5535351645f15b08efb9d592be4257cdeff,f79b04e2c47696dcdb1ca86499848669491dbffe..46321176719808dd53a38675dd0d9c558ae336dd
@@@ -112,10 -112,6 +112,10 @@@ static pthread_mutex_t deepest_delta_mu
  #define deepest_delta_lock()  lock_mutex(&deepest_delta_mutex)
  #define deepest_delta_unlock()        unlock_mutex(&deepest_delta_mutex)
  
 +static pthread_mutex_t type_cas_mutex;
 +#define type_cas_lock()               lock_mutex(&type_cas_mutex)
 +#define type_cas_unlock()     unlock_mutex(&type_cas_mutex)
 +
  static pthread_key_t key;
  
  static inline void lock_mutex(pthread_mutex_t *mutex)
@@@ -139,7 -135,6 +139,7 @@@ static void init_thread(void
        init_recursive_mutex(&read_mutex);
        pthread_mutex_init(&counter_mutex, NULL);
        pthread_mutex_init(&work_mutex, NULL);
 +      pthread_mutex_init(&type_cas_mutex, NULL);
        if (show_stat)
                pthread_mutex_init(&deepest_delta_mutex, NULL);
        pthread_key_create(&key, NULL);
@@@ -162,7 -157,6 +162,7 @@@ static void cleanup_thread(void
        pthread_mutex_destroy(&read_mutex);
        pthread_mutex_destroy(&counter_mutex);
        pthread_mutex_destroy(&work_mutex);
 +      pthread_mutex_destroy(&type_cas_mutex);
        if (show_stat)
                pthread_mutex_destroy(&deepest_delta_mutex);
        for (i = 0; i < nr_threads; i++)
  #define deepest_delta_lock()
  #define deepest_delta_unlock()
  
 +#define type_cas_lock()
 +#define type_cas_unlock()
 +
  #endif
  
  
@@@ -447,7 -438,7 +447,7 @@@ static void *unpack_entry_data(unsigne
        if (type == OBJ_BLOB && size > big_file_threshold)
                buf = fixed_buf;
        else
-               buf = xmalloc(size);
+               buf = xmallocz(size);
  
        memset(&stream, 0, sizeof(stream));
        git_inflate_init(&stream);
@@@ -552,7 -543,7 +552,7 @@@ static void *unpack_data(struct object_
        git_zstream stream;
        int status;
  
-       data = xmalloc(consume ? 64*1024 : obj->size);
+       data = xmallocz(consume ? 64*1024 : obj->size);
        inbuf = xmalloc((len < 64*1024) ? len : 64*1024);
  
        memset(&stream, 0, sizeof(stream));
@@@ -872,6 -863,7 +872,6 @@@ static void resolve_delta(struct object
  {
        void *base_data, *delta_data;
  
 -      delta_obj->real_type = base->obj->real_type;
        if (show_stat) {
                delta_obj->delta_depth = base->obj->delta_depth + 1;
                deepest_delta_lock();
        counter_unlock();
  }
  
 +/*
 + * Standard boolean compare-and-swap: atomically check whether "*type" is
 + * "want"; if so, swap in "set" and return true. Otherwise, leave it untouched
 + * and return false.
 + */
 +static int compare_and_swap_type(enum object_type *type,
 +                               enum object_type want,
 +                               enum object_type set)
 +{
 +      enum object_type old;
 +
 +      type_cas_lock();
 +      old = *type;
 +      if (old == want)
 +              *type = set;
 +      type_cas_unlock();
 +
 +      return old == want;
 +}
 +
  static struct base_data *find_unresolved_deltas_1(struct base_data *base,
                                                  struct base_data *prev_base)
  {
                struct object_entry *child = objects + deltas[base->ref_first].obj_no;
                struct base_data *result = alloc_base_data();
  
 -              assert(child->real_type == OBJ_REF_DELTA);
 +              if (!compare_and_swap_type(&child->real_type, OBJ_REF_DELTA,
 +                                         base->obj->real_type))
 +                      die("BUG: child->real_type != OBJ_REF_DELTA");
 +
                resolve_delta(child, base, result);
                if (base->ref_first == base->ref_last && base->ofs_last == -1)
                        free_base_data(base);
                struct base_data *result = alloc_base_data();
  
                assert(child->real_type == OBJ_OFS_DELTA);
 +              child->real_type = base->obj->real_type;
                resolve_delta(child, base, result);
                if (base->ofs_first == base->ofs_last)
                        free_base_data(base);
@@@ -1173,7 -1141,9 +1173,7 @@@ static void conclude_pack(int fix_thin_
                int nr_objects_initial = nr_objects;
                if (nr_unresolved <= 0)
                        die(_("confusion beyond insanity"));
 -              objects = xrealloc(objects,
 -                                 (nr_objects + nr_unresolved + 1)
 -                                 * sizeof(*objects));
 +              REALLOC_ARRAY(objects, nr_objects + nr_unresolved + 1);
                memset(objects + nr_objects + 1, 0,
                       nr_unresolved * sizeof(*objects));
                f = sha1fd(output_fd, curr_pack);
diff --combined fsck.c
index 032419463126234fa855b934bae85fb27d6b26a4,88c92e82d19c9848b560822fe7c8002e759cf655..10bcb651516e2c25979f8f5af62f87407289e8e6
--- 1/fsck.c
--- 2/fsck.c
+++ b/fsck.c
@@@ -7,7 -7,6 +7,7 @@@
  #include "tag.h"
  #include "fsck.h"
  #include "refs.h"
 +#include "utf8.h"
  
  static int fsck_walk_tree(struct tree *tree, fsck_walk_func walk, void *data)
  {
@@@ -172,9 -171,7 +172,9 @@@ static int fsck_tree(struct tree *item
                has_empty_name |= !*name;
                has_dot |= !strcmp(name, ".");
                has_dotdot |= !strcmp(name, "..");
 -              has_dotgit |= !strcmp(name, ".git");
 +              has_dotgit |= (!strcmp(name, ".git") ||
 +                             is_hfs_dotgit(name) ||
 +                             is_ntfs_dotgit(name));
                has_zero_pad |= *(char *)desc.buffer == '0';
                update_tree_entry(&desc);
  
@@@ -426,7 -423,8 +426,8 @@@ static int fsck_tag_buffer(struct tag *
        }
        strbuf_addf(&sb, "refs/tags/%.*s", (int)(eol - buffer), buffer);
        if (check_refname_format(sb.buf, 0))
-               error_func(&tag->object, FSCK_WARN, "invalid 'tag' name: %s", buffer);
+               error_func(&tag->object, FSCK_WARN, "invalid 'tag' name: %.*s",
+                          (int)(eol - buffer), buffer);
        buffer = eol + 1;
  
        if (!skip_prefix(buffer, "tagger ", &buffer))
diff --combined t/t1450-fsck.sh
index d00b70f99deffc6a4009fcea2ef387aade68d3e6,7850607783cf6d59b893b93a641e4c00d96c56db..1f04b8aa3ff925f469ffa60ce47f9d5659b9acaa
@@@ -69,7 -69,7 +69,7 @@@ test_expect_success 'object with bad sh
        git update-ref refs/heads/bogus $cmt &&
        test_when_finished "git update-ref -d refs/heads/bogus" &&
  
 -      test_might_fail git fsck 2>out &&
 +      test_must_fail git fsck 2>out &&
        cat out &&
        grep "$sha.*corrupt" out
  '
@@@ -101,7 -101,7 +101,7 @@@ test_expect_success 'email with embedde
        test_when_finished "remove_object $new" &&
        git update-ref refs/heads/bogus "$new" &&
        test_when_finished "git update-ref -d refs/heads/bogus" &&
 -      git fsck 2>out &&
 +      test_must_fail git fsck 2>out &&
        cat out &&
        grep "error in commit $new" out
  '
@@@ -113,7 -113,7 +113,7 @@@ test_expect_success 'missing < email de
        test_when_finished "remove_object $new" &&
        git update-ref refs/heads/bogus "$new" &&
        test_when_finished "git update-ref -d refs/heads/bogus" &&
 -      git fsck 2>out &&
 +      test_must_fail git fsck 2>out &&
        cat out &&
        grep "error in commit $new.* - bad name" out
  '
@@@ -125,7 -125,7 +125,7 @@@ test_expect_success 'missing email is r
        test_when_finished "remove_object $new" &&
        git update-ref refs/heads/bogus "$new" &&
        test_when_finished "git update-ref -d refs/heads/bogus" &&
 -      git fsck 2>out &&
 +      test_must_fail git fsck 2>out &&
        cat out &&
        grep "error in commit $new.* - missing email" out
  '
@@@ -137,7 -137,7 +137,7 @@@ test_expect_success '> in name is repor
        test_when_finished "remove_object $new" &&
        git update-ref refs/heads/bogus "$new" &&
        test_when_finished "git update-ref -d refs/heads/bogus" &&
 -      git fsck 2>out &&
 +      test_must_fail git fsck 2>out &&
        cat out &&
        grep "error in commit $new" out
  '
@@@ -151,31 -151,11 +151,31 @@@ test_expect_success 'integer overflow i
        test_when_finished "remove_object $new" &&
        git update-ref refs/heads/bogus "$new" &&
        test_when_finished "git update-ref -d refs/heads/bogus" &&
 -      git fsck 2>out &&
 +      test_must_fail git fsck 2>out &&
        cat out &&
        grep "error in commit $new.*integer overflow" out
  '
  
 +test_expect_success 'malformatted tree object' '
 +      test_when_finished "git update-ref -d refs/tags/wrong" &&
 +      test_when_finished "remove_object \$T" &&
 +      T=$(
 +              GIT_INDEX_FILE=test-index &&
 +              export GIT_INDEX_FILE &&
 +              rm -f test-index &&
 +              >x &&
 +              git add x &&
 +              T=$(git write-tree) &&
 +              (
 +                      git cat-file tree $T &&
 +                      git cat-file tree $T
 +              ) |
 +              git hash-object -w -t tree --stdin
 +      ) &&
 +      test_must_fail git fsck 2>out &&
 +      grep "error in tree .*contains duplicate file entries" out
 +'
 +
  test_expect_success 'tag pointing to nonexistent' '
        cat >invalid-tag <<-\EOF &&
        object ffffffffffffffffffffffffffffffffffffffff
@@@ -229,29 -209,14 +229,33 @@@ test_expect_success 'tag with incorrec
        echo $tag >.git/refs/tags/wrong &&
        test_when_finished "git update-ref -d refs/tags/wrong" &&
        git fsck --tags 2>out &&
-       grep "invalid .tag. name" out &&
-       grep "expected .tagger. line" out
+       cat >expect <<-EOF &&
+       warning in tag $tag: invalid '\''tag'\'' name: wrong name format
+       warning in tag $tag: invalid format - expected '\''tagger'\'' line
+       EOF
+       test_cmp expect out
  '
  
 +test_expect_success 'tag with bad tagger' '
 +      sha=$(git rev-parse HEAD) &&
 +      cat >wrong-tag <<-EOF &&
 +      object $sha
 +      type commit
 +      tag not-quite-wrong
 +      tagger Bad Tagger Name
 +
 +      This is an invalid tag.
 +      EOF
 +
 +      tag=$(git hash-object --literally -t tag -w --stdin <wrong-tag) &&
 +      test_when_finished "remove_object $tag" &&
 +      echo $tag >.git/refs/tags/wrong &&
 +      test_when_finished "git update-ref -d refs/tags/wrong" &&
 +      test_must_fail git fsck --tags 2>out &&
 +      grep "error in tag .*: invalid author/committer" out
 +'
 +
  test_expect_success 'cleaned up' '
        git fsck >actual 2>&1 &&
        test_cmp empty actual
@@@ -309,96 -274,35 +313,96 @@@ test_expect_success 'fsck notices submo
        )
  '
  
 -test_expect_success 'fsck notices "." and ".." in trees' '
 +while read name path pretty; do
 +      while read mode type; do
 +              : ${pretty:=$path}
 +              test_expect_success "fsck notices $pretty as $type" '
 +              (
 +                      git init $name-$type &&
 +                      cd $name-$type &&
 +                      echo content >file &&
 +                      git add file &&
 +                      git commit -m base &&
 +                      blob=$(git rev-parse :file) &&
 +                      tree=$(git rev-parse HEAD^{tree}) &&
 +                      value=$(eval "echo \$$type") &&
 +                      printf "$mode $type %s\t%s" "$value" "$path" >bad &&
 +                      bad_tree=$(git mktree <bad) &&
 +                      git fsck 2>out &&
 +                      cat out &&
 +                      grep "warning.*tree $bad_tree" out
 +              )'
 +      done <<-\EOF
 +      100644 blob
 +      040000 tree
 +      EOF
 +done <<-EOF
 +dot .
 +dotdot ..
 +dotgit .git
 +dotgit-case .GIT
 +dotgit-unicode .gI${u200c}T .gI{u200c}T
 +dotgit-case2 .Git
 +git-tilde1 git~1
 +dotgitdot .git.
 +dot-backslash-case .\\\\.GIT\\\\foobar
 +dotgit-case-backslash .git\\\\foobar
 +EOF
 +
 +# create a static test repo which is broken by omitting
 +# one particular object ($1, which is looked up via rev-parse
 +# in the new repository).
 +create_repo_missing () {
 +      rm -rf missing &&
 +      git init missing &&
        (
 -              git init dots &&
 -              cd dots &&
 -              blob=$(echo foo | git hash-object -w --stdin) &&
 -              tab=$(printf "\\t") &&
 -              git mktree <<-EOF &&
 -              100644 blob $blob$tab.
 -              100644 blob $blob$tab..
 -              EOF
 -              git fsck 2>out &&
 -              cat out &&
 -              grep "warning.*\\." out
 +              cd missing &&
 +              git commit -m one --allow-empty &&
 +              mkdir subdir &&
 +              echo content >subdir/file &&
 +              git add subdir/file &&
 +              git commit -m two &&
 +              unrelated=$(echo unrelated | git hash-object --stdin -w) &&
 +              git tag -m foo tag $unrelated &&
 +              sha1=$(git rev-parse --verify "$1") &&
 +              path=$(echo $sha1 | sed 's|..|&/|') &&
 +              rm .git/objects/$path
        )
 +}
 +
 +test_expect_success 'fsck notices missing blob' '
 +      create_repo_missing HEAD:subdir/file &&
 +      test_must_fail git -C missing fsck
  '
  
 -test_expect_success 'fsck notices ".git" in trees' '
 -      (
 -              git init dotgit &&
 -              cd dotgit &&
 -              blob=$(echo foo | git hash-object -w --stdin) &&
 -              tab=$(printf "\\t") &&
 -              git mktree <<-EOF &&
 -              100644 blob $blob$tab.git
 -              EOF
 -              git fsck 2>out &&
 -              cat out &&
 -              grep "warning.*\\.git" out
 -      )
 +test_expect_success 'fsck notices missing subtree' '
 +      create_repo_missing HEAD:subdir &&
 +      test_must_fail git -C missing fsck
 +'
 +
 +test_expect_success 'fsck notices missing root tree' '
 +      create_repo_missing HEAD^{tree} &&
 +      test_must_fail git -C missing fsck
 +'
 +
 +test_expect_success 'fsck notices missing parent' '
 +      create_repo_missing HEAD^ &&
 +      test_must_fail git -C missing fsck
 +'
 +
 +test_expect_success 'fsck notices missing tagged object' '
 +      create_repo_missing tag^{blob} &&
 +      test_must_fail git -C missing fsck
 +'
 +
 +test_expect_success 'fsck notices ref pointing to missing commit' '
 +      create_repo_missing HEAD &&
 +      test_must_fail git -C missing fsck
 +'
 +
 +test_expect_success 'fsck notices ref pointing to missing tag' '
 +      create_repo_missing tag &&
 +      test_must_fail git -C missing fsck
  '
  
  test_done