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?

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);