Pass over a notes ref that names no tree

A pusher can point refs/notes/commits or a signature notes ref at any
object, and git dies loading one that is no tree, which ended every page
of the repository, the dumb transport included.

5 files changed, 56 insertions, 4 deletions

diff --git a/source/cgit.c b/source/cgit.cindex 2ef4cf1..8a3d8c1 100644--- a/source/cgit.c+++ b/source/cgit.c@@ -1098,9 +1098,20 @@ static void prepare_repo_env(int *nongit, int *err) 	setup_git_directory_gently(the_repository, nongit); 	*err = errno; 	// Notes come out of the object store, which a repository that failed-	// to open has none of.-	if (!*nongit)-		load_display_notes(NULL);+	// to open has none of, and git dies loading a notes ref that names+	// no tree, so such a ref is passed over.+	if (!*nongit) {+		struct display_notes_opt opt;+		char *ref = default_notes_ref(the_repository);++		// The notes are always set up, since the commit and log pages+		// ask for them, only with no ref when the default names no tree.+		init_display_notes(&opt);+		opt.use_default_notes = cgit_notes_ref_usable(ref);+		load_display_notes(&opt);+		release_display_notes(&opt);+		free(ref);+	} }  /*
diff --git a/source/cgit.c b/source/cgit.c
index 2ef4cf1..8a3d8c1 100644
--- a/source/cgit.c
+++ b/source/cgit.c
@@ -1098,9 +1098,20 @@ static void prepare_repo_env(int *nongit, int *err)
 	setup_git_directory_gently(the_repository, nongit);
 	*err = errno;
 	// Notes come out of the object store, which a repository that failed
-	// to open has none of.
-	if (!*nongit)
-		load_display_notes(NULL);
+	// to open has none of, and git dies loading a notes ref that names
+	// no tree, so such a ref is passed over.
+	if (!*nongit) {
+		struct display_notes_opt opt;
+		char *ref = default_notes_ref(the_repository);
+
+		// The notes are always set up, since the commit and log pages
+		// ask for them, only with no ref when the default names no tree.
+		init_display_notes(&opt);
+		opt.use_default_notes = cgit_notes_ref_usable(ref);
+		load_display_notes(&opt);
+		release_display_notes(&opt);
+		free(ref);
+	}
 }
 
 /*
diff --git a/source/shared.c b/source/shared.cindex 0bd9fe3..a20773b 100644--- a/source/shared.c+++ b/source/shared.c@@ -756,3 +756,12 @@ int cgit_readme_is_tracked(const char *readme)  	return colon && colon[1]; }++int cgit_notes_ref_usable(const char *notes_ref)+{+	struct object_id oid;++	if (repo_get_oid_treeish(the_repository, notes_ref, &oid))+		return 0;+	return !!parse_tree_indirect(&oid);+}
diff --git a/source/shared.c b/source/shared.c
index 0bd9fe3..a20773b 100644
--- a/source/shared.c
+++ b/source/shared.c
@@ -756,3 +756,12 @@ int cgit_readme_is_tracked(const char *readme)
 
 	return colon && colon[1];
 }
+
+int cgit_notes_ref_usable(const char *notes_ref)
+{
+	struct object_id oid;
+
+	if (repo_get_oid_treeish(the_repository, notes_ref, &oid))
+		return 0;
+	return !!parse_tree_indirect(&oid);
+}
diff --git a/source/shared.h b/source/shared.hindex 10aa659..e22eb57 100644--- a/source/shared.h+++ b/source/shared.h@@ -133,4 +133,11 @@ extern int cgit_readme_is_tracked(const char *readme);  */ extern int cgit_valid_rev(const char *rev); +/*+ * Whether notes_ref names a tree, which is what a notes tree can be loaded+ * from. A ref pointing at anything else makes git die on loading it, and a+ * pusher can point one anywhere.+ */+extern int cgit_notes_ref_usable(const char *notes_ref);+ #endif // CGIT_SHARED_H
diff --git a/source/shared.h b/source/shared.h
index 10aa659..e22eb57 100644
--- a/source/shared.h
+++ b/source/shared.h
@@ -133,4 +133,11 @@ extern int cgit_readme_is_tracked(const char *readme);
  */
 extern int cgit_valid_rev(const char *rev);
 
+/*
+ * Whether notes_ref names a tree, which is what a notes tree can be loaded
+ * from. A ref pointing at anything else makes git die on loading it, and a
+ * pusher can point one anywhere.
+ */
+extern int cgit_notes_ref_usable(const char *notes_ref);
+
 #endif // CGIT_SHARED_H
diff --git a/source/ui-snapshot.c b/source/ui-snapshot.cindex 32d40f8..6a059a0 100644--- a/source/ui-snapshot.c+++ b/source/ui-snapshot.c@@ -311,7 +311,12 @@ const struct object_id *cgit_snapshot_get_sig(const char *ref, const struct cgit 		// dropped, so plain tar is refs/notes/signatures/tar. 		strbuf_addf(&notes_ref, "refs/notes/signatures/%s", f->suffix + 1); -		init_notes(tree, notes_ref.buf, combine_notes_ignore, 0);+		// A ref that names no tree is passed over, since git would die+		// loading it, and a signature it cannot hold is just absent.+		if (cgit_notes_ref_usable(notes_ref.buf))+			init_notes(tree, notes_ref.buf, combine_notes_ignore, 0);+		else+			init_notes(tree, notes_ref.buf, combine_notes_ignore, NOTES_INIT_EMPTY); 		strbuf_release(&notes_ref); 	} 
diff --git a/source/ui-snapshot.c b/source/ui-snapshot.c
index 32d40f8..6a059a0 100644
--- a/source/ui-snapshot.c
+++ b/source/ui-snapshot.c
@@ -311,7 +311,12 @@ const struct object_id *cgit_snapshot_get_sig(const char *ref, const struct cgit
 		// dropped, so plain tar is refs/notes/signatures/tar.
 		strbuf_addf(&notes_ref, "refs/notes/signatures/%s", f->suffix + 1);
 
-		init_notes(tree, notes_ref.buf, combine_notes_ignore, 0);
+		// A ref that names no tree is passed over, since git would die
+		// loading it, and a signature it cannot hold is just absent.
+		if (cgit_notes_ref_usable(notes_ref.buf))
+			init_notes(tree, notes_ref.buf, combine_notes_ignore, 0);
+		else
+			init_notes(tree, notes_ref.buf, combine_notes_ignore, NOTES_INIT_EMPTY);
 		strbuf_release(&notes_ref);
 	}
 
diff --git a/tests/t0303-robustness.sh b/tests/t0303-robustness.shindex c9e4994..9969687 100755--- a/tests/t0303-robustness.sh+++ b/tests/t0303-robustness.sh@@ -835,4 +835,24 @@ test_expect_success 'odd dates and zones in pushed objects render' ' 	grep "1970-01-01" tmp ' +# A notes ref a pusher pointed at something other than a tree made git die on+# loading it, which took every page of the repository down with it.+test_expect_success 'a notes ref that names no tree is passed over' '+	blob=$(echo not-a-tree | git -C repos/dates hash-object -w --stdin) &&+	git -C repos/dates update-ref refs/notes/commits $blob &&+	git -C repos/dates update-ref refs/notes/signatures/tar $blob &&+	{+		echo "snapshots=tar" &&+		cat datesrc+	} >notesrc &&+	for q in "url=dates/" "url=dates/log/" "url=dates/refs/" "url=dates/tree/" \+		"url=dates/snapshot/dates-base.tar.asc"+	do+		CGIT_CONFIG="$PWD/notesrc" QUERY_STRING="$q" cgit >tmp 2>err &&+		! grep "Unable to complete" tmp &&+		! grep "notes tree" err || return 1+	done &&+	grep "^Status: 404" tmp+'+ test_done
diff --git a/tests/t0303-robustness.sh b/tests/t0303-robustness.sh
index c9e4994..9969687 100755
--- a/tests/t0303-robustness.sh
+++ b/tests/t0303-robustness.sh
@@ -835,4 +835,24 @@ test_expect_success 'odd dates and zones in pushed objects render' '
 	grep "1970-01-01" tmp
 '
 
+# A notes ref a pusher pointed at something other than a tree made git die on
+# loading it, which took every page of the repository down with it.
+test_expect_success 'a notes ref that names no tree is passed over' '
+	blob=$(echo not-a-tree | git -C repos/dates hash-object -w --stdin) &&
+	git -C repos/dates update-ref refs/notes/commits $blob &&
+	git -C repos/dates update-ref refs/notes/signatures/tar $blob &&
+	{
+		echo "snapshots=tar" &&
+		cat datesrc
+	} >notesrc &&
+	for q in "url=dates/" "url=dates/log/" "url=dates/refs/" "url=dates/tree/" \
+		"url=dates/snapshot/dates-base.tar.asc"
+	do
+		CGIT_CONFIG="$PWD/notesrc" QUERY_STRING="$q" cgit >tmp 2>err &&
+		! grep "Unable to complete" tmp &&
+		! grep "notes tree" err || return 1
+	done &&
+	grep "^Status: 404" tmp
+'
+
 test_done