Restyle the sources and fix the audit's findings

1 file changed, 96 insertions, 48 deletions

diff --git a/source/ui-plain.c b/source/ui-plain.c
index 8085382..9ce25b3 100644
--- a/source/ui-plain.c
+++ b/source/ui-plain.c
@@ -1,23 +1,53 @@
-/* ui-plain.c: functions for output of plain blobs by path
- *
- * 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 plain page, which hands over a repository's own bytes rather than
+ * rendering a view of them. A file is written out whole under a content type
+ * guessed from its name, though a repository that has not enabled html serving
+ * keeps only the types a browser will not act on. A directory, or a request
+ * carrying no path, is answered with a bare document of links to the entries
+ * below it rather than with one of cgit's themed pages.
  */
 
 #define USE_THE_REPOSITORY_VARIABLE
 
 #include "cgit.h"
-#include "ui-plain.h"
 #include "html.h"
+#include "shared.h"
+#include "ui-plain.h"
 #include "ui-shared.h"
 
+/*
+ * A listing is opened by the entry that matched and closed only once the walk
+ * is over, so the end of the page has to tell the three cases apart.
+ */
+enum response {
+	RESPONSE_NONE,
+	RESPONSE_BLOB,
+	RESPONSE_LISTING
+};
+
 struct walk_tree_context {
-	int match_baselen;
-	int match;
+	// Length of the directory part of the requested path, slash included,
+	// and -1 when no path was requested so that no base length can equal
+	// it.
+	int dir_len;
+	enum response response;
 };
 
+/*
+ * Everything below text/ and application/ can carry markup or script that a
+ * browser would run against the site, so only PDF is let back through.
+ */
+static int is_unsafe_type(const char *mimetype)
+{
+	return (starts_with(mimetype, "text/") ||
+		starts_with(mimetype, "application/")) &&
+		strcmp(mimetype, "application/pdf");
+}
+
+/*
+ * A nonzero return says the response has been written, error pages included,
+ * so the walk does not go on to report the path as missing.
+ */
 static int print_object(const struct object_id *oid, const char *path)
 {
 	enum object_type type;
@@ -30,8 +60,10 @@ static int print_object(const struct object_id *oid, const char *path)
 		return 1;
 	}
 
-	/* Reject an oversized object before reading it whole into memory. */
-	if (ctx.cfg.max_blob_size && size / 1024 > (unsigned long)ctx.cfg.max_blob_size) {
+	// The limit counts kilobytes and is checked before the read, so a huge
+	// blob is kept out of memory rather than noticed once it is there.
+	if (ctx.cfg.max_blob_size &&
+	    size / 1024 > (unsigned long)ctx.cfg.max_blob_size) {
 		cgit_print_error_page(413, "Too large",
 			"Object size (%luKB) exceeds limit (%dKB)",
 			size / 1024, ctx.cfg.max_blob_size);
@@ -48,13 +80,14 @@ static int print_object(const struct object_id *oid, const char *path)
 	ctx.page.mimetype = mimetype;
 
 	if (!ctx.repo->enable_html_serving) {
+		// The bytes are whatever the repository holds, so the browser
+		// is told not to guess a type of its own and not to load
+		// anything they reference. Both lines must go out before
+		// cgit_print_http_headers, which closes the header block.
 		html("X-Content-Type-Options: nosniff\n");
 		html("Content-Security-Policy: default-src 'none'\n");
-		if (mimetype) {
-			/* Built-in white list allows PDF and everything that isn't text/ and application/ */
-			if ((!strncmp(mimetype, "text/", 5) || !strncmp(mimetype, "application/", 12)) && strcmp(mimetype, "application/pdf"))
-				ctx.page.mimetype = NULL;
-		}
+		if (mimetype && is_unsafe_type(mimetype))
+			ctx.page.mimetype = NULL;
 	}
 
 	if (!ctx.page.mimetype) {
@@ -75,7 +108,7 @@ static int print_object(const struct object_id *oid, const char *path)
 	return 1;
 }
 
-static char *buildpath(const char *base, int baselen, const char *path)
+static char *build_path(const char *base, int baselen, const char *path)
 {
 	if (path[0])
 		return cgit_fmtalloc("%.*s%s/", baselen, base, path);
@@ -86,20 +119,25 @@ static char *buildpath(const char *base, int baselen, const char *path)
 static void print_dir(const struct object_id *oid, const char *base,
 		      int baselen, const char *path)
 {
-	char *fullpath, *slash;
+	char *fullpath;
+	const char *leading_slash;
 	size_t len;
 
-	fullpath = buildpath(base, baselen, path);
-	slash = (fullpath[0] == '/' ? "" : "/");
+	fullpath = build_path(base, baselen, path);
+	leading_slash = (fullpath[0] == '/' ? "" : "/");
 	ctx.page.etag = oid_to_hex(oid);
 	cgit_print_http_headers();
-	htmlf("<html><head><title>%s", slash);
+	htmlf("<html><head><title>%s", leading_slash);
 	html_txt(fullpath);
-	htmlf("</title></head>\n<body>\n<h2>%s", slash);
+	htmlf("</title></head>\n<body>\n<h2>%s", leading_slash);
 	html_txt(fullpath);
 	html("</h2>\n<ul>\n");
 	len = strlen(fullpath);
 	if (len > 1) {
+		char *slash;
+
+		// Nothing left to drop means the parent is the root, which
+		// cgit_plain_link is asked for with a null path.
 		fullpath[len - 1] = 0;
 		slash = strrchr(fullpath, '/');
 		if (slash)
@@ -121,13 +159,13 @@ static void print_dir_entry(const struct object_id *oid, const char *base,
 {
 	char *fullpath;
 
-	fullpath = buildpath(base, baselen, path);
+	fullpath = build_path(base, baselen, path);
 	if (!S_ISDIR(mode) && !S_ISGITLINK(mode))
 		fullpath[strlen(fullpath) - 1] = 0;
 	html("  <li>");
-	if (S_ISGITLINK(mode)) {
+	if (S_ISGITLINK(mode))
 		cgit_submodule_link(NULL, fullpath, oid_to_hex(oid));
-	} else
+	else
 		cgit_plain_link(path, NULL, NULL, ctx.qry.head, ctx.qry.oid,
 				fullpath);
 	html("</li>\n");
@@ -139,25 +177,27 @@ static void print_dir_tail(void)
 	html(" </ul>\n</body></html>\n");
 }
 
+/*
+ * read_tree reads the return value as a direction rather than a status, so
+ * READ_TREE_RECURSIVE means step into this entry and zero means step over it.
+ */
 static int walk_tree(const struct object_id *oid, struct strbuf *base,
-		const char *pathname, unsigned mode, void *cbdata)
+		const char *pathname, unsigned mode, void *context)
 {
-	struct walk_tree_context *walk_tree_ctx = cbdata;
+	struct walk_tree_context *walk = context;
 
-	// match_baselen is -1 when no path was given, which no length equals.
-	if (walk_tree_ctx->match_baselen >= 0 &&
-	    base->len == (size_t)walk_tree_ctx->match_baselen) {
+	if (walk->dir_len >= 0 && base->len == (size_t)walk->dir_len) {
 		if (S_ISREG(mode) || S_ISLNK(mode)) {
 			if (print_object(oid, pathname))
-				walk_tree_ctx->match = 1;
+				walk->response = RESPONSE_BLOB;
 		} else if (S_ISDIR(mode)) {
 			print_dir(oid, base->buf, base->len, pathname);
-			walk_tree_ctx->match = 2;
+			walk->response = RESPONSE_LISTING;
 			return READ_TREE_RECURSIVE;
 		}
-	} else if (base->len < INT_MAX && (int)base->len > walk_tree_ctx->match_baselen) {
+	} else if (base->len < INT_MAX && (int)base->len > walk->dir_len) {
 		print_dir_entry(oid, base->buf, base->len, pathname, mode);
-		walk_tree_ctx->match = 2;
+		walk->response = RESPONSE_LISTING;
 	} else if (S_ISDIR(mode)) {
 		return READ_TREE_RECURSIVE;
 	}
@@ -165,11 +205,11 @@ static int walk_tree(const struct object_id *oid, struct strbuf *base,
 	return 0;
 }
 
-static int basedir_len(const char *path)
+static int dir_prefix_len(const char *path)
 {
-	const char *p = strrchr(path, '/');
-	if (p)
-		return p - path + 1;
+	const char *slash = strrchr(path, '/');
+	if (slash)
+		return slash - path + 1;
 	return 0;
 }
 
@@ -178,16 +218,22 @@ void cgit_print_plain(void)
 	const char *rev = ctx.qry.oid;
 	struct object_id oid;
 	struct commit *commit;
+	int path_len = ctx.qry.path ? strlen(ctx.qry.path) : 0;
+	// A hand built pathspec leaves nowildcard_len at zero, which tells git
+	// the match may be a glob. It would then hand this walk every entry a
+	// pattern like * matches, and each one would be answered with its own
+	// set of HTTP headers inside the body of the first.
 	struct pathspec_item path_items = {
 		.match = ctx.qry.path,
-		.len = ctx.qry.path ? strlen(ctx.qry.path) : 0
+		.len = path_len,
+		.nowildcard_len = path_len
 	};
 	struct pathspec paths = {
 		.nr = 1,
 		.items = &path_items
 	};
-	struct walk_tree_context walk_tree_ctx = {
-		.match = 0
+	struct walk_tree_context walk = {
+		.response = RESPONSE_NONE
 	};
 
 	if (!rev)
@@ -203,17 +249,19 @@ void cgit_print_plain(void)
 		return;
 	}
 	if (!path_items.match) {
+		// The walk is never handed an entry for the top of the tree
+		// itself, so the listing it would have opened is opened here.
 		path_items.match = "";
-		walk_tree_ctx.match_baselen = -1;
+		walk.dir_len = -1;
 		print_dir(get_commit_tree_oid(commit), "", 0, "");
-		walk_tree_ctx.match = 2;
+		walk.response = RESPONSE_LISTING;
+	} else {
+		walk.dir_len = dir_prefix_len(path_items.match);
 	}
-	else
-		walk_tree_ctx.match_baselen = basedir_len(path_items.match);
 	read_tree(the_repository, repo_get_commit_tree(the_repository, commit),
-		  &paths, walk_tree, &walk_tree_ctx);
-	if (!walk_tree_ctx.match)
+		  &paths, walk_tree, &walk);
+	if (walk.response == RESPONSE_NONE)
 		cgit_print_error_page(404, "Not found", "Not found");
-	else if (walk_tree_ctx.match == 2)
+	else if (walk.response == RESPONSE_LISTING)
 		print_dir_tail();
 }