Harden the page renderers

1 file changed, 35 insertions, 7 deletions

diff --git a/source/ui-blob.c b/source/ui-blob.cindex a298e54e..8407342d 100644--- a/source/ui-blob.c+++ b/source/ui-blob.c@@ -22,6 +22,18 @@ struct walk_tree_context { };  /*+ * An annotated tag names a commit through its tag object, and a path is looked+ * up in the commit's tree, not in the tag.+ */+static void peel_to_commit(struct object_id *oid)+{+	struct commit *commit = lookup_commit_reference_gently(the_repository, oid, 1);++	if (commit)+		oidcpy(oid, &commit->object.oid);+}++/*  * 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.  */@@ -30,9 +42,14 @@ static int walk_tree(const struct object_id *oid, struct strbuf *base, { 	struct walk_tree_context *walk = context; +	// Stepping into a submodule entry would have git look its commit up+	// in this repository, which does not hold it, and die. 	if (walk->file_only && !S_ISREG(mode))-		return READ_TREE_RECURSIVE;-	if (strncmp(base->buf, walk->match_path, base->len) || strcmp(walk->match_path + base->len, pathname))+		return S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0;+	if (+		strncmp(base->buf, walk->match_path, base->len) ||+		strcmp(walk->match_path + base->len, pathname)+	) 		return READ_TREE_RECURSIVE; 	oidcpy(walk->matched_oid, oid); 	walk->found_path = 1;@@ -47,6 +64,7 @@ static int walk_tree(const struct object_id *oid, struct strbuf *base, static int find_path_oid(struct object_id *oid, char *path, int file_only) { 	struct commit *commit = lookup_commit_reference(the_repository, oid);+	struct tree *tree; 	// nowildcard_len matching len makes git treat the path as literal 	// rather than as a glob. 	struct pathspec_item item = {@@ -65,7 +83,14 @@ static int find_path_oid(struct object_id *oid, char *path, int file_only) 		.file_only = file_only 	}; -	read_tree(the_repository, repo_get_commit_tree(the_repository, commit), &paths, walk_tree, &walk);+	// A commit git cannot parse, one with a broken tree line for example,+	// comes back null and has no tree to search.+	if (!commit)+		return 0;+	tree = repo_get_commit_tree(the_repository, commit);+	if (!tree)+		return 0;+	read_tree(the_repository, tree, &paths, walk_tree, &walk); 	return walk.found_path; } @@ -105,6 +130,7 @@ int cgit_print_file(char *path, const char *head, int file_only, int html_escape  	if (repo_get_oid(the_repository, head, &oid)) 		return -1;+	peel_to_commit(&oid); 	type = odb_read_object_info(the_repository->objects, &oid, &size); 	if (type == OBJ_COMMIT) { 		if (!find_path_oid(&oid, path, file_only))@@ -152,14 +178,15 @@ void cgit_print_blob(const char *hex, char *path, const char *head, int file_onl  	if (hex) { 		if (get_oid_hex(hex, &oid)) {-			cgit_print_error_page(400, "Bad Request", "Bad hex value: %s", hex);+			cgit_print_error_page(404, "Not Found", "Bad object id: %s", hex); 			return; 		} 	} else { 		if (repo_get_oid(the_repository, head, &oid)) {-			cgit_print_error_page(404, "Not Found", "Bad ref: %s", head);+			cgit_print_error_page(404, "Not Found", "Bad object id: %s", head); 			return; 		}+		peel_to_commit(&oid); 	}  	type = odb_read_object_info(the_repository->objects, &oid, &size);@@ -173,7 +200,7 @@ void cgit_print_blob(const char *hex, char *path, const char *head, int file_onl 	}  	if (type == OBJ_BAD) {-		cgit_print_error_page(404, "Not Found", "Bad object name: %s", hex ? hex : path);+		cgit_print_error_page(404, "Not Found", "Bad object id: %s", hex ? hex : path); 		return; 	} @@ -185,7 +212,8 @@ void cgit_print_blob(const char *hex, char *path, const char *head, int file_onl  	buf = odb_read_object(the_repository->objects, &oid, &type, &size); 	if (!buf) {-		cgit_print_error_page(500, "Internal Server Error", "Error reading object %s", hex ? hex : path);+		cgit_print_error_page(500, "Internal Server Error", "Unable to read object %s",+			hex ? hex : path); 		return; 	} 
diff --git a/source/ui-blob.c b/source/ui-blob.c
index a298e54e..8407342d 100644
--- a/source/ui-blob.c
+++ b/source/ui-blob.c
@@ -22,6 +22,18 @@ struct walk_tree_context {
 };
 
 /*
+ * An annotated tag names a commit through its tag object, and a path is looked
+ * up in the commit's tree, not in the tag.
+ */
+static void peel_to_commit(struct object_id *oid)
+{
+	struct commit *commit = lookup_commit_reference_gently(the_repository, oid, 1);
+
+	if (commit)
+		oidcpy(oid, &commit->object.oid);
+}
+
+/*
  * 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.
  */
@@ -30,9 +42,14 @@ static int walk_tree(const struct object_id *oid, struct strbuf *base,
 {
 	struct walk_tree_context *walk = context;
 
+	// Stepping into a submodule entry would have git look its commit up
+	// in this repository, which does not hold it, and die.
 	if (walk->file_only && !S_ISREG(mode))
-		return READ_TREE_RECURSIVE;
-	if (strncmp(base->buf, walk->match_path, base->len) || strcmp(walk->match_path + base->len, pathname))
+		return S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0;
+	if (
+		strncmp(base->buf, walk->match_path, base->len) ||
+		strcmp(walk->match_path + base->len, pathname)
+	)
 		return READ_TREE_RECURSIVE;
 	oidcpy(walk->matched_oid, oid);
 	walk->found_path = 1;
@@ -47,6 +64,7 @@ static int walk_tree(const struct object_id *oid, struct strbuf *base,
 static int find_path_oid(struct object_id *oid, char *path, int file_only)
 {
 	struct commit *commit = lookup_commit_reference(the_repository, oid);
+	struct tree *tree;
 	// nowildcard_len matching len makes git treat the path as literal
 	// rather than as a glob.
 	struct pathspec_item item = {
@@ -65,7 +83,14 @@ static int find_path_oid(struct object_id *oid, char *path, int file_only)
 		.file_only = file_only
 	};
 
-	read_tree(the_repository, repo_get_commit_tree(the_repository, commit), &paths, walk_tree, &walk);
+	// A commit git cannot parse, one with a broken tree line for example,
+	// comes back null and has no tree to search.
+	if (!commit)
+		return 0;
+	tree = repo_get_commit_tree(the_repository, commit);
+	if (!tree)
+		return 0;
+	read_tree(the_repository, tree, &paths, walk_tree, &walk);
 	return walk.found_path;
 }
 
@@ -105,6 +130,7 @@ int cgit_print_file(char *path, const char *head, int file_only, int html_escape
 
 	if (repo_get_oid(the_repository, head, &oid))
 		return -1;
+	peel_to_commit(&oid);
 	type = odb_read_object_info(the_repository->objects, &oid, &size);
 	if (type == OBJ_COMMIT) {
 		if (!find_path_oid(&oid, path, file_only))
@@ -152,14 +178,15 @@ void cgit_print_blob(const char *hex, char *path, const char *head, int file_onl
 
 	if (hex) {
 		if (get_oid_hex(hex, &oid)) {
-			cgit_print_error_page(400, "Bad Request", "Bad hex value: %s", hex);
+			cgit_print_error_page(404, "Not Found", "Bad object id: %s", hex);
 			return;
 		}
 	} else {
 		if (repo_get_oid(the_repository, head, &oid)) {
-			cgit_print_error_page(404, "Not Found", "Bad ref: %s", head);
+			cgit_print_error_page(404, "Not Found", "Bad object id: %s", head);
 			return;
 		}
+		peel_to_commit(&oid);
 	}
 
 	type = odb_read_object_info(the_repository->objects, &oid, &size);
@@ -173,7 +200,7 @@ void cgit_print_blob(const char *hex, char *path, const char *head, int file_onl
 	}
 
 	if (type == OBJ_BAD) {
-		cgit_print_error_page(404, "Not Found", "Bad object name: %s", hex ? hex : path);
+		cgit_print_error_page(404, "Not Found", "Bad object id: %s", hex ? hex : path);
 		return;
 	}
 
@@ -185,7 +212,8 @@ void cgit_print_blob(const char *hex, char *path, const char *head, int file_onl
 
 	buf = odb_read_object(the_repository->objects, &oid, &type, &size);
 	if (!buf) {
-		cgit_print_error_page(500, "Internal Server Error", "Error reading object %s", hex ? hex : path);
+		cgit_print_error_page(500, "Internal Server Error", "Unable to read object %s",
+			hex ? hex : path);
 		return;
 	}