commit 28a5e8a2773338dd831d61aeafa5d279417be5b6
from: Stefan Sperling <stsp@stsp.name>
via: Thomas Adam <thomas@xteddy.org>
date: Tue Oct 01 23:00:28 2024 UTC

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@

commit - 3e6e13b22e6744ac9ba40feadac972d441b7e6c5
commit + 28a5e8a2773338dd831d61aeafa5d279417be5b6
blob - 325788b21bd4d9bdeadb462054ea0181f3832eb0
blob + d980f9e5fcf16e77de392818150df0d36e6e87ce
--- libexec/got-read-pack/got-read-pack.c
+++ libexec/got-read-pack/got-read-pack.c
@@ -1385,8 +1385,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)
@@ -1461,29 +1508,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