[PATCH] Return HTTP status codes for error conditions for various commands
Dirk Best
mail at dirk-best.de
Fri Nov 15 15:01:25 CET 2013
This returns 404 Not found for most errors, the only exception is for the stats, where it will return 400
Bad request for unknown statistics types. Note that this patch also fixes various incomplete header errors,
which manifest themselves like this in the web server error log: "Premature end of script headers:
cgit.cgi".
Signed-off-by: Dirk Best <mail at dirk-best.de>
---
cgit.c | 33 +++++----------------------------
cmd.c | 58 ++++++++++++++++++++++++++++++++++------------------------
cmd.h | 1 -
ui-commit.c | 11 ++++++++++-
ui-diff.c | 20 +++++++++++++++++---
ui-diff.h | 2 +-
ui-patch.c | 8 ++++++++
ui-shared.c | 16 ++++++++++++++++
ui-shared.h | 3 +++
ui-stats.c | 7 +++++++
ui-summary.c | 5 +++++
ui-tag.c | 16 +++++++++++++++-
ui-tree.c | 8 ++++++++
13 files changed, 129 insertions(+), 59 deletions(-)
diff --git a/cgit.c b/cgit.c
index 861352a..f1ba6dd 100644
--- a/cgit.c
+++ b/cgit.c
@@ -575,9 +575,7 @@ static int prepare_repo_cmd(struct cgit_context *ctx)
ctx->page.title = fmtalloc("%s - %s", ctx->cfg.root_title,
"config error");
ctx->repo = NULL;
- cgit_print_http_headers(ctx);
- cgit_print_docstart(ctx);
- cgit_print_pageheader(ctx);
+ cgit_print_pagestart(ctx);
cgit_print_error("Failed to open %s: %s", name,
rc ? strerror(rc) : "Not a valid git repository");
cgit_print_docend();
@@ -594,9 +592,7 @@ static int prepare_repo_cmd(struct cgit_context *ctx)
}
if (!ctx->qry.head) {
- cgit_print_http_headers(ctx);
- cgit_print_docstart(ctx);
- cgit_print_pageheader(ctx);
+ cgit_print_pagestart(ctx);
cgit_print_error("Repository seems to be empty");
cgit_print_docend();
return 1;
@@ -605,11 +601,7 @@ static int prepare_repo_cmd(struct cgit_context *ctx)
if (get_sha1(ctx->qry.head, sha1)) {
char *tmp = xstrdup(ctx->qry.head);
ctx->qry.head = ctx->repo->defbranch;
- ctx->page.status = 404;
- ctx->page.statusmsg = "Not found";
- cgit_print_http_headers(ctx);
- cgit_print_docstart(ctx);
- cgit_print_pageheader(ctx);
+ cgit_print_pagestart_error(ctx, 404, "Not found");
cgit_print_error("Invalid branch: %s", tmp);
cgit_print_docend();
return 1;
@@ -628,11 +620,7 @@ static void process_request(void *cbdata)
cmd = cgit_get_cmd(ctx);
if (!cmd) {
ctx->page.title = "cgit error";
- ctx->page.status = 404;
- ctx->page.statusmsg = "Not found";
- cgit_print_http_headers(ctx);
- cgit_print_docstart(ctx);
- cgit_print_pageheader(ctx);
+ cgit_print_pagestart_error(ctx, 404, "Not found");
cgit_print_error("Invalid request");
cgit_print_docend();
return;
@@ -650,9 +638,7 @@ static void process_request(void *cbdata)
ctx->qry.vpath = cmd->want_vpath ? ctx->qry.path : NULL;
if (cmd->want_repo && !ctx->repo) {
- cgit_print_http_headers(ctx);
- cgit_print_docstart(ctx);
- cgit_print_pageheader(ctx);
+ cgit_print_pagestart(ctx);
cgit_print_error("No repository selected");
cgit_print_docend();
return;
@@ -661,16 +647,7 @@ static void process_request(void *cbdata)
if (ctx->repo && prepare_repo_cmd(ctx))
return;
- if (cmd->want_layout) {
- cgit_print_http_headers(ctx);
- cgit_print_docstart(ctx);
- cgit_print_pageheader(ctx);
- }
-
cmd->fn(ctx);
-
- if (cmd->want_layout)
- cgit_print_docend();
}
static int cmp_repos(const void *a, const void *b)
diff --git a/cmd.c b/cmd.c
index 0202917..d414450 100644
--- a/cmd.c
+++ b/cmd.c
@@ -39,10 +39,14 @@ static void atom_fn(struct cgit_context *ctx)
static void about_fn(struct cgit_context *ctx)
{
+ cgit_print_pagestart(ctx);
+
if (ctx->repo)
cgit_print_repo_readme(ctx->qry.path);
else
cgit_print_site_readme();
+
+ cgit_print_docend();
}
static void blob_fn(struct cgit_context *ctx)
@@ -57,12 +61,12 @@ static void commit_fn(struct cgit_context *ctx)
static void diff_fn(struct cgit_context *ctx)
{
- cgit_print_diff(ctx->qry.sha1, ctx->qry.sha2, ctx->qry.path, 1, 0);
+ cgit_print_diff(ctx->qry.sha1, ctx->qry.sha2, ctx->qry.path, 1, 0, 1);
}
static void rawdiff_fn(struct cgit_context *ctx)
{
- cgit_print_diff(ctx->qry.sha1, ctx->qry.sha2, ctx->qry.path, 1, 1);
+ cgit_print_diff(ctx->qry.sha1, ctx->qry.sha2, ctx->qry.path, 1, 1, 1);
}
static void info_fn(struct cgit_context *ctx)
@@ -72,10 +76,14 @@ static void info_fn(struct cgit_context *ctx)
static void log_fn(struct cgit_context *ctx)
{
+ cgit_print_pagestart(ctx);
+
cgit_print_log(ctx->qry.sha1, ctx->qry.ofs, ctx->cfg.max_commit_count,
ctx->qry.grep, ctx->qry.search, ctx->qry.path, 1,
ctx->repo->enable_commit_graph,
ctx->repo->commit_sort);
+
+ cgit_print_docend();
}
static void ls_cache_fn(struct cgit_context *ctx)
@@ -108,7 +116,9 @@ static void plain_fn(struct cgit_context *ctx)
static void refs_fn(struct cgit_context *ctx)
{
+ cgit_print_pagestart(ctx);
cgit_print_refs();
+ cgit_print_docend();
}
static void snapshot_fn(struct cgit_context *ctx)
@@ -137,32 +147,32 @@ static void tree_fn(struct cgit_context *ctx)
cgit_print_tree(ctx->qry.sha1, ctx->qry.path);
}
-#define def_cmd(name, want_repo, want_layout, want_vpath, is_clone) \
- {#name, name##_fn, want_repo, want_layout, want_vpath, is_clone}
+#define def_cmd(name, want_repo, want_vpath, is_clone) \
+ {#name, name##_fn, want_repo, want_vpath, is_clone}
struct cgit_cmd *cgit_get_cmd(struct cgit_context *ctx)
{
static struct cgit_cmd cmds[] = {
- def_cmd(HEAD, 1, 0, 0, 1),
- def_cmd(atom, 1, 0, 0, 0),
- def_cmd(about, 0, 1, 0, 0),
- def_cmd(blob, 1, 0, 0, 0),
- def_cmd(commit, 1, 1, 1, 0),
- def_cmd(diff, 1, 1, 1, 0),
- def_cmd(info, 1, 0, 0, 1),
- def_cmd(log, 1, 1, 1, 0),
- def_cmd(ls_cache, 0, 0, 0, 0),
- def_cmd(objects, 1, 0, 0, 1),
- def_cmd(patch, 1, 0, 1, 0),
- def_cmd(plain, 1, 0, 0, 0),
- def_cmd(rawdiff, 1, 0, 1, 0),
- def_cmd(refs, 1, 1, 0, 0),
- def_cmd(repolist, 0, 0, 0, 0),
- def_cmd(snapshot, 1, 0, 0, 0),
- def_cmd(stats, 1, 1, 1, 0),
- def_cmd(summary, 1, 1, 0, 0),
- def_cmd(tag, 1, 1, 0, 0),
- def_cmd(tree, 1, 1, 1, 0),
+ def_cmd(HEAD, 1, 0, 1),
+ def_cmd(atom, 1, 0, 0),
+ def_cmd(about, 0, 0, 0),
+ def_cmd(blob, 1, 0, 0),
+ def_cmd(commit, 1, 1, 0),
+ def_cmd(diff, 1, 1, 0),
+ def_cmd(info, 1, 0, 1),
+ def_cmd(log, 1, 1, 0),
+ def_cmd(ls_cache, 0, 0, 0),
+ def_cmd(objects, 1, 0, 1),
+ def_cmd(patch, 1, 1, 0),
+ def_cmd(plain, 1, 0, 0),
+ def_cmd(rawdiff, 1, 1, 0),
+ def_cmd(refs, 1, 0, 0),
+ def_cmd(repolist, 0, 0, 0),
+ def_cmd(snapshot, 1, 0, 0),
+ def_cmd(stats, 1, 1, 0),
+ def_cmd(summary, 1, 0, 0),
+ def_cmd(tag, 1, 0, 0),
+ def_cmd(tree, 1, 1, 0),
};
int i;
diff --git a/cmd.h b/cmd.h
index eb5bc87..8c88b32 100644
--- a/cmd.h
+++ b/cmd.h
@@ -7,7 +7,6 @@ struct cgit_cmd {
const char *name;
cgit_cmd_fn fn;
unsigned int want_repo:1,
- want_layout:1,
want_vpath:1,
is_clone:1;
};
diff --git a/ui-commit.c b/ui-commit.c
index ef85a49..d29039d 100644
--- a/ui-commit.c
+++ b/ui-commit.c
@@ -27,14 +27,21 @@ void cgit_print_commit(char *hex, const char *prefix)
hex = ctx.qry.head;
if (get_sha1(hex, sha1)) {
+ cgit_print_pagestart_error(&ctx, 404, "Not found");
cgit_print_error("Bad object id: %s", hex);
+ cgit_print_docend();
return;
}
commit = lookup_commit_reference(sha1);
if (!commit) {
+ cgit_print_pagestart_error(&ctx, 404, "Not found");
cgit_print_error("Bad commit reference: %s", hex);
+ cgit_print_docend();
return;
}
+
+ cgit_print_pagestart(&ctx);
+
info = cgit_parse_commit(commit);
format_display_notes(sha1, ¬es, PAGE_ENCODING, 0);
@@ -137,8 +144,10 @@ void cgit_print_commit(char *hex, const char *prefix)
tmp = sha1_to_hex(commit->parents->item->object.sha1);
else
tmp = NULL;
- cgit_print_diff(ctx.qry.sha1, tmp, prefix, 0, 0);
+ cgit_print_diff(ctx.qry.sha1, tmp, prefix, 0, 0, 0);
}
strbuf_release(¬es);
cgit_free_commitinfo(info);
+
+ cgit_print_docend();
}
diff --git a/ui-diff.c b/ui-diff.c
index 1209c47..000c794 100644
--- a/ui-diff.c
+++ b/ui-diff.c
@@ -358,25 +358,31 @@ void cgit_print_diff_ctrls()
}
void cgit_print_diff(const char *new_rev, const char *old_rev,
- const char *prefix, int show_ctrls, int raw)
+ const char *prefix, int show_ctrls, int raw, int full_page)
{
struct commit *commit, *commit2;
if (!new_rev)
new_rev = ctx.qry.head;
if (get_sha1(new_rev, new_rev_sha1)) {
+ cgit_print_pagestart_error(&ctx, 404, "Not found");
cgit_print_error("Bad object name: %s", new_rev);
- return;
+ cgit_print_docend();
+ return;
}
commit = lookup_commit_reference(new_rev_sha1);
if (!commit || parse_commit(commit)) {
+ cgit_print_pagestart_error(&ctx, 404, "Not found");
cgit_print_error("Bad commit: %s", sha1_to_hex(new_rev_sha1));
- return;
+ cgit_print_docend();
+ return;
}
if (old_rev) {
if (get_sha1(old_rev, old_rev_sha1)) {
+ cgit_print_pagestart_error(&ctx, 404, "Not found");
cgit_print_error("Bad object name: %s", old_rev);
+ cgit_print_docend();
return;
}
} else if (commit->parents && commit->parents->item) {
@@ -388,7 +394,9 @@ void cgit_print_diff(const char *new_rev, const char *old_rev,
if (!is_null_sha1(old_rev_sha1)) {
commit2 = lookup_commit_reference(old_rev_sha1);
if (!commit2 || parse_commit(commit2)) {
+ cgit_print_pagestart_error(&ctx, 404, "Not found");
cgit_print_error("Bad commit: %s", sha1_to_hex(old_rev_sha1));
+ cgit_print_docend();
return;
}
}
@@ -401,6 +409,9 @@ void cgit_print_diff(const char *new_rev, const char *old_rev,
return;
}
+ if (full_page)
+ cgit_print_pagestart(&ctx);
+
use_ssdiff = ctx.qry.has_ssdiff ? ctx.qry.ssdiff : ctx.cfg.ssdiff;
if (show_ctrls)
@@ -419,4 +430,7 @@ void cgit_print_diff(const char *new_rev, const char *old_rev,
if (!use_ssdiff)
html("</td></tr>");
html("</table>");
+
+ if (full_page)
+ cgit_print_docend();
}
diff --git a/ui-diff.h b/ui-diff.h
index 04f9029..dac7a59 100644
--- a/ui-diff.h
+++ b/ui-diff.h
@@ -4,7 +4,7 @@
extern void cgit_print_diff_ctrls();
extern void cgit_print_diff(const char *new_hex, const char *old_hex,
- const char *prefix, int show_ctrls, int raw);
+ const char *prefix, int show_ctrls, int raw, int full_page);
extern struct diff_filespec *cgit_get_current_old_file(void);
extern struct diff_filespec *cgit_get_current_new_file(void);
diff --git a/ui-patch.c b/ui-patch.c
index 4d35167..9aec0a3 100644
--- a/ui-patch.c
+++ b/ui-patch.c
@@ -25,22 +25,30 @@ void cgit_print_patch(const char *new_rev, const char *old_rev,
new_rev = ctx.qry.head;
if (get_sha1(new_rev, new_rev_sha1)) {
+ cgit_print_pagestart_error(&ctx, 404, "Not found");
cgit_print_error("Bad object id: %s", new_rev);
+ cgit_print_docend();
return;
}
commit = lookup_commit_reference(new_rev_sha1);
if (!commit) {
+ cgit_print_pagestart_error(&ctx, 404, "Not found");
cgit_print_error("Bad commit reference: %s", new_rev);
+ cgit_print_docend();
return;
}
if (old_rev) {
if (get_sha1(old_rev, old_rev_sha1)) {
+ cgit_print_pagestart_error(&ctx, 404, "Not found");
cgit_print_error("Bad object id: %s", old_rev);
+ cgit_print_docend();
return;
}
if (!lookup_commit_reference(old_rev_sha1)) {
+ cgit_print_pagestart_error(&ctx, 404, "Not found");
cgit_print_error("Bad commit reference: %s", old_rev);
+ cgit_print_docend();
return;
}
} else if (commit->parents && commit->parents->item) {
diff --git a/ui-shared.c b/ui-shared.c
index 1e19421..2c0cac5 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -696,6 +696,22 @@ void cgit_print_docstart(struct cgit_context *ctx)
html_include(ctx->cfg.header);
}
+void cgit_print_pagestart(struct cgit_context *ctx)
+{
+ cgit_print_http_headers(ctx);
+ cgit_print_docstart(ctx);
+ cgit_print_pageheader(ctx);
+}
+
+void cgit_print_pagestart_error(struct cgit_context *ctx, int status, const char *reason)
+{
+ ctx->page.status = status;
+ ctx->page.statusmsg = reason;
+ cgit_print_http_headers(ctx);
+ cgit_print_docstart(ctx);
+ cgit_print_pageheader(ctx);
+}
+
void cgit_print_docend()
{
html("</div> <!-- class=content -->\n");
diff --git a/ui-shared.h b/ui-shared.h
index a337dce..4110f83 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -60,6 +60,9 @@ extern void cgit_print_date(time_t secs, const char *format, int local_time);
extern void cgit_print_age(time_t t, time_t max_relative, const char *format);
extern void cgit_print_http_headers(struct cgit_context *ctx);
extern void cgit_print_docstart(struct cgit_context *ctx);
+extern void cgit_print_pagestart(struct cgit_context *ctx);
+extern void cgit_print_pagestart_error(struct cgit_context *ctx, int status,
+ const char *reason);
extern void cgit_print_docend();
extern void cgit_print_pageheader(struct cgit_context *ctx);
extern void cgit_print_filemode(unsigned short mode);
diff --git a/ui-stats.c b/ui-stats.c
index 28b794f..cefea9f 100644
--- a/ui-stats.c
+++ b/ui-stats.c
@@ -374,9 +374,14 @@ void cgit_show_stats(struct cgit_context *ctx)
i = cgit_find_stats_period(code, &period);
if (!i) {
+ cgit_print_pagestart_error(ctx, 400, "Bad request");
cgit_print_error("Unknown statistics type: %c", code[0]);
+ cgit_print_docend();
return;
}
+
+ cgit_print_pagestart(ctx);
+
if (i > ctx->repo->max_stats) {
cgit_print_error("Statistics type disabled: %s", period->name);
return;
@@ -423,5 +428,7 @@ void cgit_show_stats(struct cgit_context *ctx)
}
html("</h2>");
print_authors(&authors, top, period);
+
+ cgit_print_docend();
}
diff --git a/ui-summary.c b/ui-summary.c
index 5598d08..8c15980 100644
--- a/ui-summary.c
+++ b/ui-summary.c
@@ -13,6 +13,7 @@
#include "ui-log.h"
#include "ui-refs.h"
#include "ui-blob.h"
+#include "ui-shared.h"
#include <libgen.h>
static void print_url(char *base, char *suffix)
@@ -80,6 +81,8 @@ void cgit_print_summary()
if (ctx.repo->enable_log_linecount)
columns++;
+ cgit_print_pagestart(&ctx);
+
html("<table summary='repository info' class='list nowrap'>");
cgit_print_branches(ctx.cfg.summary_branches);
htmlf("<tr class='nohover'><td colspan='%d'> </td></tr>", columns);
@@ -94,6 +97,8 @@ void cgit_print_summary()
else if (ctx.cfg.clone_prefix)
print_urls(ctx.cfg.clone_prefix, ctx.repo->url);
html("</table>");
+
+ cgit_print_docend();
}
/* The caller must free the return value. */
diff --git a/ui-tag.c b/ui-tag.c
index aea7958..103f9be 100644
--- a/ui-tag.c
+++ b/ui-tag.c
@@ -44,7 +44,7 @@ void cgit_print_tag(char *revname)
struct strbuf fullref = STRBUF_INIT;
unsigned char sha1[20];
struct object *obj;
- struct tag *tag;
+ struct tag *tag = NULL;
struct taginfo *info;
if (!revname)
@@ -52,20 +52,32 @@ void cgit_print_tag(char *revname)
strbuf_addf(&fullref, "refs/tags/%s", revname);
if (get_sha1(fullref.buf, sha1)) {
+ cgit_print_pagestart_error(&ctx, 404, "Not found");
cgit_print_error("Bad tag reference: %s", revname);
+ cgit_print_docend();
goto cleanup;
}
obj = parse_object(sha1);
if (!obj) {
+ cgit_print_pagestart_error(&ctx, 404, "Not found");
cgit_print_error("Bad object id: %s", sha1_to_hex(sha1));
+ cgit_print_docend();
goto cleanup;
}
+
if (obj->type == OBJ_TAG) {
tag = lookup_tag(sha1);
if (!tag || parse_tag(tag) || !(info = cgit_parse_tag(tag))) {
+ cgit_print_pagestart_error(&ctx, 404, "Not found");
cgit_print_error("Bad tag object: %s", revname);
+ cgit_print_docend();
goto cleanup;
}
+ }
+
+ cgit_print_pagestart(&ctx);
+
+ if (tag) {
html("<table class='commit-info'>\n");
htmlf("<tr><td>tag name</td><td>");
html_txt(revname);
@@ -104,6 +116,8 @@ void cgit_print_tag(char *revname)
html("</table>\n");
}
+ cgit_print_docend();
+
cleanup:
strbuf_release(&fullref);
}
diff --git a/ui-tree.c b/ui-tree.c
index aa5dee9..0074a69 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -271,15 +271,21 @@ void cgit_print_tree(const char *rev, char *path)
rev = ctx.qry.head;
if (get_sha1(rev, sha1)) {
+ cgit_print_pagestart_error(&ctx, 404, "Not found");
cgit_print_error("Invalid revision name: %s", rev);
+ cgit_print_docend();
return;
}
commit = lookup_commit_reference(sha1);
if (!commit || parse_commit(commit)) {
+ cgit_print_pagestart_error(&ctx, 404, "Not found");
cgit_print_error("Invalid commit reference: %s", rev);
+ cgit_print_docend();
return;
}
+ cgit_print_pagestart(&ctx);
+
walk_tree_ctx.curr_rev = xstrdup(rev);
if (path == NULL) {
@@ -293,4 +299,6 @@ void cgit_print_tree(const char *rev, char *path)
cleanup:
free(walk_tree_ctx.curr_rev);
+
+ cgit_print_docend();
}
--
1.8.3.2
More information about the CGit
mailing list