Restyle the sources and fix the audit's findings

1 file changed, 83 insertions, 60 deletions

diff --git a/source/ui-summary.c b/source/ui-summary.c
index 2169048..ada6b1b 100644
--- a/source/ui-summary.c
+++ b/source/ui-summary.c
@@ -1,32 +1,49 @@
-/* ui-summary.c: functions for generating repo summary page
- *
- * Copyright (C) 2006-2014 cgit Development Team <cgit@lists.zx2c4.com>
- *
- * Licensed under GNU General Public License v2
- *   (see LICENSE.txt for full license text)
+/*
+ * The repository summary page and the about page. The summary stacks the
+ * branch list, the tag list and the head of the log into one table and ends
+ * with the clone urls, reusing the listings ui-refs.c and ui-log.c draw
+ * elsewhere. The about page renders the readme that config parsing picked for
+ * the repository, either through the configured about filter or escaped as
+ * plain text, and it can serve a file sitting beside that readme so links
+ * inside the readme resolve.
  */
 
 #include "cgit.h"
-#include "ui-summary.h"
+#include "filter.h"
 #include "html.h"
+#include "shared.h"
 #include "ui-blob.h"
 #include "ui-log.h"
 #include "ui-plain.h"
 #include "ui-refs.h"
 #include "ui-shared.h"
+#include "ui-summary.h"
+
+// Age, commit message and author, the three columns a log row always has.
+// ui-log.c adds one more for each of the two optional counts, and the rows
+// this page stretches across the table have to match that width.
+#define LOG_BASE_COLUMNS 3
 
-static int urls;
+static int clone_urls_printed;
 
-static void print_url(const char *url)
+static int log_columns(void)
 {
-	int columns = 3;
+	int columns = LOG_BASE_COLUMNS;
 
 	if (ctx.repo->enable_log_filecount)
 		columns++;
 	if (ctx.repo->enable_log_linecount)
 		columns++;
+	return columns;
+}
+
+static void print_clone_url(const char *url)
+{
+	int columns = log_columns();
 
-	if (urls++ == 0) {
+	// cgit_add_clone_urls may call back no times at all, so the heading
+	// waits for a first url rather than being printed ahead of the walk.
+	if (clone_urls_printed++ == 0) {
 		htmlf("<tr class='nohover'><td colspan='%d'>&nbsp;</td></tr>", columns);
 		htmlf("<tr class='nohover'><th class='left' colspan='%d'>Clone</th></tr>\n", columns);
 	}
@@ -40,43 +57,38 @@ static void print_url(const char *url)
 	html("</a></td></tr>\n");
 }
 
-void cgit_print_summary(void)
+/*
+ * Without the separator boundary a sibling directory that merely shares the
+ * base as a name prefix, such as repo.git-backup beside repo.git, would pass.
+ */
+static int path_within(const char *base, const char *path)
 {
-	int columns = 3;
-
-	if (ctx.repo->enable_log_filecount)
-		columns++;
-	if (ctx.repo->enable_log_linecount)
-		columns++;
+	size_t len = strlen(base);
 
-	cgit_print_layout_start();
-	html("<table summary='repository info' class='list nowrap'>");
-	cgit_print_branches(ctx.cfg.summary_branches);
-	htmlf("<tr class='nohover'><td colspan='%d'>&nbsp;</td></tr>", columns);
-	cgit_print_tags(ctx.cfg.summary_tags);
-	if (ctx.cfg.summary_log > 0) {
-		htmlf("<tr class='nohover'><td colspan='%d'>&nbsp;</td></tr>", columns);
-		cgit_print_log(ctx.qry.head, 0, ctx.cfg.summary_log, NULL,
-			       NULL, NULL, 0, 0, 0);
-	}
-	urls = 0;
-	cgit_add_clone_urls(print_url);
-	html("</table>");
-	cgit_print_layout_end();
+	return starts_with(path, base) &&
+	       (path[len] == '\0' || path[len] == '/');
 }
 
-/* The caller must free the return value. */
-static char* append_readme_path(const char *filename, const char *ref, const char *path)
+/*
+ * Returns a path the caller must free, or NULL when the request cannot be
+ * served. A null ref means the readme is a file on the server's disk rather
+ * than a path inside a ref, and such a readme is confined to its own
+ * directory, so one named without a directory has nothing to confine it to
+ * and is refused.
+ */
+static char *resolve_about_path(const char *filename, const char *ref,
+				const char *path)
 {
-	char *file, *base_dir, *full_path, *resolved_base = NULL, *resolved_full = NULL;
-	/* If a subpath is specified for the about page, make it relative
-	 * to the directory containing the configured readme. */
+	char *copy, *base_dir, *full_path;
+	char *resolved_base = NULL, *resolved_full = NULL;
 
-	file = xstrdup(filename);
-	base_dir = dirname(file);
+	// dirname is allowed to write into its argument and to return a
+	// pointer into it, so base_dir borrows from a copy we keep alive.
+	copy = xstrdup(filename);
+	base_dir = dirname(copy);
 	if (!strcmp(base_dir, ".") || !strcmp(base_dir, "..")) {
 		if (!ref) {
-			free(file);
+			free(copy);
 			return NULL;
 		}
 		full_path = xstrdup(path);
@@ -86,32 +98,48 @@ static char* append_readme_path(const char *filename, const char *ref, const cha
 	if (!ref) {
 		resolved_base = realpath(base_dir, NULL);
 		resolved_full = realpath(full_path, NULL);
-		/* Require a path-separator boundary after the base so a sibling
-		 * directory that merely shares the base as a name prefix (say
-		 * repo.git-backup next to repo.git) cannot pass the check. */
 		if (!resolved_base || !resolved_full ||
-		    !starts_with(resolved_full, resolved_base) ||
-		    (resolved_full[strlen(resolved_base)] != '\0' &&
-		     resolved_full[strlen(resolved_base)] != '/')) {
+		    !path_within(resolved_base, resolved_full)) {
 			free(full_path);
 			full_path = NULL;
 		}
 	}
 
-	free(file);
+	free(copy);
 	free(resolved_base);
 	free(resolved_full);
 
 	return full_path;
 }
 
+void cgit_print_summary(void)
+{
+	int columns = log_columns();
+
+	cgit_print_layout_start();
+	html("<table summary='repository info' class='list nowrap'>");
+	cgit_print_branches(ctx.cfg.summary_branches);
+	htmlf("<tr class='nohover'><td colspan='%d'>&nbsp;</td></tr>", columns);
+	cgit_print_tags(ctx.cfg.summary_tags);
+	if (ctx.cfg.summary_log > 0) {
+		htmlf("<tr class='nohover'><td colspan='%d'>&nbsp;</td></tr>", columns);
+		cgit_print_log(ctx.qry.head, 0, ctx.cfg.summary_log, NULL,
+			       NULL, NULL, 0, 0, 0);
+	}
+	clone_urls_printed = 0;
+	cgit_add_clone_urls(print_clone_url);
+	html("</table>");
+	cgit_print_layout_end();
+}
+
 void cgit_print_repo_readme(const char *path)
 {
 	char *filename, *ref, *mimetype;
 	int free_filename = 0;
 
 	mimetype = cgit_get_mimetype_for_filename(path);
-	if (mimetype && (!strncmp(mimetype, "image/", 6) || !strncmp(mimetype, "video/", 6))) {
+	if (mimetype && (starts_with(mimetype, "image/") ||
+			 starts_with(mimetype, "video/"))) {
 		ctx.page.mimetype = mimetype;
 		ctx.page.charset = NULL;
 		cgit_print_plain();
@@ -129,21 +157,17 @@ void cgit_print_repo_readme(const char *path)
 
 	if (path) {
 		free_filename = 1;
-		filename = append_readme_path(filename, ref, path);
+		filename = resolve_about_path(filename, ref, path);
 		if (!filename)
 			goto done;
 	}
 
 	html("<div id='summary'>");
 	if (!ctx.repo->about_filter) {
-		/* No about-filter is configured, so there is nothing to turn
-		 * the readme source into safe HTML. Escape it rather than serve
-		 * repo content raw, which would let an untrusted repository
-		 * inject script into the about page. The pre keeps the line
-		 * structure of the text, which bare escaped output loses. Point
-		 * about-filter at the bundled about-render.lua to render a
-		 * markdown or man readme instead, see cgitrc.5.txt.
-		 */
+		// With no about-filter configured there is nothing to turn the
+		// readme source into safe HTML, so it is escaped rather than
+		// served raw, which would let an untrusted repository put
+		// script on this page.
 		html("<pre class='plaintext'>");
 		if (ref) {
 			cgit_print_file(filename, ref, 1, 1);
@@ -155,9 +179,8 @@ void cgit_print_repo_readme(const char *path)
 		}
 		html("</pre>");
 	} else {
-		/* An about-filter is configured and is responsible for turning
-		 * the source into safe HTML, so pass it through the filter raw.
-		 */
+		// The filter is what makes the source safe here, so it is fed
+		// through unescaped.
 		cgit_open_filter(ctx.repo->about_filter, filename);
 		if (ref)
 			cgit_print_file(filename, ref, 1, 0);