Download raw body.
httpd: drain abort response via bufferevent
On Thu, 25 Jun 2026 13:03:33 +0200,
Rafael Sadowski <rafael@sizeofvoid.org> wrote:
>
> 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:
>
Make sense for me.
I see logic in old behaviour, but this one is probably better.
So, OK kirill@
> 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)
> {
>
--
wbr, Kirill
httpd: drain abort response via bufferevent