Diff options
| author | Bryce Kwon <bryce@brycekwon.com> | |
|---|---|---|
| committer | Bryce Kwon <bryce@brycekwon.com> | |
| commit | ||
| parent | ||
| tree | ||
| download | ||
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.
| -rw-r--r-- | source/ui-blame.c | 5 | |
| -rw-r--r-- | source/ui-blob.c | 9 | |
| -rw-r--r-- | source/ui-plain.c | 4 | |
| -rw-r--r-- | source/ui-tree.c | 22 |
4 files changed, 27 insertions, 13 deletions
diff --git a/source/ui-blame.c b/source/ui-blame.c index 5c6f36e..1b9df88 100644 --- a/source/ui-blame.c +++ b/source/ui-blame.c @@ -229,9 +229,10 @@ static void print_object(const struct object_id *oid, const char *path, html("</tr>\n</table>\n"); - cgit_print_layout_end(); - cleanup: + /* The binary and oversized branches jump here with the layout still + * open, so close it on every path rather than only the normal one. */ + cgit_print_layout_end(); free(buf); } diff --git a/source/ui-blob.c b/source/ui-blob.c index 7720a28..ad84f3d 100644 --- a/source/ui-blob.c +++ b/source/ui-blob.c @@ -156,19 +156,24 @@ void cgit_print_blob(const char *hex, char *path, const char *head, int file_onl 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) { + cgit_print_error_page(404, "Not found", + "Path not found: %s", path); + return; + } type = odb_read_object_info(the_repository->objects, &oid, &size); } if (type == OBJ_BAD) { cgit_print_error_page(404, "Not found", - "Bad object name: %s", hex); + "Bad object name: %s", hex ? hex : path); return; } 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); + "Error reading object %s", hex ? hex : path); return; } diff --git a/source/ui-plain.c b/source/ui-plain.c index a2a4087..c041711 100644 --- a/source/ui-plain.c +++ b/source/ui-plain.c @@ -27,13 +27,13 @@ static int print_object(const struct object_id *oid, const char *path) type = odb_read_object_info(the_repository->objects, oid, &size); if (type == OBJ_BAD) { cgit_print_error_page(404, "Not found", "Not found"); - return 0; + return 1; } buf = odb_read_object(the_repository->objects, oid, &type, &size); if (!buf) { cgit_print_error_page(404, "Not found", "Not found"); - return 0; + return 1; } mimetype = get_mimetype_for_filename(path); 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); |