commit e66656e96ceb74b7356b6be8a962717a975f496c
from: Stefan Sperling <stsp@stsp.name>
date: Fri Sep 27 14:45:52 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 - 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