9 files changed, 66 insertions, 88 deletions

diff --git a/source/ui-blame.c b/source/ui-blame.cindex 2e281cf..28fd16f 100644--- a/source/ui-blame.c+++ b/source/ui-blame.c@@ -388,15 +388,9 @@ void cgit_print_blame(void) 	if (!rev) 		rev = ctx.qry.head; -	if (repo_get_oid(the_repository, rev, &oid)) {-		cgit_print_error_page(404, "Not Found", "Bad object id: %s", rev);+	commit = cgit_get_commit(rev, &oid);+	if (!commit) 		return;-	}-	commit = lookup_commit_reference(the_repository, &oid);-	if (!commit || repo_parse_commit(the_repository, commit)) {-		cgit_print_error_page(404, "Not Found", "Not a commit: %s", rev);-		return;-	}  	walk.rev = rev; 	walk.match_baselen = path_items.match ? basedir_len(path_items.match) : -1;
diff --git a/source/ui-blame.c b/source/ui-blame.c
index 2e281cf..28fd16f 100644
--- a/source/ui-blame.c
+++ b/source/ui-blame.c
@@ -388,15 +388,9 @@ void cgit_print_blame(void)
 	if (!rev)
 		rev = ctx.qry.head;
 
-	if (repo_get_oid(the_repository, rev, &oid)) {
-		cgit_print_error_page(404, "Not Found", "Bad object id: %s", rev);
+	commit = cgit_get_commit(rev, &oid);
+	if (!commit)
 		return;
-	}
-	commit = lookup_commit_reference(the_repository, &oid);
-	if (!commit || repo_parse_commit(the_repository, commit)) {
-		cgit_print_error_page(404, "Not Found", "Not a commit: %s", rev);
-		return;
-	}
 
 	walk.rev = rev;
 	walk.match_baselen = path_items.match ? basedir_len(path_items.match) : -1;
diff --git a/source/ui-commit.c b/source/ui-commit.cindex 9b2f8b9..c290a2c 100644--- a/source/ui-commit.c+++ b/source/ui-commit.c@@ -251,15 +251,9 @@ void cgit_print_commit(char *hex, const char *prefix) 	if (!hex) 		hex = ctx.qry.head; -	if (repo_get_oid(the_repository, hex, &oid)) {-		cgit_print_error_page(404, "Not Found", "Bad object id: %s", hex);+	commit = cgit_get_commit(hex, &oid);+	if (!commit) 		return;-	}-	commit = lookup_commit_reference(the_repository, &oid);-	if (!commit) {-		cgit_print_error_page(404, "Not Found", "Not a commit: %s", hex);-		return;-	} 	info = cgit_parse_commit(commit);  	format_display_notes(&commit->object.oid, &notes, PAGE_ENCODING, 1);
diff --git a/source/ui-commit.c b/source/ui-commit.c
index 9b2f8b9..c290a2c 100644
--- a/source/ui-commit.c
+++ b/source/ui-commit.c
@@ -251,15 +251,9 @@ void cgit_print_commit(char *hex, const char *prefix)
 	if (!hex)
 		hex = ctx.qry.head;
 
-	if (repo_get_oid(the_repository, hex, &oid)) {
-		cgit_print_error_page(404, "Not Found", "Bad object id: %s", hex);
+	commit = cgit_get_commit(hex, &oid);
+	if (!commit)
 		return;
-	}
-	commit = lookup_commit_reference(the_repository, &oid);
-	if (!commit) {
-		cgit_print_error_page(404, "Not Found", "Not a commit: %s", hex);
-		return;
-	}
 	info = cgit_parse_commit(commit);
 
 	format_display_notes(&commit->object.oid, &notes, PAGE_ENCODING, 1);
diff --git a/source/ui-diff.c b/source/ui-diff.cindex 736cdaf..51df73d 100644--- a/source/ui-diff.c+++ b/source/ui-diff.c@@ -604,31 +604,17 @@ void cgit_print_diff(const char *new_rev, const char *old_rev, const char *prefi  	if (!new_rev) 		new_rev = ctx.qry.head;-	if (repo_get_oid(the_repository, new_rev, new_rev_oid)) {-		cgit_print_error_page(404, "Not Found", "Bad object id: %s", new_rev);+	new_commit = cgit_get_commit(new_rev, new_rev_oid);+	if (!new_commit || cgit_get_old_oid(old_rev, new_commit, old_rev_oid)) 		return;-	}-	new_commit = lookup_commit_reference(the_repository, new_rev_oid);-	if (!new_commit || repo_parse_commit(the_repository, new_commit)) {-		cgit_print_error_page(404, "Not Found", "Not a commit: %s", oid_to_hex(new_rev_oid));-		return;-	} 	new_tree_oid = get_commit_tree_oid(new_commit); -	if (old_rev) {-		if (repo_get_oid(the_repository, old_rev, old_rev_oid)) {-			cgit_print_error_page(404, "Not Found", "Bad object id: %s", old_rev);-			return;-		}-	} else if (new_commit->parents && new_commit->parents->item) {-		oidcpy(old_rev_oid, &new_commit->parents->item->object.oid);-	} else {-		oidclr(old_rev_oid, the_repository->hash_algo);-	}- 	if (!is_null_oid(old_rev_oid)) {+		// A named old side is a commit already, so only a first parent+		// missing from the repository fails here, and it has no name+		// but its id. 		old_commit = lookup_commit_reference(the_repository, old_rev_oid);-		if (!old_commit || repo_parse_commit(the_repository, old_commit)) {+		if (!old_commit) { 			cgit_print_error_page(404, "Not Found", "Not a commit: %s", oid_to_hex(old_rev_oid)); 			return; 		}
diff --git a/source/ui-diff.c b/source/ui-diff.c
index 736cdaf..51df73d 100644
--- a/source/ui-diff.c
+++ b/source/ui-diff.c
@@ -604,31 +604,17 @@ void cgit_print_diff(const char *new_rev, const char *old_rev, const char *prefi
 
 	if (!new_rev)
 		new_rev = ctx.qry.head;
-	if (repo_get_oid(the_repository, new_rev, new_rev_oid)) {
-		cgit_print_error_page(404, "Not Found", "Bad object id: %s", new_rev);
+	new_commit = cgit_get_commit(new_rev, new_rev_oid);
+	if (!new_commit || cgit_get_old_oid(old_rev, new_commit, old_rev_oid))
 		return;
-	}
-	new_commit = lookup_commit_reference(the_repository, new_rev_oid);
-	if (!new_commit || repo_parse_commit(the_repository, new_commit)) {
-		cgit_print_error_page(404, "Not Found", "Not a commit: %s", oid_to_hex(new_rev_oid));
-		return;
-	}
 	new_tree_oid = get_commit_tree_oid(new_commit);
 
-	if (old_rev) {
-		if (repo_get_oid(the_repository, old_rev, old_rev_oid)) {
-			cgit_print_error_page(404, "Not Found", "Bad object id: %s", old_rev);
-			return;
-		}
-	} else if (new_commit->parents && new_commit->parents->item) {
-		oidcpy(old_rev_oid, &new_commit->parents->item->object.oid);
-	} else {
-		oidclr(old_rev_oid, the_repository->hash_algo);
-	}
-
 	if (!is_null_oid(old_rev_oid)) {
+		// A named old side is a commit already, so only a first parent
+		// missing from the repository fails here, and it has no name
+		// but its id.
 		old_commit = lookup_commit_reference(the_repository, old_rev_oid);
-		if (!old_commit || repo_parse_commit(the_repository, old_commit)) {
+		if (!old_commit) {
 			cgit_print_error_page(404, "Not Found", "Not a commit: %s", oid_to_hex(old_rev_oid));
 			return;
 		}
diff --git a/source/ui-patch.c b/source/ui-patch.cindex 9cf61c0..763776a 100644--- a/source/ui-patch.c+++ b/source/ui-patch.c@@ -16,42 +16,6 @@ // Room for two hex object ids, the two dots between them, and the null byte. #define REV_RANGE_LEN (2 * GIT_MAX_HEXSZ + 3) -/*- * A root commit has no parent, so the old end is left null and the caller asks- * for that one commit instead.- */-static int resolve_range(const char *new_rev, const char *old_rev,-	struct object_id *new_oid, struct object_id *old_oid)-{-	struct commit *commit;--	if (repo_get_oid(the_repository, new_rev, new_oid)) {-		cgit_print_error_page(404, "Not Found", "Bad object id: %s", new_rev);-		return -1;-	}-	commit = lookup_commit_reference(the_repository, new_oid);-	if (!commit) {-		cgit_print_error_page(404, "Not Found", "Not a commit: %s", new_rev);-		return -1;-	}--	if (old_rev) {-		if (repo_get_oid(the_repository, old_rev, old_oid)) {-			cgit_print_error_page(404, "Not Found", "Bad object id: %s", old_rev);-			return -1;-		}-		if (!lookup_commit_reference(the_repository, old_oid)) {-			cgit_print_error_page(404, "Not Found", "Not a commit: %s", old_rev);-			return -1;-		}-	} else if (commit->parents && commit->parents->item) {-		oidcpy(old_oid, &commit->parents->item->object.oid);-	} else {-		oidclr(old_oid, the_repository->hash_algo);-	}-	return 0;-}- void cgit_print_patch(const char *new_rev, const char *old_rev, const char *prefix) { 	struct rev_info rev;@@ -71,7 +35,8 @@ void cgit_print_patch(const char *new_rev, const char *old_rev, const char *pref 		return; 	} -	if (resolve_range(new_rev, old_rev, &new_oid, &old_oid))+	commit = cgit_get_commit(new_rev, &new_oid);+	if (!commit || cgit_get_old_oid(old_rev, commit, &old_oid)) 		return;  	if (is_null_oid(&old_oid)) {
diff --git a/source/ui-patch.c b/source/ui-patch.c
index 9cf61c0..763776a 100644
--- a/source/ui-patch.c
+++ b/source/ui-patch.c
@@ -16,42 +16,6 @@
 // Room for two hex object ids, the two dots between them, and the null byte.
 #define REV_RANGE_LEN (2 * GIT_MAX_HEXSZ + 3)
 
-/*
- * A root commit has no parent, so the old end is left null and the caller asks
- * for that one commit instead.
- */
-static int resolve_range(const char *new_rev, const char *old_rev,
-	struct object_id *new_oid, struct object_id *old_oid)
-{
-	struct commit *commit;
-
-	if (repo_get_oid(the_repository, new_rev, new_oid)) {
-		cgit_print_error_page(404, "Not Found", "Bad object id: %s", new_rev);
-		return -1;
-	}
-	commit = lookup_commit_reference(the_repository, new_oid);
-	if (!commit) {
-		cgit_print_error_page(404, "Not Found", "Not a commit: %s", new_rev);
-		return -1;
-	}
-
-	if (old_rev) {
-		if (repo_get_oid(the_repository, old_rev, old_oid)) {
-			cgit_print_error_page(404, "Not Found", "Bad object id: %s", old_rev);
-			return -1;
-		}
-		if (!lookup_commit_reference(the_repository, old_oid)) {
-			cgit_print_error_page(404, "Not Found", "Not a commit: %s", old_rev);
-			return -1;
-		}
-	} else if (commit->parents && commit->parents->item) {
-		oidcpy(old_oid, &commit->parents->item->object.oid);
-	} else {
-		oidclr(old_oid, the_repository->hash_algo);
-	}
-	return 0;
-}
-
 void cgit_print_patch(const char *new_rev, const char *old_rev, const char *prefix)
 {
 	struct rev_info rev;
@@ -71,7 +35,8 @@ void cgit_print_patch(const char *new_rev, const char *old_rev, const char *pref
 		return;
 	}
 
-	if (resolve_range(new_rev, old_rev, &new_oid, &old_oid))
+	commit = cgit_get_commit(new_rev, &new_oid);
+	if (!commit || cgit_get_old_oid(old_rev, commit, &old_oid))
 		return;
 
 	if (is_null_oid(&old_oid)) {
diff --git a/source/ui-shared.c b/source/ui-shared.cindex 5f5dcca..b34861d 100644--- a/source/ui-shared.c+++ b/source/ui-shared.c@@ -1499,6 +1499,33 @@ void cgit_print_blob_too_large(size_t size) 		size / 1024, ctx.cfg.max_blob_size); } +struct commit *cgit_get_commit(const char *rev, struct object_id *oid)+{+	struct commit *commit;++	if (repo_get_oid(the_repository, rev, oid)) {+		cgit_print_error_page(404, "Not Found", "Bad object id: %s", rev);+		return NULL;+	}+	// lookup_commit_reference parses the commit, and fails on one git+	// cannot parse.+	commit = lookup_commit_reference(the_repository, oid);+	if (!commit)+		cgit_print_error_page(404, "Not Found", "Not a commit: %s", rev);+	return commit;+}++int cgit_get_old_oid(const char *old_rev, const struct commit *commit, struct object_id *oid)+{+	if (old_rev)+		return cgit_get_commit(old_rev, oid) ? 0 : -1;+	if (commit->parents && commit->parents->item)+		oidcpy(oid, &commit->parents->item->object.oid);+	else+		oidclr(oid, the_repository->hash_algo);+	return 0;+}+ static void add_clone_urls(void (*fn)(const char *), char *urls, char *suffix) { 	struct strbuf **url_list = strbuf_split_str(urls, ' ', 0);
diff --git a/source/ui-shared.c b/source/ui-shared.c
index 5f5dcca..b34861d 100644
--- a/source/ui-shared.c
+++ b/source/ui-shared.c
@@ -1499,6 +1499,33 @@ void cgit_print_blob_too_large(size_t size)
 		size / 1024, ctx.cfg.max_blob_size);
 }
 
+struct commit *cgit_get_commit(const char *rev, struct object_id *oid)
+{
+	struct commit *commit;
+
+	if (repo_get_oid(the_repository, rev, oid)) {
+		cgit_print_error_page(404, "Not Found", "Bad object id: %s", rev);
+		return NULL;
+	}
+	// lookup_commit_reference parses the commit, and fails on one git
+	// cannot parse.
+	commit = lookup_commit_reference(the_repository, oid);
+	if (!commit)
+		cgit_print_error_page(404, "Not Found", "Not a commit: %s", rev);
+	return commit;
+}
+
+int cgit_get_old_oid(const char *old_rev, const struct commit *commit, struct object_id *oid)
+{
+	if (old_rev)
+		return cgit_get_commit(old_rev, oid) ? 0 : -1;
+	if (commit->parents && commit->parents->item)
+		oidcpy(oid, &commit->parents->item->object.oid);
+	else
+		oidclr(oid, the_repository->hash_algo);
+	return 0;
+}
+
 static void add_clone_urls(void (*fn)(const char *), char *urls, char *suffix)
 {
 	struct strbuf **url_list = strbuf_split_str(urls, ' ', 0);
diff --git a/source/ui-shared.h b/source/ui-shared.hindex fb02d7b..871dba0 100644--- a/source/ui-shared.h+++ b/source/ui-shared.h@@ -121,6 +121,20 @@ extern void cgit_print_error_page(int code, const char *msg, const char *fmt, ..  * Answer with the error page for a blob of size bytes past max-blob-size.  */ extern void cgit_print_blob_too_large(size_t size);++/*+ * Resolve rev to the parsed commit it names, peeling a tag, with oid set to+ * what rev itself names. Returns NULL once the 404 page naming rev is out.+ */+extern struct commit *cgit_get_commit(const char *rev, struct object_id *oid);++/*+ * Fill oid with the older end of a comparison with commit, which is what+ * old_rev names when it is given, else commit's first parent, else the null+ * id for a root commit. Returns -1 once the 404 page for old_rev is out.+ */+extern int cgit_get_old_oid(const char *old_rev, const struct commit *commit, struct object_id *oid);+ extern void cgit_print_filemode(unsigned short mode); extern void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref, 	const char *separator);
diff --git a/source/ui-shared.h b/source/ui-shared.h
index fb02d7b..871dba0 100644
--- a/source/ui-shared.h
+++ b/source/ui-shared.h
@@ -121,6 +121,20 @@ extern void cgit_print_error_page(int code, const char *msg, const char *fmt, ..
  * Answer with the error page for a blob of size bytes past max-blob-size.
  */
 extern void cgit_print_blob_too_large(size_t size);
+
+/*
+ * Resolve rev to the parsed commit it names, peeling a tag, with oid set to
+ * what rev itself names. Returns NULL once the 404 page naming rev is out.
+ */
+extern struct commit *cgit_get_commit(const char *rev, struct object_id *oid);
+
+/*
+ * Fill oid with the older end of a comparison with commit, which is what
+ * old_rev names when it is given, else commit's first parent, else the null
+ * id for a root commit. Returns -1 once the 404 page for old_rev is out.
+ */
+extern int cgit_get_old_oid(const char *old_rev, const struct commit *commit, struct object_id *oid);
+
 extern void cgit_print_filemode(unsigned short mode);
 extern void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref,
 	const char *separator);
diff --git a/source/ui-snapshot.c b/source/ui-snapshot.cindex 2fa121f..333bce6 100644--- a/source/ui-snapshot.c+++ b/source/ui-snapshot.c@@ -224,14 +224,8 @@ static void send_snapshot(const struct cgit_snapshot_format *format, const char { 	struct object_id oid; -	if (repo_get_oid(the_repository, hex, &oid)) {-		cgit_print_error_page(404, "Not Found", "Bad object id: %s", hex);+	if (!cgit_get_commit(hex, &oid)) 		return;-	}-	if (!lookup_commit_reference(the_repository, &oid)) {-		cgit_print_error_page(404, "Not Found", "Not a commit: %s", hex);-		return;-	} 	// A compressor that cannot run only shows up as a broken pipe once 	// the tar is under way, so it is looked for while a status can still 	// say so.
diff --git a/source/ui-snapshot.c b/source/ui-snapshot.c
index 2fa121f..333bce6 100644
--- a/source/ui-snapshot.c
+++ b/source/ui-snapshot.c
@@ -224,14 +224,8 @@ static void send_snapshot(const struct cgit_snapshot_format *format, const char
 {
 	struct object_id oid;
 
-	if (repo_get_oid(the_repository, hex, &oid)) {
-		cgit_print_error_page(404, "Not Found", "Bad object id: %s", hex);
+	if (!cgit_get_commit(hex, &oid))
 		return;
-	}
-	if (!lookup_commit_reference(the_repository, &oid)) {
-		cgit_print_error_page(404, "Not Found", "Not a commit: %s", hex);
-		return;
-	}
 	// A compressor that cannot run only shows up as a broken pipe once
 	// the tar is under way, so it is looked for while a status can still
 	// say so.
diff --git a/source/ui-tree.c b/source/ui-tree.cindex 1c29e30..300207f 100644--- a/source/ui-tree.c+++ b/source/ui-tree.c@@ -486,15 +486,9 @@ void cgit_print_tree(const char *rev, char *path) 	if (!rev) 		rev = ctx.qry.head; -	if (repo_get_oid(the_repository, rev, &oid)) {-		cgit_print_error_page(404, "Not Found", "Bad object id: %s", rev);+	commit = cgit_get_commit(rev, &oid);+	if (!commit) 		return;-	}-	commit = lookup_commit_reference(the_repository, &oid);-	if (!commit || repo_parse_commit(the_repository, commit)) {-		cgit_print_error_page(404, "Not Found", "Not a commit: %s", rev);-		return;-	}  	walk.rev = rev; 
diff --git a/source/ui-tree.c b/source/ui-tree.c
index 1c29e30..300207f 100644
--- a/source/ui-tree.c
+++ b/source/ui-tree.c
@@ -486,15 +486,9 @@ void cgit_print_tree(const char *rev, char *path)
 	if (!rev)
 		rev = ctx.qry.head;
 
-	if (repo_get_oid(the_repository, rev, &oid)) {
-		cgit_print_error_page(404, "Not Found", "Bad object id: %s", rev);
+	commit = cgit_get_commit(rev, &oid);
+	if (!commit)
 		return;
-	}
-	commit = lookup_commit_reference(the_repository, &oid);
-	if (!commit || repo_parse_commit(the_repository, commit)) {
-		cgit_print_error_page(404, "Not Found", "Not a commit: %s", rev);
-		return;
-	}
 
 	walk.rev = rev;
 
diff --git a/tests/t0107-diff.sh b/tests/t0107-diff.shindex f058246..96e37f5 100755--- a/tests/t0107-diff.sh+++ b/tests/t0107-diff.sh@@ -22,6 +22,16 @@ test_expect_success 'find added line' ' 	grep "<div class=.add.>+5</div>" tmp ' +# A revision that names no commit is quoted the way the request wrote it, as+# the commit and patch pages quote it, on either side of the diff.+test_expect_success 'a tree on either side is named as the request gave it' '+	tree=$(git -C repos/foo rev-parse --short HEAD^{tree}) &&+	cgit_query "url=foo/diff/&id=$tree" >tmp &&+	grep "<div class=.error.>Not a commit: $tree</div>" tmp &&+	cgit_query "url=foo/diff/&id2=$tree" >tmp &&+	grep "<div class=.error.>Not a commit: $tree</div>" tmp+'+ # git writes a hunk length only when the hunk spans more than one line, so the # header for a single line reads "@@ -1 +1 @@" with no comma in it. Requiring # that comma left those hunks numbered from zero, which dropped the line
diff --git a/tests/t0107-diff.sh b/tests/t0107-diff.sh
index f058246..96e37f5 100755
--- a/tests/t0107-diff.sh
+++ b/tests/t0107-diff.sh
@@ -22,6 +22,16 @@ test_expect_success 'find added line' '
 	grep "<div class=.add.>+5</div>" tmp
 '
 
+# A revision that names no commit is quoted the way the request wrote it, as
+# the commit and patch pages quote it, on either side of the diff.
+test_expect_success 'a tree on either side is named as the request gave it' '
+	tree=$(git -C repos/foo rev-parse --short HEAD^{tree}) &&
+	cgit_query "url=foo/diff/&id=$tree" >tmp &&
+	grep "<div class=.error.>Not a commit: $tree</div>" tmp &&
+	cgit_query "url=foo/diff/&id2=$tree" >tmp &&
+	grep "<div class=.error.>Not a commit: $tree</div>" tmp
+'
+
 # git writes a hunk length only when the hunk spans more than one line, so the
 # header for a single line reads "@@ -1 +1 @@" with no comma in it. Requiring
 # that comma left those hunks numbered from zero, which dropped the line