Harden the page renderers

1 file changed, 42 insertions, 45 deletions

diff --git a/source/ui-atom.c b/source/ui-atom.cindex 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));
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));