httpd: drain abort response via bufferevent

This is an all-in-one diff that replaces server_dump() with a
bufferevent aka event-loop approach.

server_dump() did one non-blocking write(2)/tls_write(3) and discarded
the return value, so partial writes were lost, meaning that large
user-defined error documents were silently truncated in transit before.

This diff merge three commits into one. I don’t think it's too long to
review. If we're OK, I'll split it into three separate commits:

1. ref: server_create_builtin
https://rsadowski.gothub.org/?action=diff&commit=7dc036be4782f5e6a31fa6bdd9ba1a57bea72f5a&path=httpd.git
2. drain abort response via bufferevent
https://rsadowski.gothub.org/?action=diff&commit=b07039fc1f28bfe260ab72963e644c7575cad267&path=httpd.git
3. error loading the document in server_create_errdoc()
https://rsadowski.gothub.org/?action=diff&commit=ea58fa5107a8182e42d5c180ce80c23fed0ad7d2&path=httpd.git

Cheers, Rafael

diff --git a/httpd.h b/httpd.h
index 12122c6..664524e 100644
--- a/httpd.h
+++ b/httpd.h
@@ -344,6 +344,8 @@ struct client {
 	unsigned int		 clt_pipelining;
 	int			 clt_line;
 	int			 clt_done;
+	int			 clt_close_after_write;
+	char			*clt_close_msg;
 	int			 clt_chunk;
 	int			 clt_inflight;
 	int			 clt_fcgi_count;
@@ -651,7 +653,6 @@ void	 server_log(struct client *, const char *);
 void	 server_sendlog(struct server_config *, int, const char *, ...)
 	    __attribute__((__format__ (printf, 3, 4)));
 void	 server_close(struct client *, const char *);
-void	 server_dump(struct client *, const void *, size_t);
 int	 server_client_cmp(struct client *, struct client *);
 int	 server_bufferevent_printf(struct client *, const char *, ...)
 	    __attribute__((__format__ (printf, 2, 3)));
@@ -662,6 +663,7 @@ int	 server_bufferevent_write_chunk(struct client *,
 	    struct evbuffer *, size_t);
 int	 server_bufferevent_add(struct event *, int);
 int	 server_bufferevent_write(struct client *, void *, size_t);
+int	 server_bufferevent_write_close(struct client *, void *, size_t);
 struct server *
 	 server_byaddr(struct sockaddr *, in_port_t);
 struct server_config *
diff --git a/server.c b/server.c
index 0f42ccd..43ea95e 100644
--- a/server.c
+++ b/server.c
@@ -937,6 +937,12 @@ server_write(struct bufferevent *bev, void *arg)
 	struct client		*clt = arg;
 	struct evbuffer		*dst = EVBUFFER_OUTPUT(bev);
 
+	if (EVBUFFER_LENGTH(dst) == 0 && clt->clt_close_after_write) {
+		server_close(clt, clt->clt_close_msg != NULL ?
+		    clt->clt_close_msg : "response sent");
+		return;
+	}
+
 	if (EVBUFFER_LENGTH(dst) == 0 &&
 	    clt->clt_toread == TOREAD_HTTP_NONE)
 		goto done;
@@ -957,24 +963,6 @@ server_write(struct bufferevent *bev, void *arg)
 	return;
 }
 
-void
-server_dump(struct client *clt, const void *buf, size_t len)
-{
-	if (!len)
-		return;
-
-	/*
-	 * This function will dump the specified message directly
-	 * to the underlying client, without waiting for success
-	 * of non-blocking events etc. This is useful to print an
-	 * error message before gracefully closing the client.
-	 */
-	if (clt->clt_tls_ctx != NULL)
-		(void)tls_write(clt->clt_tls_ctx, buf, len);
-	else
-		(void)write(clt->clt_s, buf, len);
-}
-
 void
 server_read(struct bufferevent *bev, void *arg)
 {
@@ -1340,6 +1328,8 @@ server_close(struct client *clt, const char *msg)
 	if (clt->clt_log != NULL)
 		evbuffer_free(clt->clt_log);
 
+	free(clt->clt_close_msg);
+
 	free(clt);
 	server_clients--;
 }
@@ -1472,6 +1462,20 @@ server_bufferevent_write(struct client *clt, void *data, size_t size)
 	return (bufferevent_write(clt->clt_bev, data, size));
 }
 
+int
+server_bufferevent_write_close(struct client *clt, void *data, size_t size)
+{
+	if (clt->clt_bev == NULL)
+		return (-1);
+	if (clt->clt_close_after_write)
+		return (-1);
+
+	clt->clt_persist = 0;
+	clt->clt_close_after_write = 1;
+
+	return (bufferevent_write(clt->clt_bev, data, size));
+}
+
 int
 server_client_cmp(struct client *a, struct client *b)
 {
diff --git a/server_http.c b/server_http.c
index 5ecb65c..28667bf 100644
--- a/server_http.c
+++ b/server_http.c
@@ -55,6 +55,10 @@ char		*server_expand_http(struct client *, const char *,
 		    char *, size_t);
 char		*replace_var(char *, const char *, const char *);
 char		*read_errdoc(const char *, const char *);
+ssize_t		 server_create_builtin(struct server_config *, char **,
+		    unsigned int, const char *);
+char		*server_create_errdoc(struct server_config *, unsigned int,
+		    const char *);
 
 static struct http_method	 http_methods[] = HTTP_METHODS;
 static struct http_error	 http_errors[] = HTTP_ERRORS;
@@ -880,15 +884,15 @@ server_abort_http(struct client *clt, unsigned int code, const char *msg)
 	struct server_config	*srv_conf = clt->clt_srv_conf;
 	struct bufferevent	*bev = clt->clt_bev;
 	struct http_descriptor	*desc = clt->clt_descreq;
-	const char		*httperr = NULL, *style;
+	const char		*httperr = NULL;
 	char			*httpmsg, *body = NULL, *extraheader = NULL;
 	char			 tmbuf[32], hbuf[128], *hstsheader = NULL;
 	char			*clenheader = NULL;
-	char			*bannerheader = NULL, *bannertoken = NULL;
+	char			*bannerheader = NULL;
 	char			 buf[IBUF_READ_SIZE];
 	char			*escapedmsg = NULL;
-	char			 cstr[5];
 	ssize_t			 bodylen;
+	ssize_t			 httpmsglen;
 
 	if (code == 0) {
 		server_close(clt, "dropped");
@@ -941,6 +945,7 @@ server_abort_http(struct client *clt, unsigned int code, const char *msg)
 			code = 500;
 			extraheader = NULL;
 		}
+		free(escapedmsg);
 		break;
 	case 416:
 		if (msg == NULL)
@@ -962,66 +967,12 @@ server_abort_http(struct client *clt, unsigned int code, const char *msg)
 		break;
 	}
 
-	free(escapedmsg);
-
-	if ((srv_conf->flags & SRVFLAG_ERRDOCS) == 0)
-		goto builtin; /* errdocs not enabled */
-	if ((size_t)snprintf(cstr, sizeof(cstr), "%03u", code) >= sizeof(cstr))
-		goto builtin;
-
-	if ((body = read_errdoc(srv_conf->errdocroot, cstr)) == NULL &&
-	    (body = read_errdoc(srv_conf->errdocroot, HTTPD_ERRDOCTEMPLATE))
-	    == NULL)
-		goto builtin;
-
-	body = replace_var(body, "$HTTP_ERROR", httperr);
-	body = replace_var(body, "$RESPONSE_CODE", cstr);
-	/* Check if server banner is suppressed */
-	if ((srv_conf->flags & SRVFLAG_NO_BANNER) == 0)
-		body = replace_var(body, "$SERVER_SOFTWARE", HTTPD_SERVERNAME);
-	else
-		body = replace_var(body, "$SERVER_SOFTWARE", "");
-	bodylen = strlen(body);
-	goto send;
-
- builtin:
-	/* A CSS stylesheet allows minimal customization by the user */
-	style = "body { background-color: white; color: black; font-family: "
-	    "'Comic Sans MS', 'Chalkboard SE', 'Comic Neue', sans-serif; }\n"
-	    "hr { border: 0; border-bottom: 1px dashed; }\n"
-	    "@media (prefers-color-scheme: dark) {\n"
-	    "body { background-color: #1E1F21; color: #EEEFF1; }\n"
-	    "a { color: #BAD7FF; }\n}";
-
-	/* If banner is suppressed, don't write it to the error document */
-	if ((srv_conf->flags & SRVFLAG_NO_BANNER) == 0)
-		if (asprintf(&bannertoken, "<hr>\n<address>%s</address>\n",
-		    HTTPD_SERVERNAME) == -1) {
-			bannertoken = NULL;
-			goto done;
-		}
-
-	/* Generate simple HTML error document */
-	if ((bodylen = asprintf(&body,
-	    "<!DOCTYPE html>\n"
-	    "<html>\n"
-	    "<head>\n"
-	    "<meta charset=\"utf-8\">\n"
-	    "<title>%03d %s</title>\n"
-	    "<style type=\"text/css\"><!--\n%s\n--></style>\n"
-	    "</head>\n"
-	    "<body>\n"
-	    "<h1>%03d %s</h1>\n"
-	    "%s"
-	    "</body>\n"
-	    "</html>\n",
-	    code, httperr, style, code, httperr,
-	    bannertoken == NULL ? "" : bannertoken)) == -1) {
-		body = NULL;
+	if ((body = server_create_errdoc(srv_conf, code, httperr)) != NULL) {
+		bodylen = strlen(body);
+	} else if ((bodylen = server_create_builtin(srv_conf, &body, code,
+	    httperr)) == -1)
 		goto done;
-	}
 
- send:
 	if (srv_conf->flags & SRVFLAG_SERVER_HSTS &&
 	    srv_conf->flags & SRVFLAG_TLS) {
 		if (asprintf(&hstsheader, "Strict-Transport-Security: "
@@ -1054,7 +1005,7 @@ server_abort_http(struct client *clt, unsigned int code, const char *msg)
 		}
 
 	/* Add basic HTTP headers */
-	if (asprintf(&httpmsg,
+	if ((httpmsglen = asprintf(&httpmsg,
 	    "HTTP/1.0 %03d %s\r\n"
 	    "Date: %s\r\n"
 	    "%s"
@@ -1071,12 +1022,27 @@ server_abort_http(struct client *clt, unsigned int code, const char *msg)
 	    extraheader == NULL ? "" : extraheader,
 	    hstsheader == NULL ? "" : hstsheader,
 	    desc->http_method == HTTP_METHOD_HEAD || clenheader == NULL ?
-	    "" : body) == -1)
+	    "" : body)) == -1)
 		goto done;
 
-	/* Dump the message without checking for success */
-	server_dump(clt, httpmsg, strlen(httpmsg));
+	free(clt->clt_close_msg);
+	if (asprintf(&clt->clt_close_msg, "%s (%03d %s)",
+	    msg == NULL ? "\"\"" : msg, code, httperr) == -1)
+		clt->clt_close_msg = NULL;
+
+	if (server_bufferevent_write_close(clt, httpmsg,
+	    (size_t)httpmsglen) == -1) {
+		/* fall back to synchronous close */
+		free(httpmsg);
+		goto done;
+	}
 	free(httpmsg);
+	free(body);
+	free(extraheader);
+	free(hstsheader);
+	free(clenheader);
+	free(bannerheader);
+	return;
 
  done:
 	free(body);
@@ -1084,7 +1050,6 @@ server_abort_http(struct client *clt, unsigned int code, const char *msg)
 	free(hstsheader);
 	free(clenheader);
 	free(bannerheader);
-	free(bannertoken);
 	if (msg == NULL)
 		msg = "\"\"";
 	if (asprintf(&httpmsg, "%s (%03d %s)", msg, code, httperr) == -1) {
@@ -1095,6 +1060,80 @@ server_abort_http(struct client *clt, unsigned int code, const char *msg)
 	}
 }
 
+char *
+server_create_errdoc(struct server_config *srv_conf, unsigned int code,
+    const char *httperr)
+{
+	char	 cstr[5];
+	char	*body;
+
+	if (!(srv_conf->flags & SRVFLAG_ERRDOCS))
+		return (NULL);
+
+	if ((size_t)snprintf(cstr, sizeof(cstr), "%03u", code) >= sizeof(cstr))
+		return (NULL);
+
+	if ((body = read_errdoc(srv_conf->errdocroot, cstr)) == NULL &&
+	    (body = read_errdoc(srv_conf->errdocroot, HTTPD_ERRDOCTEMPLATE))
+	    == NULL)
+		return (NULL);
+
+	body = replace_var(body, "$HTTP_ERROR", httperr);
+	body = replace_var(body, "$RESPONSE_CODE", cstr);
+	body = replace_var(body, "$SERVER_SOFTWARE",
+	    (srv_conf->flags & SRVFLAG_NO_BANNER) ? "" : HTTPD_SERVERNAME);
+
+	return (body);
+}
+
+ssize_t
+server_create_builtin(struct server_config *srv_conf, char **body,
+    unsigned int code, const char *httperr)
+{
+	const char	*style;
+	char		*bannertoken = NULL;
+	ssize_t		 bodylen;
+
+	/* A CSS stylesheet allows minimal customization by the user */
+	style = "body { background-color: white; color: black; font-family: "
+	    "'Comic Sans MS', 'Chalkboard SE', 'Comic Neue', sans-serif; }\n"
+	    "hr { border: 0; border-bottom: 1px dashed; }\n"
+	    "@media (prefers-color-scheme: dark) {\n"
+	    "body { background-color: #1E1F21; color: #EEEFF1; }\n"
+	    "a { color: #BAD7FF; }\n}";
+
+	/* If banner is suppressed, don't write it to the error document */
+	if ((srv_conf->flags & SRVFLAG_NO_BANNER) == 0) {
+		if (asprintf(&bannertoken, "<hr>\n<address>%s</address>\n",
+		    HTTPD_SERVERNAME) == -1)
+			return (-1);
+	}
+
+	/* Generate simple HTML error document */
+	bodylen = asprintf(body,
+	    "<!DOCTYPE html>\n"
+	    "<html>\n"
+	    "<head>\n"
+	    "<meta charset=\"utf-8\">\n"
+	    "<title>%03d %s</title>\n"
+	    "<style type=\"text/css\"><!--\n%s\n--></style>\n"
+	    "</head>\n"
+	    "<body>\n"
+	    "<h1>%03d %s</h1>\n"
+	    "%s"
+	    "</body>\n"
+	    "</html>\n",
+	    code, httperr, style, code, httperr,
+	    bannertoken == NULL ? "" : bannertoken);
+
+	free(bannertoken);
+	if (bodylen == -1) {
+		*body = NULL;
+		return (-1);
+	}
+	return (bodylen);
+}
+
 void
 server_close_http(struct client *clt)
 {