Harden the page renderers

1 file changed, 42 insertions, 45 deletions

diff --git a/source/ui-atom.c b/source/ui-atom.c
index 46039a0..b3265b7 100644
--- a/source/ui-atom.c
+++ b/source/ui-atom.c
@@ -14,6 +14,9 @@
 #include "ui-atom.h"
 #include "ui-shared.h"
 
+// Stands in for every byte the feed cannot carry.
+#define XML_REPLACEMENT "?"
+
 /*
  * Atom timestamps have to be RFC 3339, so a feed ignores the date-format and
  * local-time settings the browsable pages honour. The zero is the timezone
@@ -24,9 +27,6 @@ static const char *feed_date(timestamp_t when)
 	return show_date(when, 0, date_mode_from_type(DATE_ISO8601_STRICT));
 }
 
-// Stands in for every byte the feed cannot carry.
-#define XML_REPLACEMENT "?"
-
 /*
  * How many bytes the UTF-8 sequence at p holds, or 0 when invalid. The
  * lead-byte ranges fold in the overlong, surrogate and out-of-range cases,
@@ -130,7 +130,8 @@ static void print_email(const char *email)
 static void print_entry(struct commit *commit, const char *host)
 {
 	struct commitinfo *info;
-	char *hex;
+	char *hex, *pageurl;
+	char delim = '&';
 
 	info = cgit_parse_commit(commit);
 	hex = oid_to_hex(&commit->object.oid);
@@ -158,24 +159,18 @@ static void print_entry(struct commit *commit, const char *host)
 	html("<published>");
 	xml_txt(feed_date(info->author_date));
 	html("</published>\n");
-	{
-		char *pageurl;
-		char delim = '&';
-
-		html("<link rel='alternate' type='text/html' href='");
-		html(cgit_httpscheme());
-		html_attr(host);
-		pageurl = cgit_pageurl(ctx.repo->url, "commit", NULL);
-		html_attr(pageurl);
-		// Without a virtual root the page url is already a query
-		// string, so the commit id continues it instead of opening
-		// one.
-		if (ctx.cfg.virtual_root)
-			delim = '?';
-		html_attrf("%cid=%s", delim, hex);
-		html("'/>\n");
-		free(pageurl);
-	}
+	html("<link rel='alternate' type='text/html' href='");
+	html(cgit_httpscheme());
+	html_attr(host);
+	pageurl = cgit_pageurl(ctx.repo->url, "commit", NULL);
+	html_attr(pageurl);
+	// Without a virtual root the page url is already a query string, so
+	// the commit id continues it instead of opening one.
+	if (ctx.cfg.virtual_root)
+		delim = '?';
+	html_attrf("%cid=%s", delim, hex);
+	html("'/>\n");
+	free(pageurl);
 	html("<id>");
 	html_txtf("urn:%s:%s", the_hash_algo->name, hex);
 	html("</id>\n");
@@ -188,12 +183,13 @@ static void print_entry(struct commit *commit, const char *host)
 
 void cgit_print_atom(char *tip, const char *path, int max_count)
 {
-	char *host;
+	char *host, *fullurl, *repourl;
 	// setup_revisions reads a command line, so the first slot is the
 	// unused program name and parsing starts at the second.
-	const char *argv[] = {NULL, tip, NULL, NULL, NULL};
+	const char *argv[] = { NULL, tip, NULL, NULL, NULL };
 	struct commit *commit;
 	struct rev_info rev;
+	struct strbuf idbuf = STRBUF_INIT;
 	int argc = 2;
 	bool need_updated = true;
 
@@ -214,7 +210,12 @@ void cgit_print_atom(char *tip, const char *path, int max_count)
 	rev.show_root_diff = 0;
 	rev.max_count = max_count;
 	setup_revisions(argc, argv, &rev, NULL);
-	prepare_revision_walk(&rev);
+	// A failed setup leaves the walk holding freed commits, so it must
+	// not be read from.
+	if (prepare_revision_walk(&rev)) {
+		cgit_print_error_page(500, "Internal Server Error", "Unable to read the history");
+		return;
+	}
 
 	// CGI guarantees a server name, so only a bare test run reaches the
 	// fallback, which keeps the mandatory feed id and links present.
@@ -240,26 +241,22 @@ void cgit_print_atom(char *tip, const char *path, int max_count)
 	html("<subtitle>");
 	xml_txt(ctx.repo->desc);
 	html("</subtitle>\n");
-	{
-		char *fullurl = cgit_currentfullurl();
-		char *repourl = cgit_repourl(ctx.repo->url);
-		struct strbuf idbuf = STRBUF_INIT;
-
-		strbuf_addf(&idbuf, "%s%s%s", cgit_httpscheme(), host, fullurl);
-		html("<id>");
-		xml_txt(idbuf.buf);
-		html("</id>\n");
-		strbuf_release(&idbuf);
-		html("<link rel='self' href='");
-		html_attrf("%s%s%s", cgit_httpscheme(), host, fullurl);
-		html("'/>\n");
-		html("<link rel='alternate' type='text/html' href='");
-		html_attrf("%s%s%s", cgit_httpscheme(), host, repourl);
-		html("'/>\n");
-		free(fullurl);
-		free(repourl);
-	}
-	while ((commit = get_revision(&rev)) != NULL) {
+	fullurl = cgit_currentfullurl();
+	repourl = cgit_repourl(ctx.repo->url);
+	strbuf_addf(&idbuf, "%s%s%s", cgit_httpscheme(), host, fullurl);
+	html("<id>");
+	xml_txt(idbuf.buf);
+	html("</id>\n");
+	strbuf_release(&idbuf);
+	html("<link rel='self' href='");
+	html_attrf("%s%s%s", cgit_httpscheme(), host, fullurl);
+	html("'/>\n");
+	html("<link rel='alternate' type='text/html' href='");
+	html_attrf("%s%s%s", cgit_httpscheme(), host, repourl);
+	html("'/>\n");
+	free(fullurl);
+	free(repourl);
+	while ((commit = get_revision(&rev))) {
 		if (need_updated) {
 			html("<updated>");
 			xml_txt(feed_date(commit->date));