From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 22 Dec 2014 20:27:41 +0000 (-0800)
Subject: Merge branch 'js/fsck-tag-validation'
X-Git-Tag: v2.3.0-rc0~49
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/1cb4b3d380fe42a837adfb68126a312f605680f8?ds=inline;hp=-c

Merge branch 'js/fsck-tag-validation'

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
---

1cb4b3d380fe42a837adfb68126a312f605680f8
diff --combined builtin/index-pack.c
index a369f55353,f79b04e2c4..4632117671
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@@ -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++)
@@@ -185,9 -179,6 +185,9 @@@
  #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();
@@@ -897,26 -889,6 +897,26 @@@
  	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)
  {
@@@ -944,10 -916,7 +944,10 @@@
  		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);
@@@ -961,7 -930,6 +961,7 @@@
  		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 0324194631,88c92e82d1..10bcb65151
--- a/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 d00b70f99d,7850607783..1f04b8aa3f
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@@ -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