Restyle the sources and fix the audit's findings

1 file changed, 75 insertions, 72 deletions

diff --git a/source/ui-blob.c b/source/ui-blob.cindex 92edcf1..beb968b 100644--- a/source/ui-blob.c+++ b/source/ui-blob.c@@ -1,16 +1,17 @@-/* ui-blob.c: show blob content- *- * 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)+/*+ * Reads a blob out of the object database and writes its bytes to the client.+ * The blob is named either directly by object id or by a path walked out of a+ * commit's tree. The blob page serves those bytes as the whole response, under+ * headers that stop a browser treating repository content as markup, while the+ * summary page instead drops a readme's contents into a page it is already+ * building.  */  #define USE_THE_REPOSITORY_VARIABLE  #include "cgit.h"-#include "ui-blob.h" #include "html.h"+#include "ui-blob.h" #include "ui-shared.h"  struct walk_tree_context {@@ -20,93 +21,110 @@ struct walk_tree_context { 	unsigned int file_only:1; }; +/*+ * 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; -	if (walk_tree_ctx->file_only && !S_ISREG(mode))+	if (walk->file_only && !S_ISREG(mode)) 		return READ_TREE_RECURSIVE;-	if (strncmp(base->buf, walk_tree_ctx->match_path, base->len)-		|| strcmp(walk_tree_ctx->match_path + base->len, pathname))+	if (strncmp(base->buf, walk->match_path, base->len)+		|| strcmp(walk->match_path + base->len, pathname)) 		return READ_TREE_RECURSIVE;-	oidcpy(walk_tree_ctx->matched_oid, oid);-	walk_tree_ctx->found_path = 1;+	oidcpy(walk->matched_oid, oid);+	walk->found_path = 1; 	return 0; } -int cgit_ref_path_exists(const char *path, const char *ref, int file_only)+/*+ * oid comes in naming a commit and goes out naming the blob found at path,+ * which both callers rely on. The path has to be writable because a pathspec+ * item does not hold a const string.+ */+static int find_path_oid(struct object_id *oid, char *path, int file_only) {-	struct object_id oid;-	unsigned long size;-	struct pathspec_item path_items = {-		.match = xstrdup(path),-		.len = strlen(path)+	struct commit *commit = lookup_commit_reference(the_repository, oid);+	// nowildcard_len matching len makes git treat the path as literal+	// rather than as a glob.+	struct pathspec_item item = {+		.match = path,+		.len = strlen(path),+		.nowildcard_len = strlen(path) 	}; 	struct pathspec paths = { 		.nr = 1,-		.items = &path_items+		.items = &item 	};-	struct walk_tree_context walk_tree_ctx = {+	struct walk_tree_context walk = { 		.match_path = path,-		.matched_oid = &oid,+		.matched_oid = oid, 		.found_path = 0, 		.file_only = file_only 	}; +	read_tree(the_repository, repo_get_commit_tree(the_repository, commit),+		  &paths, walk_tree, &walk);+	return walk.found_path;+}++/*+ * Callers ask before reading the object, because the point of the limit is to+ * keep a huge blob out of memory rather than to notice it once it is already+ * there.+ */+static int over_size_limit(unsigned long size)+{+	return ctx.cfg.max_blob_size &&+		size / 1024 > (unsigned long)ctx.cfg.max_blob_size;+}++int cgit_ref_path_exists(const char *path, const char *ref, int file_only)+{+	struct object_id oid;+	unsigned long size;+	char *path_copy = xstrdup(path);+	int found = 0;+ 	if (repo_get_oid(the_repository, ref, &oid)) 		goto done; 	if (odb_read_object_info(the_repository->objects, &oid, &size) != OBJ_COMMIT) 		goto done;-	read_tree(the_repository,-		  repo_get_commit_tree(the_repository, lookup_commit_reference(the_repository, &oid)),-		  &paths, walk_tree, &walk_tree_ctx);+	found = find_path_oid(&oid, path_copy, file_only);  done:-	free(path_items.match);-	return walk_tree_ctx.found_path;+	free(path_copy);+	return found; }  int cgit_print_file(char *path, const char *head, int file_only, int html_escape) { 	struct object_id oid; 	enum object_type type;-	char *buf; 	unsigned long size;-	struct commit *commit;-	struct pathspec_item path_items = {-		.match = path,-		.len = strlen(path)-	};-	struct pathspec paths = {-		.nr = 1,-		.items = &path_items-	};-	struct walk_tree_context walk_tree_ctx = {-		.match_path = path,-		.matched_oid = &oid,-		.found_path = 0,-		.file_only = file_only-	};+	char *buf;  	if (repo_get_oid(the_repository, head, &oid)) 		return -1; 	type = odb_read_object_info(the_repository->objects, &oid, &size); 	if (type == OBJ_COMMIT) {-		commit = lookup_commit_reference(the_repository, &oid);-		read_tree(the_repository, repo_get_commit_tree(the_repository, commit),-			  &paths, walk_tree, &walk_tree_ctx);-		if (!walk_tree_ctx.found_path)+		if (!find_path_oid(&oid, path, file_only)) 			return -1; 		type = odb_read_object_info(the_repository->objects, &oid, &size); 	} 	if (type == OBJ_BAD) 		return -1;-	if (ctx.cfg.max_blob_size && size / 1024 > (unsigned long)ctx.cfg.max_blob_size)+	if (over_size_limit(size)) 		return -1; 	buf = odb_read_object(the_repository->objects, &oid, &type, &size); 	if (!buf) 		return -1;++	// html_txt wants a terminated string, and git leaves a spare byte+	// past every object it reads, so this write stays in the allocation. 	buf[size] = '\0'; 	if (html_escape) 		html_txt(buf);@@ -120,23 +138,8 @@ void cgit_print_blob(const char *hex, char *path, const char *head, int file_onl { 	struct object_id oid; 	enum object_type type;-	char *buf; 	unsigned long size;-	struct commit *commit;-	struct pathspec_item path_items = {-		.match = path,-		.len = path ? strlen(path) : 0-	};-	struct pathspec paths = {-		.nr = 1,-		.items = &path_items-	};-	struct walk_tree_context walk_tree_ctx = {-		.match_path = path,-		.matched_oid = &oid,-		.found_path = 0,-		.file_only = file_only-	};+	char *buf;  	if (hex) { 		if (get_oid_hex(hex, &oid)) {@@ -154,11 +157,8 @@ void cgit_print_blob(const char *hex, char *path, const char *head, int file_onl  	type = odb_read_object_info(the_repository->objects, &oid, &size); -	if ((!hex) && type == OBJ_COMMIT && path) {-		commit = lookup_commit_reference(the_repository, &oid);-		read_tree(the_repository, repo_get_commit_tree(the_repository, commit),-			  &paths, walk_tree, &walk_tree_ctx);-		if (!walk_tree_ctx.found_path) {+	if (!hex && type == OBJ_COMMIT && path) {+		if (!find_path_oid(&oid, path, file_only)) { 			cgit_print_error_page(404, "Not found", 					"Path not found: %s", path); 			return;@@ -172,8 +172,7 @@ void cgit_print_blob(const char *hex, char *path, const char *head, int file_onl 		return; 	} -	/* 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) {+	if (over_size_limit(size)) { 		cgit_print_error_page(413, "Too large", 				"Object size (%luKB) exceeds limit (%dKB)", 				size / 1024, ctx.cfg.max_blob_size);@@ -194,6 +193,10 @@ void cgit_print_blob(const char *hex, char *path, const char *head, int file_onl 		ctx.page.mimetype = "text/plain"; 	ctx.page.filename = path; +	// The bytes are whatever the repository holds, so the browser is told+	// not to guess a type of its own from them and not to load anything+	// they reference. Both 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"); 	cgit_print_http_headers();
diff --git a/source/ui-blob.c b/source/ui-blob.c
index 92edcf1..beb968b 100644
--- a/source/ui-blob.c
+++ b/source/ui-blob.c
@@ -1,16 +1,17 @@
-/* ui-blob.c: show blob content
- *
- * 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)
+/*
+ * Reads a blob out of the object database and writes its bytes to the client.
+ * The blob is named either directly by object id or by a path walked out of a
+ * commit's tree. The blob page serves those bytes as the whole response, under
+ * headers that stop a browser treating repository content as markup, while the
+ * summary page instead drops a readme's contents into a page it is already
+ * building.
  */
 
 #define USE_THE_REPOSITORY_VARIABLE
 
 #include "cgit.h"
-#include "ui-blob.h"
 #include "html.h"
+#include "ui-blob.h"
 #include "ui-shared.h"
 
 struct walk_tree_context {
@@ -20,93 +21,110 @@ struct walk_tree_context {
 	unsigned int file_only:1;
 };
 
+/*
+ * 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;
 
-	if (walk_tree_ctx->file_only && !S_ISREG(mode))
+	if (walk->file_only && !S_ISREG(mode))
 		return READ_TREE_RECURSIVE;
-	if (strncmp(base->buf, walk_tree_ctx->match_path, base->len)
-		|| strcmp(walk_tree_ctx->match_path + base->len, pathname))
+	if (strncmp(base->buf, walk->match_path, base->len)
+		|| strcmp(walk->match_path + base->len, pathname))
 		return READ_TREE_RECURSIVE;
-	oidcpy(walk_tree_ctx->matched_oid, oid);
-	walk_tree_ctx->found_path = 1;
+	oidcpy(walk->matched_oid, oid);
+	walk->found_path = 1;
 	return 0;
 }
 
-int cgit_ref_path_exists(const char *path, const char *ref, int file_only)
+/*
+ * oid comes in naming a commit and goes out naming the blob found at path,
+ * which both callers rely on. The path has to be writable because a pathspec
+ * item does not hold a const string.
+ */
+static int find_path_oid(struct object_id *oid, char *path, int file_only)
 {
-	struct object_id oid;
-	unsigned long size;
-	struct pathspec_item path_items = {
-		.match = xstrdup(path),
-		.len = strlen(path)
+	struct commit *commit = lookup_commit_reference(the_repository, oid);
+	// nowildcard_len matching len makes git treat the path as literal
+	// rather than as a glob.
+	struct pathspec_item item = {
+		.match = path,
+		.len = strlen(path),
+		.nowildcard_len = strlen(path)
 	};
 	struct pathspec paths = {
 		.nr = 1,
-		.items = &path_items
+		.items = &item
 	};
-	struct walk_tree_context walk_tree_ctx = {
+	struct walk_tree_context walk = {
 		.match_path = path,
-		.matched_oid = &oid,
+		.matched_oid = oid,
 		.found_path = 0,
 		.file_only = file_only
 	};
 
+	read_tree(the_repository, repo_get_commit_tree(the_repository, commit),
+		  &paths, walk_tree, &walk);
+	return walk.found_path;
+}
+
+/*
+ * Callers ask before reading the object, because the point of the limit is to
+ * keep a huge blob out of memory rather than to notice it once it is already
+ * there.
+ */
+static int over_size_limit(unsigned long size)
+{
+	return ctx.cfg.max_blob_size &&
+		size / 1024 > (unsigned long)ctx.cfg.max_blob_size;
+}
+
+int cgit_ref_path_exists(const char *path, const char *ref, int file_only)
+{
+	struct object_id oid;
+	unsigned long size;
+	char *path_copy = xstrdup(path);
+	int found = 0;
+
 	if (repo_get_oid(the_repository, ref, &oid))
 		goto done;
 	if (odb_read_object_info(the_repository->objects, &oid, &size) != OBJ_COMMIT)
 		goto done;
-	read_tree(the_repository,
-		  repo_get_commit_tree(the_repository, lookup_commit_reference(the_repository, &oid)),
-		  &paths, walk_tree, &walk_tree_ctx);
+	found = find_path_oid(&oid, path_copy, file_only);
 
 done:
-	free(path_items.match);
-	return walk_tree_ctx.found_path;
+	free(path_copy);
+	return found;
 }
 
 int cgit_print_file(char *path, const char *head, int file_only, int html_escape)
 {
 	struct object_id oid;
 	enum object_type type;
-	char *buf;
 	unsigned long size;
-	struct commit *commit;
-	struct pathspec_item path_items = {
-		.match = path,
-		.len = strlen(path)
-	};
-	struct pathspec paths = {
-		.nr = 1,
-		.items = &path_items
-	};
-	struct walk_tree_context walk_tree_ctx = {
-		.match_path = path,
-		.matched_oid = &oid,
-		.found_path = 0,
-		.file_only = file_only
-	};
+	char *buf;
 
 	if (repo_get_oid(the_repository, head, &oid))
 		return -1;
 	type = odb_read_object_info(the_repository->objects, &oid, &size);
 	if (type == OBJ_COMMIT) {
-		commit = lookup_commit_reference(the_repository, &oid);
-		read_tree(the_repository, repo_get_commit_tree(the_repository, commit),
-			  &paths, walk_tree, &walk_tree_ctx);
-		if (!walk_tree_ctx.found_path)
+		if (!find_path_oid(&oid, path, file_only))
 			return -1;
 		type = odb_read_object_info(the_repository->objects, &oid, &size);
 	}
 	if (type == OBJ_BAD)
 		return -1;
-	if (ctx.cfg.max_blob_size && size / 1024 > (unsigned long)ctx.cfg.max_blob_size)
+	if (over_size_limit(size))
 		return -1;
 	buf = odb_read_object(the_repository->objects, &oid, &type, &size);
 	if (!buf)
 		return -1;
+
+	// html_txt wants a terminated string, and git leaves a spare byte
+	// past every object it reads, so this write stays in the allocation.
 	buf[size] = '\0';
 	if (html_escape)
 		html_txt(buf);
@@ -120,23 +138,8 @@ void cgit_print_blob(const char *hex, char *path, const char *head, int file_onl
 {
 	struct object_id oid;
 	enum object_type type;
-	char *buf;
 	unsigned long size;
-	struct commit *commit;
-	struct pathspec_item path_items = {
-		.match = path,
-		.len = path ? strlen(path) : 0
-	};
-	struct pathspec paths = {
-		.nr = 1,
-		.items = &path_items
-	};
-	struct walk_tree_context walk_tree_ctx = {
-		.match_path = path,
-		.matched_oid = &oid,
-		.found_path = 0,
-		.file_only = file_only
-	};
+	char *buf;
 
 	if (hex) {
 		if (get_oid_hex(hex, &oid)) {
@@ -154,11 +157,8 @@ void cgit_print_blob(const char *hex, char *path, const char *head, int file_onl
 
 	type = odb_read_object_info(the_repository->objects, &oid, &size);
 
-	if ((!hex) && type == OBJ_COMMIT && path) {
-		commit = lookup_commit_reference(the_repository, &oid);
-		read_tree(the_repository, repo_get_commit_tree(the_repository, commit),
-			  &paths, walk_tree, &walk_tree_ctx);
-		if (!walk_tree_ctx.found_path) {
+	if (!hex && type == OBJ_COMMIT && path) {
+		if (!find_path_oid(&oid, path, file_only)) {
 			cgit_print_error_page(404, "Not found",
 					"Path not found: %s", path);
 			return;
@@ -172,8 +172,7 @@ void cgit_print_blob(const char *hex, char *path, const char *head, int file_onl
 		return;
 	}
 
-	/* 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) {
+	if (over_size_limit(size)) {
 		cgit_print_error_page(413, "Too large",
 				"Object size (%luKB) exceeds limit (%dKB)",
 				size / 1024, ctx.cfg.max_blob_size);
@@ -194,6 +193,10 @@ void cgit_print_blob(const char *hex, char *path, const char *head, int file_onl
 		ctx.page.mimetype = "text/plain";
 	ctx.page.filename = path;
 
+	// The bytes are whatever the repository holds, so the browser is told
+	// not to guess a type of its own from them and not to load anything
+	// they reference. Both 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");
 	cgit_print_http_headers();