Download raw body.
gotwebd: reply with non-200 HTTP status code
On March 11, 2023 4:16:30 AM MST, Omar Polo <op@omarpolo.com> wrote:
>Here's the second half of the previous diff; this makes gotwebd return
>400 when it encounters an error processing the request. This will
>make scripts, bots, crawlers and whatnot aware that the "gotwebd
>experienced an error" page is, in fact, an error page and maybe stop
>issuing requests for that URL.
>
>It also cleans the `goto render'/`goto done'/`goto err' madness: only
>the `err' label remains and it's used to render the error page. (Note
>that some `goto' were wrong before.)
>
>If an error occurs after gotweb_reply() was called, we can't do
>anything about it and just `return'. All the allocations are done on
>the struct transport that is free'd when tearing down the connection,
>so no risk of leaking resources. (this because we can only signal an
>error *before* sending the body of the page.)
>
>Furthermore, since we don't need to track whether we've replied or
>not, the error page can be consistently be in HTML and so I've moved
>it to a template while here.
>
>regarding the HTTP error: there are two kinds of errors
>
> - 4XX: the client request is "bad" (the path does not exists, wrong
> querystring, ...)
>
> - 5XX: an internal server error handling the request (ENOMEM and
> other run-time failures.)
>
>It's not clear to me how map a `struct got_error' to an HTTP error, so
>I've picked a generic 400 "Bad Request" and went with it. Should be
>"good enough"™ in practice.
>
>As usual, I've updated my instance with this. You can tell, for
>example, because now:
>
> % curl --head 'https://got.omarpolo.com/?action=summary&path=got.git'
> HTTP/1.1 200 OK
> [...]
>
>while a non-existant foo.git yields
>
> % curl --head 'https://got.omarpolo.com/?action=summary&path=foo.git'
> HTTP/1.1 400 Bad Request
> [...]
>
>ok?
I've only read this, so I'll trust you've tested. Ok
>
>diffstat /home/op/w/got
> M gotwebd/gotweb.c | 65+ 84-
> M gotwebd/gotwebd.h | 2+ 0-
> M gotwebd/pages.tmpl | 14+ 0-
>
>3 files changed, 81 insertions(+), 84 deletions(-)
>
>diff /home/op/w/got
>commit - df2d3cd2545e0a1579ce83ae137e52135755ed1f
>path + /home/op/w/got
>blob - ca03025fe2d5b211ca540d4ee3b9410d8c0ea611
>file + gotwebd/gotweb.c
>--- gotwebd/gotweb.c
>+++ gotwebd/gotweb.c
>@@ -143,12 +143,10 @@ gotweb_process_request(struct request *c)
> void
> gotweb_process_request(struct request *c)
> {
>- const struct got_error *error = NULL, *error2 = NULL;
>+ const struct got_error *error = NULL;
> struct server *srv = NULL;
> struct querystring *qs = NULL;
> struct repo_dir *repo_dir = NULL;
>- uint8_t err[] = "gotwebd experienced an error: ";
>- int r, html = 0;
>
> /* init the transport */
> error = gotweb_init_transport(&c->t);
>@@ -188,15 +186,15 @@ gotweb_process_request(struct request *c)
> if (qs->action == BLAME || qs->action == BLOB ||
> qs->action == BLOBRAW || qs->action == DIFF) {
> if (qs->commit == NULL) {
>- error2 = got_error(GOT_ERR_QUERYSTRING);
>- goto render;
>+ error = got_error(GOT_ERR_QUERYSTRING);
>+ goto err;
> }
> }
>
> if (qs->action != INDEX) {
> error = gotweb_init_repo_dir(&repo_dir, qs->path);
> if (error)
>- goto done;
>+ goto err;
> error = gotweb_load_got_path(c, repo_dir);
> c->t->repo_dir = repo_dir;
> if (error && error->code != GOT_ERR_LONELY_PACKIDX)
>@@ -210,12 +208,12 @@ gotweb_process_request(struct request *c)
>
> error = got_get_repo_commits(c, 1);
> if (error)
>- goto done;
>+ goto err;
>
>- error2 = got_open_blob_for_output(&c->t->blob, &c->t->fd,
>+ error = got_open_blob_for_output(&c->t->blob, &c->t->fd,
> &binary, c);
>- if (error2)
>- goto render;
>+ if (error)
>+ goto err;
>
> if (binary)
> r = gotweb_reply_file(c, "application/octet-stream",
>@@ -223,20 +221,20 @@ gotweb_process_request(struct request *c)
> else
> r = gotweb_reply(c, 200, "text/plain", NULL);
> if (r == -1)
>- goto done;
>+ return;
>
> for (;;) {
> error = got_object_blob_read_block(&len, c->t->blob);
> if (error)
>- goto done;
>+ break;
> if (len == 0)
> break;
> buf = got_object_blob_get_read_buf(c->t->blob);
> if (fcgi_gen_binary_response(c, buf, len) == -1)
>- goto done;
>+ break;
> }
>
>- goto done;
>+ return;
> }
>
> if (qs->action == BLOB) {
>@@ -253,15 +251,15 @@ gotweb_process_request(struct request *c)
>
> error = got_get_repo_commits(c, 1);
> if (error)
>- goto done;
>+ goto err;
>
>- error2 = got_open_blob_for_output(&c->t->blob, &c->t->fd,
>+ error = got_open_blob_for_output(&c->t->blob, &c->t->fd,
> &binary, c);
>- if (error2)
>- goto render;
>+ if (error)
>+ goto err;
> if (binary) {
> gotweb_reply(c, 302, NULL, &url);
>- goto done;
>+ return;
> }
> }
>
>@@ -269,28 +267,17 @@ gotweb_process_request(struct request *c)
> const char *ctype = "application/rss+xml;charset=utf-8";
>
> if (gotweb_reply_file(c, ctype, repo_dir->name, ".rss") == -1)
>- goto done;
>+ return;
>
> error = got_get_repo_tags(c, D_MAXSLCOMMDISP);
> if (error) {
> log_warnx("%s: %s", __func__, error->msg);
>- goto err;
>+ return;
> }
>- if (gotweb_render_rss(c->tp) == -1)
>- goto err;
>- goto done;
>+ gotweb_render_rss(c->tp);
>+ return;
> }
>
>-render:
>- if (gotweb_reply(c, 200, "text/html", NULL) == -1)
>- goto done;
>- html = 1;
>-
>- if (error2) {
>- error = error2;
>- goto err;
>- }
>-
> switch(qs->action) {
> case BLAME:
> error = got_get_repo_commits(c, 1);
>@@ -298,26 +285,30 @@ render:
> log_warnx("%s: %s", __func__, error->msg);
> goto err;
> }
>- if (gotweb_render_page(c->tp, gotweb_render_blame) == -1)
>- goto done;
>- break;
>+ if (gotweb_reply(c, 200, "text/html", NULL) == -1)
>+ return;
>+ gotweb_render_page(c->tp, gotweb_render_blame);
>+ return;
> case BLOB:
>- if (gotweb_render_page(c->tp, gotweb_render_blob) == -1)
>- goto err;
>- break;
>+ if (gotweb_reply(c, 200, "text/html", NULL) == -1)
>+ return;
>+ gotweb_render_page(c->tp, gotweb_render_blob);
>+ return;
> case BRIEFS:
>- if (gotweb_render_page(c->tp, gotweb_render_briefs) == -1)
>- goto err;
>- break;
>+ if (gotweb_reply(c, 200, "text/html", NULL) == -1)
>+ return;
>+ gotweb_render_page(c->tp, gotweb_render_briefs);
>+ return;
> case COMMITS:
> error = got_get_repo_commits(c, srv->max_commits_display);
> if (error) {
> log_warnx("%s: %s", __func__, error->msg);
> goto err;
> }
>- if (gotweb_render_page(c->tp, gotweb_render_commits) == -1)
>- goto err;
>- break;
>+ if (gotweb_reply(c, 200, "text/html", NULL) == -1)
>+ return;
>+ gotweb_render_page(c->tp, gotweb_render_commits);
>+ return;
> case DIFF:
> error = got_get_repo_commits(c, 1);
> if (error) {
>@@ -329,9 +320,10 @@ render:
> log_warnx("%s: %s", __func__, error->msg);
> goto err;
> }
>- if (gotweb_render_page(c->tp, gotweb_render_diff) == -1)
>- goto err;
>- break;
>+ if (gotweb_reply(c, 200, "text/html", NULL) == -1)
>+ return;
>+ gotweb_render_page(c->tp, gotweb_render_diff);
>+ return;
> case INDEX:
> c->t->nrepos = scandir(srv->repos_path, &c->t->repos, NULL,
> alphasort);
>@@ -341,9 +333,10 @@ render:
> srv->repos_path);
> goto err;
> }
>- if (gotweb_render_page(c->tp, gotweb_render_index) == -1)
>- goto err;
>- break;
>+ if (gotweb_reply(c, 200, "text/html", NULL) == -1)
>+ return;
>+ gotweb_render_page(c->tp, gotweb_render_index);
>+ return;
> case SUMMARY:
> error = got_ref_list(&c->t->refs, c->t->repo, "refs/heads",
> got_ref_cmp_by_name, NULL);
>@@ -360,9 +353,10 @@ render:
> goto err;
> }
> qs->action = SUMMARY;
>- if (gotweb_render_page(c->tp, gotweb_render_summary) == -1)
>- goto done;
>- break;
>+ if (gotweb_reply(c, 200, "text/html", NULL) == -1)
>+ return;
>+ gotweb_render_page(c->tp, gotweb_render_summary);
>+ return;
> case TAG:
> error = got_get_repo_tags(c, 1);
> if (error) {
>@@ -374,53 +368,40 @@ render:
> "bad commit id");
> goto err;
> }
>- if (gotweb_render_page(c->tp, gotweb_render_tag) == -1)
>- goto done;
>- break;
>+ if (gotweb_reply(c, 200, "text/html", NULL) == -1)
>+ return;
>+ gotweb_render_page(c->tp, gotweb_render_tag);
>+ return;
> case TAGS:
> error = got_get_repo_tags(c, srv->max_commits_display);
> if (error) {
> log_warnx("%s: %s", __func__, error->msg);
> goto err;
> }
>- if (gotweb_render_page(c->tp, gotweb_render_tags) == -1)
>- goto done;
>- break;
>+ if (gotweb_reply(c, 200, "text/html", NULL) == -1)
>+ return;
>+ gotweb_render_page(c->tp, gotweb_render_tags);
>+ return;
> case TREE:
> error = got_get_repo_commits(c, 1);
> if (error) {
> log_warnx("%s: %s", __func__, error->msg);
> goto err;
> }
>- if (gotweb_render_page(c->tp, gotweb_render_tree) == -1)
>- goto err;
>- break;
>+ if (gotweb_reply(c, 200, "text/html", NULL) == -1)
>+ return;
>+ gotweb_render_page(c->tp, gotweb_render_tree);
>+ return;
> case ERR:
> default:
>- r = fcgi_printf(c, "<div id='err_content'>%s</div>\n",
>- "Erorr: Bad Querystring");
>- if (r == -1)
>- goto err;
>- break;
>+ error = got_error(GOT_ERR_BAD_QUERYSTRING);
> }
>
>- goto done;
> err:
>- if (html && fcgi_printf(c, "<div id='err_content'>") == -1)
>+ c->t->error = error;
>+ if (gotweb_reply(c, 400, "text/html", NULL) == -1)
> return;
>- if (fcgi_printf(c, "\n%s", err) == -1)
>- return;
>- if (error) {
>- if (fcgi_printf(c, "%s", error->msg) == -1)
>- return;
>- } else {
>- if (fcgi_printf(c, "see daemon logs for details") == -1)
>- return;
>- }
>- if (html && fcgi_printf(c, "</div>\n") == -1)
>- return;
>-done:
>- return;
>+ gotweb_render_page(c->tp, gotweb_render_error);
> }
>
> struct server *
>blob - 56c7eb962abf8757106cc2b4836d502c6a6655e9
>file + gotwebd/gotwebd.h
>--- gotwebd/gotwebd.h
>+++ gotwebd/gotwebd.h
>@@ -197,6 +197,7 @@ struct transport {
> unsigned int next_disp;
> unsigned int prev_disp;
> unsigned int tag_count;
>+ const struct got_error *error;
> struct got_blob_object *blob;
> int fd;
> FILE *fp;
>@@ -470,6 +471,7 @@ int gotweb_render_repo_table_hdr(struct template *);
>
> /* pages.tmpl */
> int gotweb_render_page(struct template *, int (*)(struct template *));
>+int gotweb_render_error(struct template *);
> int gotweb_render_repo_table_hdr(struct template *);
> int gotweb_render_repo_fragment(struct template *, struct repo_dir *);
> int gotweb_render_briefs(struct template *);
>blob - 6517aad5f520ea885ec366bd60717093b4ce85ae
>file + gotwebd/pages.tmpl
>--- gotwebd/pages.tmpl
>+++ gotwebd/pages.tmpl
>@@ -125,6 +125,20 @@ static inline int rss_author(struct template *, char *
> </html>
> {{ end }}
>
>+{{ define gotweb_render_error(struct template *tp) }}
>+{!
>+ struct request *c = tp->tp_arg;
>+ struct transport *t = c->t;
>+!}
>+<div id="err_content">
>+ {{ if t->error }}
>+ {{ t->error->msg }}
>+ {{ else }}
>+ See daemon logs for details
>+ {{ end }}
>+</div>
>+{{ end }}
>+
> {{ define gotweb_render_repo_table_hdr(struct template *tp) }}
> {!
> struct request *c = tp->tp_arg;
>
--
Tracey Emery
Sent from my phone.
gotwebd: reply with non-200 HTTP status code