Commit Diff

Commit:
e66656e96ceb74b7356b6be8a962717a975f496c
From:
Stefan Sperling <stsp@stsp.name>
Date:
Message:
fix pack file creation in the presence of tagged tag objects If a repository contains a tag that tags another tag we could fail with a "bad object data" error while creating pack files because the packed-object-enumeration code assumed that only commits get tagged, trying to parse the tagged tag object as if it were a commit. This issue affected 'got send' and 'gotadmin pack'. There is probably more work to do here because other weird cases are known to exist in the wild, such as git.git's refs/tags/junio-gpg-pub which tags a blob. Problem with 'got send' reported by jrick@
Actions:
Patch | Tree
commit - 9c49c6c0edf6a702aa6b70f8327de7c53b2a42ad
commit + e66656e96ceb74b7356b6be8a962717a975f496c
blob - 8e713d79d7f255089269071dded73b02cc503bab
blob + 19f53cd336e1f681a848f257513519ba586232bc
--- libexec/got-read-pack/got-read-pack.c
+++ libexec/got-read-pack/got-read-pack.c
@@ -1387,8 +1387,55 @@ done:
 
 	return err;
 }
+
 
 static const struct got_error *
+resolve_tag(struct got_object **obj, struct got_object_id *id,
+    struct got_packidx *packidx, struct got_pack *pack,
+    struct got_object_cache *objcache)
+{
+	const struct got_error *err;
+	struct got_object *tagged_obj;
+	struct got_tag_object *tag;
+	uint8_t *buf;
+	size_t len;
+	int idx;
+
+	err = got_packfile_extract_object_to_mem(&buf, &len, *obj, pack);
+	if (err)
+		return err;
+
+	(*obj)->size = len;
+	err = got_object_parse_tag(&tag, buf, len, id->algo);
+	if (err)
+		goto done;
+
+	idx = got_packidx_get_object_idx(packidx, &tag->id);
+	if (idx == -1) {
+		got_object_close(*obj);
+		*obj = NULL;
+		return NULL;
+	}
+
+	tagged_obj = got_object_cache_get(objcache, &tag->id);
+	if (tagged_obj) {
+		tagged_obj->refcnt++;
+	} else {
+		err = open_object(&tagged_obj, pack, packidx,
+		    idx, &tag->id, objcache);
+		if (err)
+			goto done;
+	}
+
+	got_object_close(*obj);
+	*obj = tagged_obj;
+done:
+	got_object_tag_close(tag);
+	free(buf);
+	return err;
+}
+
+static const struct got_error *
 enumeration_request(struct imsg *imsg, struct imsgbuf *ibuf,
     struct got_pack *pack, struct got_packidx *packidx,
     struct got_object_cache *objcache)
@@ -1463,29 +1510,25 @@ enumeration_request(struct imsg *imsg, struct imsgbuf 
 		if (err)
 			goto done;
 		if (obj->type == GOT_OBJ_TYPE_TAG) {
-			struct got_tag_object *tag;
-			uint8_t *buf;
-			size_t len;
-			err = got_packfile_extract_object_to_mem(&buf,
-			    &len, obj, pack);
-			if (err)
-				goto done;
-			obj->size = len;
-			err = got_object_parse_tag(&tag, buf, len,
-			    qid->id.algo);
-			if (err) {
-				free(buf);
-				goto done;
+			while (obj->type == GOT_OBJ_TYPE_TAG) {
+				err = resolve_tag(&obj, &qid->id, packidx,
+				    pack, objcache);
+				if (err)
+					goto done;
+				if (obj == NULL)
+					break;
 			}
-			idx = got_packidx_get_object_idx(packidx, &tag->id);
-			if (idx == -1) {
+			if (obj == NULL) {
 				have_all_entries = 0;
 				break;
 			}
+			if (obj->type != GOT_OBJ_TYPE_COMMIT) {
+				got_object_qid_free(qid);
+				qid = NULL;
+				continue;
+			}
 			err = open_commit(&commit, pack, packidx, idx,
-			    &tag->id, objcache);
-			got_object_tag_close(tag);
-			free(buf);
+			    &obj->id, objcache);
 			if (err)
 				goto done;
 		} else if (obj->type == GOT_OBJ_TYPE_COMMIT) {
blob - f2acf22971de6d69864669240411b42ba43f58bc
blob + 6b8fdb74f13c5157983c8ba15522d812f4865a83
--- regress/cmdline/pack.sh
+++ regress/cmdline/pack.sh
@@ -674,7 +674,43 @@ test_pack_bad_ref() {
 	fi
 	test_done "$testroot" "$ret"
 }
+
+test_pack_tagged_tag() {
+	local testroot=`test_init pack_tagged_tag`
+
+	got tag -r $testroot/repo -m 1.0 1.0 >/dev/null
+
+	git -C $testroot/repo tag -a -m "tagging a tag" 1.0-tag 1.0 \
+		2>$testroot/stderr
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo -n "git tag failed unexpectedly:" >&2
+		cat $testroot/stderr >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
 
+	gotadmin pack -r $testroot/repo -a > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "gotadmin pack failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# try again, triggering the pack enumeration logic in got-read-pack
+	# such that it runs into a tag of a tag
+	gotadmin pack -a -r $testroot/repo -x 1.0-tag > $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "gotadmin pack failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	test_done "$testroot" "$ret"
+}
+
 test_parseargs "$@"
 run_test test_pack_all_loose_objects
 run_test test_pack_exclude
@@ -684,3 +720,4 @@ run_test test_pack_ambiguous_arg
 run_test test_pack_loose_only
 run_test test_pack_all_objects
 run_test test_pack_bad_ref
+run_test test_pack_tagged_tag