Harden the page renderers

1 file changed, 122 insertions, 65 deletions

diff --git a/source/ui-shared.c b/source/ui-shared.c
index bb786a6..25a84a6 100644
--- a/source/ui-shared.c
+++ b/source/ui-shared.c
@@ -27,9 +27,6 @@
 #define MIN_TRUNCATE_LEN 15
 #define ELLIPSIS_LEN 3
 
-// Lines of context git itself defaults to, which a link has no reason to name.
-#define DEFAULT_DIFF_CONTEXT 3
-
 // Bounds the breadcrumbs, so that one request cannot turn into an unbounded
 // row of links.
 #define MAX_CRUMB_LEVELS 15
@@ -49,7 +46,7 @@ static const char *repo_basename(const char *reponame)
 
 	len = strlcpy(buf, reponame, sizeof(buf));
 	if (len >= sizeof(buf))
-		die("repo_basename: truncated repository name '%s'", reponame);
+		die("Repository name too long: %s", reponame);
 	last = len - 1;
 	while (last && buf[last] == '/')
 		buf[last--] = '\0';
@@ -212,7 +209,7 @@ static void emit_diff_args(const char *delim)
 {
 	if (ctx.qry.difftype) {
 		html(delim);
-		htmlf("dt=%d", ctx.qry.difftype);
+		htmlf("dt=%d", (int)ctx.qry.difftype);
 		delim = "&";
 	}
 	if (ctx.qry.context > 0 && ctx.qry.context != DEFAULT_DIFF_CONTEXT) {
@@ -395,9 +392,16 @@ static void add_clone_urls(void (*fn)(const char *), char *urls, char *suffix)
 static const struct object_id *pinned_oid(void)
 {
 	static struct object_id oid;
+	static int resolved, pinned;
 	struct object_id head_oid;
 	struct commit *commit;
 
+	// The chrome asks several times per page and the answer cannot change
+	// within a request.
+	if (resolved)
+		return pinned ? &oid : NULL;
+	resolved = 1;
+
 	if (!ctx.repo || !ctx.qry.has_oid || !ctx.qry.oid || !ctx.qry.head)
 		return NULL;
 	if (repo_get_oid(the_repository, ctx.qry.oid, &oid) ||
@@ -412,6 +416,7 @@ static const struct object_id *pinned_oid(void)
 	oidcpy(&oid, &commit->object.oid);
 	if (oideq(&oid, &head_oid))
 		return NULL;
+	pinned = 1;
 	return &oid;
 }
 
@@ -463,6 +468,7 @@ static int print_branch_option(const struct reference *ref, void *data)
 {
 	struct branch_option_data *opt = data;
 	const char *name = ref->name;
+
 	// The switcher runs on every page, so it is bounded like the refs list.
 	if (ctx.cfg.max_ref_count && opt->count >= ctx.cfg.max_ref_count)
 		return -1;
@@ -501,13 +507,15 @@ static void print_header(void)
 		cgit_index_link("index", NULL, NULL, NULL, NULL, 0, 1);
 		html(" : ");
 		cgit_summary_link(ctx.repo->name, NULL, NULL, NULL);
-	} else
+	} else {
 		html_txt(ctx.cfg.root_title);
+	}
 	html("</h1>\n");
 
 	// A repository with no commits has no branches to list, so the
-	// switcher would be an empty select next to a switch button.
-	if (ctx.repo && ctx.env.authenticated && !ctx.empty_repo) {
+	// switcher would be an empty select next to a switch button, and an
+	// error raised before the head was resolved has nothing to select.
+	if (ctx.repo && ctx.env.authenticated && !ctx.empty_repo && ctx.qry.head) {
 		const struct object_id *pinned = pinned_oid();
 		// Only one option may carry selected, and a pinned commit
 		// outranks the branch it was reached from.
@@ -577,7 +585,8 @@ static void print_repo_tabs(void)
 	html("<ul>\n");
 	if (ctx.repo->readme.nr) {
 		html("<li>");
-		reporevlink("about", "about", "About this repository", tab_class("about"), ctx.qry.head, NULL, NULL);
+		reporevlink("about", "about", "About this repository", tab_class("about"), ctx.qry.head,
+			NULL, NULL);
 		html("</li>\n");
 	}
 	html("<li>");
@@ -677,10 +686,14 @@ static void snapshot_link(const char *name, const char *title, const char *class
 	reporevlink("snapshot", name, title, class, head, rev, archivename);
 }
 
-// The link is built out of the request in ctx.qry, so a caller that alters a
-// field of ctx.qry first gets a link differing in exactly that.
+/*
+ * The link is built out of the request in ctx.qry, so a caller that alters a
+ * field of ctx.qry first gets a link differing in exactly that.
+ */
 static void self_link(const char *name, const char *title, const char *class)
 {
+	const char *rev = ctx.qry.has_oid ? ctx.qry.oid : NULL;
+
 	if (!strcmp(ctx.qry.page, "repolist"))
 		cgit_index_link(name, title, class, ctx.qry.search, ctx.qry.sort, ctx.qry.ofs, 1);
 	else if (!strcmp(ctx.qry.page, "summary"))
@@ -688,32 +701,31 @@ static void self_link(const char *name, const char *title, const char *class)
 	else if (!strcmp(ctx.qry.page, "tag"))
 		cgit_tag_link(name, title, class, ctx.qry.has_oid ? ctx.qry.oid : ctx.qry.head);
 	else if (!strcmp(ctx.qry.page, "tree"))
-		cgit_tree_link(name, title, class, ctx.qry.head, ctx.qry.has_oid ? ctx.qry.oid : NULL, ctx.qry.path);
+		cgit_tree_link(name, title, class, ctx.qry.head, rev, ctx.qry.path);
 	else if (!strcmp(ctx.qry.page, "plain"))
-		cgit_plain_link(name, title, class, ctx.qry.head, ctx.qry.has_oid ? ctx.qry.oid : NULL, ctx.qry.path);
+		cgit_plain_link(name, title, class, ctx.qry.head, rev, ctx.qry.path);
 	else if (!strcmp(ctx.qry.page, "blame"))
-		cgit_blame_link(name, title, class, ctx.qry.head, ctx.qry.has_oid ? ctx.qry.oid : NULL, ctx.qry.path);
+		cgit_blame_link(name, title, class, ctx.qry.head, rev, ctx.qry.path);
 	else if (!strcmp(ctx.qry.page, "log"))
 		cgit_log_link(
-			name, title, class, ctx.qry.head, ctx.qry.has_oid ? ctx.qry.oid : NULL,
-			ctx.qry.path, ctx.qry.ofs, ctx.qry.grep, ctx.qry.search,
-			ctx.qry.showmsg, ctx.qry.follow
+			name, title, class, ctx.qry.head, rev, ctx.qry.path, ctx.qry.ofs, ctx.qry.grep,
+			ctx.qry.search, ctx.qry.showmsg, ctx.qry.follow
 		);
 	else if (!strcmp(ctx.qry.page, "commit"))
-		cgit_commit_link(name, title, class, ctx.qry.head, ctx.qry.has_oid ? ctx.qry.oid : NULL, ctx.qry.path);
+		cgit_commit_link(name, title, class, ctx.qry.head, rev, ctx.qry.path);
 	else if (!strcmp(ctx.qry.page, "patch"))
-		cgit_patch_link(name, title, class, ctx.qry.head, ctx.qry.has_oid ? ctx.qry.oid : NULL, ctx.qry.path);
+		cgit_patch_link(name, title, class, ctx.qry.head, rev, ctx.qry.path);
 	else if (!strcmp(ctx.qry.page, "refs"))
-		cgit_refs_link(name, title, class, ctx.qry.head, ctx.qry.has_oid ? ctx.qry.oid : NULL, ctx.qry.path);
+		cgit_refs_link(name, title, class, ctx.qry.head, rev, ctx.qry.path);
 	else if (!strcmp(ctx.qry.page, "snapshot"))
-		snapshot_link(name, title, class, ctx.qry.head, ctx.qry.has_oid ? ctx.qry.oid : NULL, ctx.qry.path);
+		snapshot_link(name, title, class, ctx.qry.head, rev, ctx.qry.path);
 	else if (!strcmp(ctx.qry.page, "diff"))
 		cgit_diff_link(name, title, class, ctx.qry.head, ctx.qry.oid, ctx.qry.oid2, ctx.qry.path);
 	else if (!strcmp(ctx.qry.page, "stats"))
 		cgit_stats_link(name, title, class, ctx.qry.head, ctx.qry.path);
 	else {
 		// A page name this switch does not know still gets a plain
-		// repolink, which covers any simple page added later.
+		// repolink.
 		repolink(title, class, ctx.qry.page, ctx.qry.head, ctx.qry.path);
 		html("'>");
 		html_txt(name);
@@ -746,7 +758,9 @@ static void print_path_crumbs(char *path)
 	ctx.qry.path = old_path;
 }
 
-// A reverse memchr, which glibc has as memrchr but macOS and the BSDs do not.
+/*
+ * A reverse memchr, which glibc has as memrchr but macOS and the BSDs do not.
+ */
 static const char *find_last_char(const char *start, const char *end, int c)
 {
 	while (end > start) {
@@ -759,6 +773,7 @@ static const char *find_last_char(const char *start, const char *end, int c)
 static void vprint_error(const char *fmt, va_list ap)
 {
 	va_list cp;
+
 	html("<div class='error'>");
 	va_copy(cp, ap);
 	html_vtxtf(fmt, cp);
@@ -769,6 +784,7 @@ static void vprint_error(const char *fmt, va_list ap)
 void cgit_print_error(const char *fmt, ...)
 {
 	va_list ap;
+
 	va_start(ap, fmt);
 	vprint_error(fmt, ap);
 	va_end(ap);
@@ -819,15 +835,16 @@ char *cgit_currentfullurl(void)
 	memcpy(query + 1, orig_query, len + 1);
 	query[0] = '?';
 	match = query;
-	while ((match = strstr(match, "url=")) != NULL) {
+	while ((match = strstr(match, "url="))) {
 		if (match[-1] == '?' || match[-1] == '&') {
 			const char *next = strchr(match, '&');
 			if (next)
 				memmove(match, next + 1, strlen(next));
 			else
 				match[0] = '\0';
-		} else
-			++match;
+		} else {
+			match++;
+		}
 	}
 	if (!query[1])
 		query[0] = '\0';
@@ -845,6 +862,7 @@ char *cgit_currentfullurl(void)
 const char *cgit_loginurl(void)
 {
 	static const char *login_url;
+
 	if (!login_url)
 		login_url = cgit_fmtalloc("%s?p=login", root_url());
 	return login_url;
@@ -868,7 +886,7 @@ char *cgit_fileurl(const char *reponame, const char *pagename, const char *filen
 	// and each sink escapes the whole string for wherever it lands.
 	if (ctx.cfg.virtual_root) {
 		strbuf_addf(&sb, "%s%s/%s/%s", ctx.cfg.virtual_root, reponame, pagename,
-			(filename ? filename:""));
+			filename ? filename : "");
 		delim = "?";
 	} else {
 		strbuf_addf(&sb, "?url=%s/%s/%s", reponame, pagename, (filename ? filename : ""));
@@ -974,12 +992,20 @@ void cgit_commit_link(const char *name, const char *title, const char *class,
 	html("'>");
 	if (name && name[0] != '\0') {
 		if (ctx.cfg.max_msg_len >= MIN_TRUNCATE_LEN && strlen(name) > (size_t)ctx.cfg.max_msg_len) {
-			html_ntxt(name, ctx.cfg.max_msg_len - ELLIPSIS_LEN);
+			size_t len = ctx.cfg.max_msg_len - ELLIPSIS_LEN;
+
+			// A cut inside a multibyte character would leave an
+			// invalid sequence on the page.
+			while (len > 0 && (name[len] & 0xC0) == 0x80)
+				len--;
+			html_ntxt(name, len);
 			html("...");
-		} else
+		} else {
 			html_txt(name);
-	} else
+		}
+	} else {
 		html_txt("(no commit message)");
+	}
 	html("</a>");
 }
 
@@ -1151,11 +1177,9 @@ static struct cgit_repo *repo_serving_url(const char *url)
 	return best;
 }
 
-/*
- * Hosts whose repository and commit pages follow a known form, so an ssh
- * url can still be offered as a browser link. kernel.org runs cgit, which
- * keeps the .git suffix in its own page urls.
- */
+// Hosts whose repository and commit pages follow a known form, so an ssh url
+// can still be offered as a browser link. kernel.org runs cgit, which keeps
+// the .git suffix in its own page urls.
 static const struct forge {
 	const char *host;
 	const char *commit_seg;
@@ -1336,6 +1360,7 @@ void cgit_submodule_link(const char *class, char *path, const char *rev)
 struct date_mode cgit_date_mode(enum date_mode_type type)
 {
 	static struct date_mode mode;
+
 	mode.type = type;
 	mode.local = ctx.cfg.local_time;
 	return mode;
@@ -1382,6 +1407,7 @@ void cgit_print_age(time_t t, int tz, time_t max_relative)
 
 void cgit_print_http_headers(void)
 {
+	ctx.page.headers_sent = 1;
 	if (ctx.env.no_http && !strcmp(ctx.env.no_http, "1"))
 		return;
 
@@ -1396,7 +1422,7 @@ void cgit_print_http_headers(void)
 	else if (ctx.page.mimetype)
 		htmlf("Content-Type: %s\n", ctx.page.mimetype);
 	if (ctx.page.size)
-		htmlf("Content-Length: %zd\n", ctx.page.size);
+		htmlf("Content-Length: %zu\n", ctx.page.size);
 	if (ctx.page.filename) {
 		html("Content-Disposition: inline; filename=\"");
 		html_header_arg_in_quotes(ctx.page.filename);
@@ -1539,6 +1565,7 @@ void cgit_print_docend(void)
 void cgit_print_error_page(int code, const char *msg, const char *fmt, ...)
 {
 	va_list ap;
+
 	va_start(ap, fmt);
 	cgit_vprint_error_page(code, msg, fmt, ap);
 	va_end(ap);
@@ -1550,9 +1577,21 @@ void cgit_vprint_error_page(int code, const char *msg, const char *fmt, va_list
 	// holding it. A request naming a commit not yet pushed would cache its
 	// 404 under the never-expiring static ttl and keep serving it after
 	// the push, so the error is rendered straight to the visitor instead.
+	html_flush();
 	cache_abandon_fill();
+	// Once the status line is out the error can only join the body under
+	// way, and the page that hit it carries on to its own end.
+	if (ctx.page.headers_sent) {
+		vprint_error(fmt, ap);
+		return;
+	}
 	ctx.page.status = code;
 	ctx.page.statusmsg = msg;
+	// A page that had already chosen another type, such as an image
+	// served through the about page, still answers its error as html.
+	ctx.page.mimetype = "text/html";
+	ctx.page.charset = PAGE_ENCODING;
+	ctx.page.filename = NULL;
 	cgit_print_layout_start();
 	vprint_error(fmt, ap);
 	cgit_print_layout_end();
@@ -1581,7 +1620,9 @@ void cgit_add_clone_urls(void (*fn)(const char *))
 static char *http_clone_url;
 static int http_clone_url_scanned;
 
-// Scheme names compare case-insensitively.
+/*
+ * Scheme names compare case-insensitively.
+ */
 static int is_http_url(const char *url)
 {
 	return istarts_with(url, "http://") || istarts_with(url, "https://");
@@ -1745,47 +1786,60 @@ void cgit_print_filemode(unsigned short mode)
 	html_fileperm(mode);
 }
 
-/*
- * One tag's claim on a snapshot stem, the stem being the tag name with any
- * leading v or V removed. Claims are counted over the tag list itself rather
- * than by looking refs up by name, because a loose ref lookup on a
- * case-insensitive filesystem finds v1.2 when asked for V1.2 and would call
- * every tag still stored loose ambiguous.
- */
-struct snapshot_stem {
-	const char *tag;
-	const char *stem;
-	int claimants;
-	int is_tag;
-};
+// The tag names and their stems, a stem being the name with any leading v or V
+// removed, gathered once per request. Claims on a stem are counted over the
+// tag list itself rather than by looking refs up by name, because a loose ref
+// lookup on a case-insensitive filesystem finds v1.2 when asked for V1.2 and
+// would call every tag still stored loose ambiguous.
+static struct string_list tag_names = STRING_LIST_INIT_DUP;
+static struct string_list tag_stems = STRING_LIST_INIT_DUP;
+static int tags_collected;
 
-static int count_stem_claimants(const struct reference *ref, void *data)
+static int collect_tag(const struct reference *ref, void *data)
 {
-	struct snapshot_stem *probe = data;
 	const char *name = ref->name;
 
-	if (!strcmp(name, probe->tag))
-		probe->is_tag = 1;
+	string_list_append(&tag_names, name);
 	if (name[0] == 'v' || name[0] == 'V')
 		name++;
-	if (!strcmp(name, probe->stem))
-		probe->claimants++;
+	string_list_append(&tag_stems, name);
 	return 0;
 }
 
+/*
+ * Whether ref is a tag whose stem no other tag shares.
+ */
+static int stem_is_unique(const char *ref)
+{
+	struct string_list_item *stem;
+	size_t i;
+
+	if (!tags_collected) {
+		tags_collected = 1;
+		refs_for_each_tag_ref(get_main_ref_store(the_repository), collect_tag, NULL);
+		string_list_sort(&tag_names);
+		string_list_sort(&tag_stems);
+	}
+	if (!string_list_has_string(&tag_names, ref))
+		return 0;
+	stem = string_list_lookup(&tag_stems, ref + 1);
+	if (!stem)
+		return 0;
+	// The lookup lands on any one of equal entries, so both neighbours
+	// are checked.
+	i = stem - tag_stems.items;
+	return (i == 0 || strcmp(tag_stems.items[i - 1].string, stem->string)) &&
+		(i + 1 == tag_stems.nr || strcmp(tag_stems.items[i + 1].string, stem->string));
+}
+
 static void compose_snapshot_prefix(struct strbuf *filename, const char *base, const char *ref)
 {
 	// A tag named v1.2 or V1.2 gives its snapshot the prettier name 1.2,
 	// but only where dropping the letter cannot land two different tags on
 	// one name, so that a snapshot can still be traced back to the tag it
 	// was made from.
-	if ((ref[0] == 'v' || ref[0] == 'V') && isdigit((unsigned char)ref[1])) {
-		struct snapshot_stem probe = { .tag = ref, .stem = ref + 1 };
-
-		refs_for_each_tag_ref(get_main_ref_store(the_repository), count_stem_claimants, &probe);
-		if (probe.is_tag && probe.claimants == 1)
-			ref++;
-	}
+	if ((ref[0] == 'v' || ref[0] == 'V') && isdigit((unsigned char)ref[1]) && stem_is_unique(ref))
+		ref++;
 
 	strbuf_addf(filename, "%s-%s", base, ref);
 }
@@ -1815,7 +1869,10 @@ void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref, co
 			html(" (");
 			snapshot_link("sig", NULL, NULL, NULL, NULL, filename.buf);
 			html(")");
-		} else if (starts_with(f->suffix, ".tar") && cgit_snapshot_get_sig(ref, &cgit_snapshot_formats[0])) {
+		} else if (
+			starts_with(f->suffix, ".tar") &&
+			cgit_snapshot_get_sig(ref, &cgit_snapshot_formats[0])
+		) {
 			// A compressed tarball offers the signature made for
 			// the plain tar it expands to, which is the first
 			// format in the table.
@@ -1841,7 +1898,7 @@ void cgit_set_title_from_path(const char *path)
 	// The path is read from the end so that the title names the file first
 	// and the directories it sits in after it.
 	last_slash = path + strlen(path);
-	while ((slash = find_last_char(path, last_slash, '/')) != NULL) {
+	while ((slash = find_last_char(path, last_slash, '/'))) {
 		strbuf_add(&sb, slash + 1, last_slash - slash - 1);
 		strbuf_addstr(&sb, " < ");
 		last_slash = slash;