Harden the blob view error paths

A blob requested by ref with an unknown path fell through to the
commit object and was served with a 200 instead of a 404, two error
pages passed a null pointer to a %s format when the request carried
no object id, and the error pages left the layout open.

1 file changed, 15 insertions, 7 deletions

diff --git a/source/ui-tree.c b/source/ui-tree.c
index f537442..292bfc6 100644
--- a/source/ui-tree.c
+++ b/source/ui-tree.c
@@ -101,7 +101,9 @@ static void print_binary_buffer(char *buf, unsigned long size)
 	html("</table>\n");
 }
 
-static void print_object(const struct object_id *oid, const char *path, const char *basename, const char *rev)
+/* Returns 1 if it opened the page layout for the caller to close, or 0 if
+ * it emitted a complete standalone error page. */
+static int print_object(const struct object_id *oid, const char *path, const char *basename, const char *rev)
 {
 	enum object_type type;
 	char *buf;
@@ -112,14 +114,14 @@ static void print_object(const struct object_id *oid, const char *path, const ch
 	if (type == OBJ_BAD) {
 		cgit_print_error_page(404, "Not found",
 			"Bad object name: %s", oid_to_hex(oid));
-		return;
+		return 0;
 	}
 
 	buf = odb_read_object(the_repository->objects, oid, &type, &size);
 	if (!buf) {
 		cgit_print_error_page(500, "Internal server error",
 			"Error reading object %s", oid_to_hex(oid));
-		return;
+		return 0;
 	}
 	is_binary = buffer_is_binary(buf, size);
 
@@ -139,7 +141,8 @@ static void print_object(const struct object_id *oid, const char *path, const ch
 	if (ctx.cfg.max_blob_size && size / 1024 > ctx.cfg.max_blob_size) {
 		htmlf("<div class='error'>blob size (%ldKB) exceeds display size limit (%dKB).</div>",
 				size / 1024, ctx.cfg.max_blob_size);
-		return;
+		free(buf);
+		return 1;
 	}
 
 	if (is_binary)
@@ -148,6 +151,7 @@ static void print_object(const struct object_id *oid, const char *path, const ch
 		print_text_buffer(basename, buf, size);
 
 	free(buf);
+	return 1;
 }
 
 struct single_tree_ctx {
@@ -400,8 +404,11 @@ static int walk_tree(const struct object_id *oid, struct strbuf *base,
 			ls_head();
 			return READ_TREE_RECURSIVE;
 		} else {
-			walk_tree_ctx->state = 2;
-			print_object(oid, buffer.buf, pathname, walk_tree_ctx->curr_rev);
+			/* state 2: layout left open for us to close; state 3:
+			 * print_object already emitted a standalone error page. */
+			walk_tree_ctx->state =
+				print_object(oid, buffer.buf, pathname,
+					     walk_tree_ctx->curr_rev) ? 2 : 3;
 			strbuf_release(&buffer);
 			return 0;
 		}
@@ -461,8 +468,9 @@ void cgit_print_tree(const char *rev, char *path)
 		ls_tail();
 	} else if (walk_tree_ctx.state == 2)
 		cgit_print_layout_end();
-	else
+	else if (walk_tree_ctx.state == 0)
 		cgit_print_error_page(404, "Not found", "Path not found");
+	/* state 3: print_object already emitted a complete error page */
 
 cleanup:
 	free(walk_tree_ctx.curr_rev);