Harden the page renderers

1 file changed, 62 insertions, 26 deletions

diff --git a/source/ui-diff.c b/source/ui-diff.c
index fb86a96..ee8dc76 100644
--- a/source/ui-diff.c
+++ b/source/ui-diff.c
@@ -23,10 +23,6 @@
 // and a second walk renders the page instead.
 #define BODY_BUDGET (8 * 1024 * 1024)
 
-// What a context of zero means once the diff runs, mirroring the fallback in
-// cgit_diff_files, so the control offers the value the diff will really use.
-#define DEFAULT_CONTEXT_LINES 3
-
 struct fileinfo {
 	char status;
 	struct object_id old_oid[1];
@@ -120,7 +116,7 @@ static void print_fileinfo(struct fileinfo *info)
 		class = "stg";
 		break;
 	default:
-		die("bug: unhandled diff status %c", info->status);
+		BUG("unhandled diff status %c", info->status);
 	}
 
 	html("<tr>");
@@ -131,13 +127,18 @@ static void print_fileinfo(struct fileinfo *info)
 		cgit_print_filemode(info->new_mode);
 	}
 
-	if (info->old_mode != info->new_mode && !is_null_oid(info->old_oid) && !is_null_oid(info->new_oid)) {
+	if (
+		info->old_mode != info->new_mode	&&
+		!is_null_oid(info->old_oid)			&&
+		!is_null_oid(info->new_oid)
+	) {
 		html("<span class='modechange'>[");
 		cgit_print_filemode(info->old_mode);
 		html("]</span>");
 	}
 	htmlf("</td><td class='%s'>", class);
-	cgit_diff_link(info->new_path, NULL, NULL, ctx.qry.head, ctx.qry.oid, ctx.qry.oid2, info->new_path);
+	cgit_diff_link(info->new_path, NULL, NULL, ctx.qry.head, ctx.qry.oid, ctx.qry.oid2,
+		info->new_path);
 	if (info->status == DIFF_STATUS_COPIED || info->status == DIFF_STATUS_RENAMED) {
 		htmlf(" (%s from ", info->status == DIFF_STATUS_COPIED ? "copied" : "renamed");
 		html_txt(info->old_path);
@@ -145,10 +146,11 @@ static void print_fileinfo(struct fileinfo *info)
 	}
 	html("</td><td class='right'>");
 	if (info->binary) {
-		htmlf("bin</td><td class='graph'>%lu -> %lu bytes</td></tr>\n", info->old_size, info->new_size);
+		htmlf("bin</td><td class='graph'>%lu -> %lu bytes</td></tr>\n", info->old_size,
+			info->new_size);
 		return;
 	}
-	htmlf("%d", info->added + info->removed);
+	htmlf("%u", info->added + info->removed);
 	html("</td><td class='graph'>");
 	html("<table><tr>");
 	add_span = (int)(info->added * 1000.0 / scale + 0.5);
@@ -173,7 +175,11 @@ static void count_diff_lines(char *line, int len)
 	}
 	if (!render_line_fn || render_suppressed)
 		return;
-	if (cap_diffs && ctx.cfg.max_diff_lines > 0 && lines_added + lines_removed > ctx.cfg.max_diff_lines) {
+	if (
+		cap_diffs								&&
+		ctx.cfg.max_diff_lines > 0				&&
+		lines_added + lines_removed > ctx.cfg.max_diff_lines
+	) {
 		render_suppressed = 1;
 		return;
 	}
@@ -184,7 +190,8 @@ static int show_filepair(struct diff_filepair *pair)
 {
 	if (!current_prefix)
 		return 1;
-	return starts_with(pair->one->path, current_prefix) || starts_with(pair->two->path, current_prefix);
+	return starts_with(pair->one->path, current_prefix) ||
+		starts_with(pair->two->path, current_prefix);
 }
 
 /*
@@ -220,8 +227,8 @@ static char *abbrev_oid(const struct object_id *oid)
 	return xstrdup(repo_find_unique_abbrev(the_repository, oid, DEFAULT_ABBREV));
 }
 
-static void print_file_header(const struct object_id *old_oid, char *old_path, int old_mode,
-	const struct object_id *new_oid, char *new_path, int new_mode)
+static void print_file_header(const struct object_id *old_oid, char *old_path, unsigned old_mode,
+	const struct object_id *new_oid, char *new_path, unsigned new_mode)
 {
 	char *old_abbrev, *new_abbrev;
 	int subproject;
@@ -253,8 +260,9 @@ static void print_file_header(const struct object_id *old_oid, char *old_path, i
 		if (is_null_oid(old_oid)) {
 			old_path = "dev/null";
 			html("<br>--- /");
-		} else
+		} else {
 			html("<br>--- a/");
+		}
 		if (old_mode != 0)
 			cgit_tree_link(old_path, NULL, NULL, ctx.qry.head, oid_to_hex(old_rev_oid), old_path);
 		else
@@ -262,8 +270,9 @@ static void print_file_header(const struct object_id *old_oid, char *old_path, i
 		if (is_null_oid(new_oid)) {
 			new_path = "dev/null";
 			html("<br>+++ /");
-		} else
+		} else {
 			html("<br>+++ b/");
+		}
 		if (new_mode != 0)
 			cgit_tree_link(new_path, NULL, NULL, ctx.qry.head, oid_to_hex(new_rev_oid), new_path);
 		else
@@ -303,13 +312,15 @@ static void print_truncated(const char *path)
 	else
 		html("<div class='truncated'>");
 	html("This diff is too large to be rendered inline. ");
-	cgit_diff_link("View it on its own page", NULL, NULL, ctx.qry.head, ctx.qry.oid, ctx.qry.oid2, path);
+	cgit_diff_link("View it on its own page", NULL, NULL, ctx.qry.head, ctx.qry.oid, ctx.qry.oid2,
+		path);
 	html(".");
 	if (use_ssdiff) {
 		html("</td></tr>");
 		cgit_ssdiff_footer();
-	} else
+	} else {
 		html("</div>");
+	}
 }
 
 static struct fileinfo *reserve_item(void)
@@ -552,7 +563,7 @@ void cgit_print_diff_ctrls(void)
 	html("<select name='context'>");
 	selected = ctx.qry.context;
 	if (!selected)
-		selected = DEFAULT_CONTEXT_LINES;
+		selected = DEFAULT_DIFF_CONTEXT;
 	for (i = 1; i <= 10; i++)
 		html_intoption(i, cgit_fmt("%d", i), selected);
 	for (i = 15; i <= MAX_DIFF_CONTEXT_LINES; i += 5)
@@ -583,17 +594,37 @@ void cgit_print_diff_ctrls(void)
 	html("</div>\n");
 }
 
+/*
+ * Whether the path names a directory in either tree. The diff below it can
+ * then still span many files, so the caps stay on.
+ */
+static int prefix_is_dir(const struct object_id *old_tree, const struct object_id *new_tree,
+	const char *prefix)
+{
+	struct object_id oid;
+	unsigned short mode;
+	char *path = xstrdup(prefix);
+	size_t len = strlen(path);
+	int is_dir = 0;
+
+	if (len && path[len - 1] == '/')
+		path[len - 1] = '\0';
+	if (new_tree && !get_tree_entry(the_repository, new_tree, path, &oid, &mode))
+		is_dir = S_ISDIR(mode);
+	else if (old_tree && !get_tree_entry(the_repository, old_tree, path, &oid, &mode))
+		is_dir = S_ISDIR(mode);
+	free(path);
+	return is_dir;
+}
+
 void cgit_print_diff(const char *new_rev, const char *old_rev, const char *prefix,
 	int show_ctrls, int raw)
 {
 	struct commit *new_commit, *old_commit;
 	const struct object_id *old_tree_oid, *new_tree_oid;
+	const char *path = prefix;
 	diff_type difftype;
 
-	// Decided from the caller's prefix before the follow logic below
-	// rewrites it to "", otherwise follow=1 silently disables the caps.
-	cap_diffs = !prefix;
-
 	// Detecting renames needs the diff machinery to examine the whole
 	// commit, so with follow set the prefix is applied in show_filepair
 	// instead of being passed down.
@@ -607,19 +638,19 @@ 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 name: %s", new_rev);
+		cgit_print_error_page(404, "Not Found", "Bad object id: %s", new_rev);
 		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", "Bad commit: %s", oid_to_hex(new_rev_oid));
+		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 name: %s", old_rev);
+			cgit_print_error_page(404, "Not Found", "Bad object id: %s", old_rev);
 			return;
 		}
 	} else if (new_commit->parents && new_commit->parents->item) {
@@ -631,7 +662,7 @@ void cgit_print_diff(const char *new_rev, const char *old_rev, const char *prefi
 	if (!is_null_oid(old_rev_oid)) {
 		old_commit = lookup_commit_reference(the_repository, old_rev_oid);
 		if (!old_commit || repo_parse_commit(the_repository, old_commit)) {
-			cgit_print_error_page(404, "Not Found", "Bad commit: %s", oid_to_hex(old_rev_oid));
+			cgit_print_error_page(404, "Not Found", "Not a commit: %s", oid_to_hex(old_rev_oid));
 			return;
 		}
 		old_tree_oid = get_commit_tree_oid(old_commit);
@@ -639,6 +670,11 @@ void cgit_print_diff(const char *new_rev, const char *old_rev, const char *prefi
 		old_tree_oid = NULL;
 	}
 
+	// A path narrows the diff to one file, whose own page always renders
+	// in full. Decided from the caller's path, since the follow logic
+	// above has emptied the prefix.
+	cap_diffs = !path || prefix_is_dir(old_tree_oid, new_tree_oid, path);
+
 	if (raw) {
 		print_raw_patch(old_tree_oid, new_tree_oid);
 		return;