Download raw body.
gotwebd: refactor gotweb_render_content_type/_file
On 23-01-15 09:45AM, Omar Polo wrote:
> Hello,
>
> Now that everything^1 runs under the templates, I'd like to refactor
> gotweb_process_request so that we return a non-200 code on failure.
> The first step however, is to have a function that allow us to set a
> custom response code: this is done with the Status header on FastCGI.
>
> Diff below does a bit of cleanup, renames the functions (opinable; but
> I liked a shorter name, not attached to it however, will keep the old
> name if it's preferred) and allows to set both Location and Status.
>
> What do you think?
I like the move. I'm also partial to shorter naming provided it doesn't
compromise semantics, and in this case, I don't think it does so it looks
good to me.
I'd say ok but it would be good to hear from tracey too.
> It's the first diff in a while that doesn't remove more lines than it
> adds, it's a tie 54-54 this time.
>
> ^1: almost everything, the error page is still rendered by hand, will
> get there...
>
>
> -----------------------------------------------
> commit a5155be00a47787bc9b3fcbfc41eb761bf03559e (main)
> from: Omar Polo <op@omarpolo.com>
> date: Sun Jan 15 08:43:58 2023 UTC
>
> gotwebd: refactor gotweb_render_content_type/_file
>
> Rework them so that they allow to set the Status header (the HTTP status
> code; only way since we're behind FastCGI) and optionally a Location.
>
> Since they're now unused outside of gotweb.c, mark them as static. They
> also used to always return NULL so the error is pointless; return the -1
> on failure though.
>
> While here, rename to gotweb_reply and gotweb_reply_file.
>
> diff 760079985fc2d63ebd4155a76d4f0d20fbc2f4c5 a5155be00a47787bc9b3fcbfc41eb761bf03559e
> commit - 760079985fc2d63ebd4155a76d4f0d20fbc2f4c5
> commit + a5155be00a47787bc9b3fcbfc41eb761bf03559e
> blob - f130435179a5b33ae9f100337a219e97ce5607aa
> blob + a7105e53d7ad053fa0fcdd9dcfa7c202f39ec3fb
> --- gotwebd/gotweb.c
> +++ gotwebd/gotweb.c
> @@ -99,6 +99,46 @@ void
>
> struct server *gotweb_get_server(uint8_t *, uint8_t *);
>
> +static int
> +gotweb_reply(struct request *c, int status, const char *ctype,
> + struct gotweb_url *location)
> +{
> + const char *csp;
> +
> + if (status != 200 && fcgi_printf(c, "Status: %d\r\n", status) == -1)
> + return -1;
> +
> + if (location) {
> + if (fcgi_puts(c->tp, "Location: ") == -1 ||
> + gotweb_render_url(c, location) == -1 ||
> + fcgi_puts(c->tp, "\r\n") == -1)
> + return -1;
> + }
> +
> + csp = "Content-Security-Policy: default-src 'self'; "
> + "script-src 'none'; object-src 'none';\r\n";
> + if (fcgi_puts(c->tp, csp) == -1)
> + return -1;
> +
> + if (ctype && fcgi_printf(c, "Content-Type: %s\r\n", ctype) == -1)
> + return -1;
> +
> + return fcgi_puts(c->tp, "\r\n");
> +}
> +
> +static int
> +gotweb_reply_file(struct request *c, const char *ctype, const char *file,
> + const char *suffix)
> +{
> + int r;
> +
> + r = fcgi_printf(c, "Content-Disposition: attachment; "
> + "filename=%s%s\r\n", file, suffix ? suffix : "");
> + if (r == -1)
> + return -1;
> + return gotweb_reply(c, 200, ctype, NULL);
> +}
> +
> void
> gotweb_process_request(struct request *c)
> {
> @@ -170,7 +210,7 @@ gotweb_process_request(struct request *c)
> if (qs->action == BLOBRAW) {
> const uint8_t *buf;
> size_t len;
> - int binary;
> + int binary, r;
>
> error = got_get_repo_commits(c, 1);
> if (error)
> @@ -181,15 +221,12 @@ gotweb_process_request(struct request *c)
> goto render;
>
> if (binary)
> - error = gotweb_render_content_type_file(c,
> - "application/octet-stream", qs->file, NULL);
> + r = gotweb_reply_file(c, "application/octet-stream",
> + qs->file, NULL);
> else
> - error = gotweb_render_content_type(c, "text/plain");
> -
> - if (error) {
> - log_warnx("%s: %s", __func__, error->msg);
> + r = gotweb_reply(c, 200, "text/plain", NULL);
> + if (r == -1)
> goto done;
> - }
>
> for (;;) {
> error = got_object_blob_read_block(&len, blob);
> @@ -225,23 +262,17 @@ gotweb_process_request(struct request *c)
> if (error2)
> goto render;
> if (binary) {
> - fcgi_puts(c->tp, "Status: 302\r\n");
> - fcgi_puts(c->tp, "Location: ");
> - gotweb_render_url(c, &url);
> - fcgi_puts(c->tp, "\r\n\r\n");
> + gotweb_reply(c, 302, NULL, &url);
> goto done;
> }
> }
>
> if (qs->action == RSS) {
> - error = gotweb_render_content_type_file(c,
> - "application/rss+xml;charset=utf-8",
> - repo_dir->name, ".rss");
> - if (error) {
> - log_warnx("%s: %s", __func__, error->msg);
> - goto err;
> - }
> + const char *ctype = "application/rss+xml;charset=utf-8";
>
> + if (gotweb_reply_file(c, ctype, repo_dir->name, ".rss") == -1)
> + goto done;
> +
> error = got_get_repo_tags(c, D_MAXSLCOMMDISP);
> if (error) {
> log_warnx("%s: %s", __func__, error->msg);
> @@ -253,11 +284,8 @@ render:
> }
>
> render:
> - error = gotweb_render_content_type(c, "text/html");
> - if (error) {
> - log_warnx("%s: %s", __func__, error->msg);
> - goto err;
> - }
> + if (gotweb_reply(c, 200, "text/html", NULL) == -1)
> + goto done;
> html = 1;
>
> if (gotweb_render_header(c->tp) == -1)
> @@ -760,29 +788,6 @@ const struct got_error *
> free(t);
> }
>
> -const struct got_error *
> -gotweb_render_content_type(struct request *c, const char *type)
> -{
> - const char *csp = "default-src 'self'; script-src 'none'; "
> - "object-src 'none';";
> -
> - fcgi_printf(c,
> - "Content-Security-Policy: %s\r\n"
> - "Content-Type: %s\r\n\r\n",
> - csp, type);
> - return NULL;
> -}
> -
> -const struct got_error *
> -gotweb_render_content_type_file(struct request *c, const char *type,
> - const char *file, const char *suffix)
> -{
> - fcgi_printf(c, "Content-type: %s\r\n"
> - "Content-disposition: attachment; filename=%s%s\r\n\r\n",
> - type, file, suffix ? suffix : "");
> - return NULL;
> -}
> -
> void
> gotweb_get_navs(struct request *c, struct gotweb_url *prev, int *have_prev,
> struct gotweb_url *next, int *have_next)
> blob - bc8d26743466b3016093da2bab2a199948bf752a
> blob + 66a28ad4b968ae378a24ea55deb2ea6001240067
> --- gotwebd/gotwebd.h
> +++ gotwebd/gotwebd.h
> @@ -449,11 +449,6 @@ const struct got_error *gotweb_render_content_type(str
> int sockets_privinit(struct gotwebd *, struct socket *);
>
> /* gotweb.c */
> -const struct got_error *gotweb_render_content_type(struct request *,
> - const char *);
> -const struct got_error
> - *gotweb_render_content_type_file(struct request *, const char *,
> - const char *, const char *);
> void gotweb_get_navs(struct request *, struct gotweb_url *, int *,
> struct gotweb_url *, int *);
> const struct got_error *gotweb_get_time_str(char **, time_t, int);
>
--
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
gotwebd: refactor gotweb_render_content_type/_file