commit 62eab86e6a1d5aea8a1bf90999c3c058b9aedd07 from: Omar Polo date: Wed Sep 13 07:39:05 2023 UTC gotwebd: move the buffering from the fastcgi layer to the template Reduces the indirection in fcgi.c, starts to make the struct template opaque, simplifies the template usage. All with a net negative :-) reads fine to stsp@ (thanks!) commit - de8c0409ea9206f320235deea0781cac753fae53 commit + 62eab86e6a1d5aea8a1bf90999c3c058b9aedd07 blob - 5c06bfd1e66558fb49eb1dbacf1a866876717461 blob + b6905f7fda842f60988300371ff61ab63db68a4c --- gotwebd/fcgi.c +++ gotwebd/fcgi.c @@ -140,12 +140,6 @@ fcgi_parse_record(uint8_t *buf, size_t n, struct reque break; case FCGI_STDIN: case FCGI_ABORT_REQUEST: - if (c->sock->client_status != CLIENT_DISCONNECT && - c->outbuf_len != 0) { - fcgi_send_response(c, FCGI_STDOUT, c->outbuf, - c->outbuf_len); - } - fcgi_create_end_record(c); fcgi_cleanup_request(c); return 0; @@ -196,6 +190,7 @@ fcgi_parse_params(uint8_t *buf, uint16_t n, struct req if (n == 0) { gotweb_process_request(c); + template_flush(c->tp); return; } @@ -274,92 +269,8 @@ void fcgi_timeout(int fd, short events, void *arg) { fcgi_cleanup_request((struct request*) arg); -} - -int -fcgi_puts(struct template *tp, const char *str) -{ - if (str == NULL) - return 0; - return fcgi_gen_binary_response(tp->tp_arg, str, strlen(str)); } -int -fcgi_putc(struct template *tp, int ch) -{ - uint8_t c = ch; - return fcgi_gen_binary_response(tp->tp_arg, &c, 1); -} - -int -fcgi_vprintf(struct request *c, const char *fmt, va_list ap) -{ - char *str; - int r; - - r = vasprintf(&str, fmt, ap); - if (r == -1) { - log_warn("%s: asprintf", __func__); - return -1; - } - - r = fcgi_gen_binary_response(c, str, r); - free(str); - return r; -} - -int -fcgi_printf(struct request *c, const char *fmt, ...) -{ - va_list ap; - int r; - - va_start(ap, fmt); - r = fcgi_vprintf(c, fmt, ap); - va_end(ap); - - return r; -} - -int -fcgi_gen_binary_response(struct request *c, const uint8_t *data, int len) -{ - int r; - - if (c->sock->client_status == CLIENT_DISCONNECT) - return -1; - - if (data == NULL || len == 0) - return 0; - - /* - * special case: send big replies -like blobs- directly - * without copying. - */ - if (len > sizeof(c->outbuf)) { - if (c->outbuf_len > 0) { - fcgi_send_response(c, FCGI_STDOUT, - c->outbuf, c->outbuf_len); - c->outbuf_len = 0; - } - return fcgi_send_response(c, FCGI_STDOUT, data, len); - } - - if (len < sizeof(c->outbuf) - c->outbuf_len) { - memcpy(c->outbuf + c->outbuf_len, data, len); - c->outbuf_len += len; - return 0; - } - - r = fcgi_send_response(c, FCGI_STDOUT, c->outbuf, c->outbuf_len); - if (r == -1) - return -1; - - memcpy(c->outbuf, data, len); - c->outbuf_len = len; - return 0; -} - static int send_response(struct request *c, int type, const uint8_t *data, size_t len) @@ -464,6 +375,14 @@ fcgi_send_response(struct request *c, int type, const return send_response(c, type, data, len); } +int +fcgi_write(void *arg, const void *buf, size_t len) +{ + struct request *c = arg; + + return fcgi_send_response(c, FCGI_STDOUT, buf, len); +} + void fcgi_create_end_record(struct request *c) { blob - d9c0aa2de83b12df70d1fbce28bfddc549348a9c blob + 7dfcab373f2846a43c2eeefc605fae2754869e12 --- gotwebd/gotweb.c +++ gotwebd/gotweb.c @@ -106,25 +106,25 @@ gotweb_reply(struct request *c, int status, const char { const char *csp; - if (status != 200 && fcgi_printf(c, "Status: %d\r\n", status) == -1) + if (status != 200 && tp_writef(c->tp, "Status: %d\r\n", status) == -1) return -1; if (location) { - if (fcgi_puts(c->tp, "Location: ") == -1 || + if (tp_writes(c->tp, "Location: ") == -1 || gotweb_render_url(c, location) == -1 || - fcgi_puts(c->tp, "\r\n") == -1) + tp_writes(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) + if (tp_writes(c->tp, csp) == -1) return -1; - if (ctype && fcgi_printf(c, "Content-Type: %s\r\n", ctype) == -1) + if (ctype && tp_writef(c->tp, "Content-Type: %s\r\n", ctype) == -1) return -1; - return fcgi_puts(c->tp, "\r\n"); + return tp_writes(c->tp, "\r\n"); } static int @@ -133,7 +133,7 @@ gotweb_reply_file(struct request *c, const char *ctype { int r; - r = fcgi_printf(c, "Content-Disposition: attachment; " + r = tp_writef(c->tp, "Content-Disposition: attachment; " "filename=%s%s\r\n", file, suffix ? suffix : ""); if (r == -1) return -1; @@ -254,6 +254,8 @@ gotweb_process_request(struct request *c) else r = gotweb_reply(c, 200, "text/plain", NULL); if (r == -1) + return; + if (template_flush(c->tp) == -1) return; for (;;) { @@ -263,7 +265,7 @@ gotweb_process_request(struct request *c) if (len == 0) break; buf = got_object_blob_get_read_buf(c->t->blob); - if (fcgi_gen_binary_response(c, buf, len) == -1) + if (fcgi_write(c, buf, len) == -1) break; } return; @@ -1004,25 +1006,25 @@ gotweb_render_url(struct request *c, struct gotweb_url action = gotweb_action_name(url->action); if (action != NULL) { - if (fcgi_printf(c, "?action=%s", action) == -1) + if (tp_writef(c->tp, "?action=%s", action) == -1) return -1; sep = "&"; } if (url->commit) { - if (fcgi_printf(c, "%scommit=%s", sep, url->commit) == -1) + if (tp_writef(c->tp, "%scommit=%s", sep, url->commit) == -1) return -1; sep = "&"; } if (url->previd) { - if (fcgi_printf(c, "%sprevid=%s", sep, url->previd) == -1) + if (tp_writef(c->tp, "%sprevid=%s", sep, url->previd) == -1) return -1; sep = "&"; } if (url->prevset) { - if (fcgi_printf(c, "%sprevset=%s", sep, url->prevset) == -1) + if (tp_writef(c->tp, "%sprevset=%s", sep, url->prevset) == -1) return -1; sep = "&"; } @@ -1031,7 +1033,7 @@ gotweb_render_url(struct request *c, struct gotweb_url tmp = gotweb_urlencode(url->file); if (tmp == NULL) return -1; - r = fcgi_printf(c, "%sfile=%s", sep, tmp); + r = tp_writef(c->tp, "%sfile=%s", sep, tmp); free(tmp); if (r == -1) return -1; @@ -1042,7 +1044,7 @@ gotweb_render_url(struct request *c, struct gotweb_url tmp = gotweb_urlencode(url->folder); if (tmp == NULL) return -1; - r = fcgi_printf(c, "%sfolder=%s", sep, tmp); + r = tp_writef(c->tp, "%sfolder=%s", sep, tmp); free(tmp); if (r == -1) return -1; @@ -1053,7 +1055,7 @@ gotweb_render_url(struct request *c, struct gotweb_url tmp = gotweb_urlencode(url->headref); if (tmp == NULL) return -1; - r = fcgi_printf(c, "%sheadref=%s", sep, url->headref); + r = tp_writef(c->tp, "%sheadref=%s", sep, url->headref); free(tmp); if (r == -1) return -1; @@ -1061,7 +1063,7 @@ gotweb_render_url(struct request *c, struct gotweb_url } if (url->index_page != -1) { - if (fcgi_printf(c, "%sindex_page=%d", sep, + if (tp_writef(c->tp, "%sindex_page=%d", sep, url->index_page) == -1) return -1; sep = "&"; @@ -1071,7 +1073,7 @@ gotweb_render_url(struct request *c, struct gotweb_url tmp = gotweb_urlencode(url->path); if (tmp == NULL) return -1; - r = fcgi_printf(c, "%spath=%s", sep, tmp); + r = tp_writef(c->tp, "%spath=%s", sep, tmp); free(tmp); if (r == -1) return -1; @@ -1079,7 +1081,7 @@ gotweb_render_url(struct request *c, struct gotweb_url } if (url->page != -1) { - if (fcgi_printf(c, "%spage=%d", sep, url->page) == -1) + if (tp_writef(c->tp, "%spage=%d", sep, url->page) == -1) return -1; sep = "&"; } @@ -1093,8 +1095,8 @@ gotweb_render_absolute_url(struct request *c, struct g struct template *tp = c->tp; const char *proto = c->https ? "https" : "http"; - if (fcgi_puts(tp, proto) == -1 || - fcgi_puts(tp, "://") == -1 || + if (tp_writes(tp, proto) == -1 || + tp_writes(tp, "://") == -1 || tp_htmlescape(tp, c->server_name) == -1 || tp_htmlescape(tp, c->document_uri) == -1) return -1; @@ -1368,36 +1370,36 @@ gotweb_render_age(struct template *tp, time_t committe case TM_DIFF: diff_time = time(NULL) - committer_time; if (diff_time > 60 * 60 * 24 * 365 * 2) { - if (fcgi_printf(c, "%lld %s", + if (tp_writef(c->tp, "%lld %s", (diff_time / 60 / 60 / 24 / 365), years) == -1) return -1; } else if (diff_time > 60 * 60 * 24 * (365 / 12) * 2) { - if (fcgi_printf(c, "%lld %s", + if (tp_writef(c->tp, "%lld %s", (diff_time / 60 / 60 / 24 / (365 / 12)), months) == -1) return -1; } else if (diff_time > 60 * 60 * 24 * 7 * 2) { - if (fcgi_printf(c, "%lld %s", + if (tp_writef(c->tp, "%lld %s", (diff_time / 60 / 60 / 24 / 7), weeks) == -1) return -1; } else if (diff_time > 60 * 60 * 24 * 2) { - if (fcgi_printf(c, "%lld %s", + if (tp_writef(c->tp, "%lld %s", (diff_time / 60 / 60 / 24), days) == -1) return -1; } else if (diff_time > 60 * 60 * 2) { - if (fcgi_printf(c, "%lld %s", + if (tp_writef(c->tp, "%lld %s", (diff_time / 60 / 60), hours) == -1) return -1; } else if (diff_time > 60 * 2) { - if (fcgi_printf(c, "%lld %s", (diff_time / 60), + if (tp_writef(c->tp, "%lld %s", (diff_time / 60), minutes) == -1) return -1; } else if (diff_time > 2) { - if (fcgi_printf(c, "%lld %s", diff_time, + if (tp_writef(c->tp, "%lld %s", diff_time, seconds) == -1) return -1; } else { - if (fcgi_puts(tp, now) == -1) + if (tp_writes(tp, now) == -1) return -1; } break; @@ -1409,8 +1411,8 @@ gotweb_render_age(struct template *tp, time_t committe if (s == NULL) return -1; - if (fcgi_puts(tp, datebuf) == -1 || - fcgi_puts(tp, " UTC") == -1) + if (tp_writes(tp, datebuf) == -1 || + tp_writes(tp, " UTC") == -1) return -1; break; case TM_RFC822: @@ -1422,7 +1424,7 @@ gotweb_render_age(struct template *tp, time_t committe if (r == 0) return -1; - if (fcgi_puts(tp, datebuf) == -1) + if (tp_writes(tp, datebuf) == -1) return -1; break; } blob - d276e0ebe586663208dfe4640e9c7714bed9f58d blob + ac6c5c8b552e91a539cec6f809ae7ca9d3e6cded --- gotwebd/gotwebd.h +++ gotwebd/gotwebd.h @@ -240,7 +240,6 @@ struct request { size_t buf_len; uint8_t outbuf[GOTWEBD_CACHESIZE]; - size_t outbuf_len; char querystring[MAX_QUERYSTRING]; char document_uri[MAX_DOCUMENT_URI]; @@ -495,13 +494,7 @@ void fcgi_timeout(int, short, void *); void fcgi_cleanup_request(struct request *); void fcgi_create_end_record(struct request *); void dump_fcgi_record(const char *, struct fcgi_record_header *); -int fcgi_puts(struct template *, const char *); -int fcgi_putc(struct template *, int); -int fcgi_vprintf(struct request *, const char *, va_list); -int fcgi_printf(struct request *, const char *, ...) - __attribute__((__format__(printf, 2, 3))) - __attribute__((__nonnull__(2))); -int fcgi_gen_binary_response(struct request *, const uint8_t *, int); +int fcgi_write(void *, const void *, size_t); /* got_operations.c */ const struct got_error *got_gotweb_closefile(FILE *); blob - 3f932978bf05a203c0c4bcd43bf7390fb4d153c8 blob + 341d3774c799acfb106876120fa0e5ae0b9131c0 --- gotwebd/sockets.c +++ gotwebd/sockets.c @@ -620,7 +620,7 @@ sockets_socket_accept(int fd, short event, void *arg) return; } - c->tp = template(c, fcgi_puts, fcgi_putc); + c->tp = template(c, &fcgi_write, c->outbuf, sizeof(c->outbuf)); if (c->tp == NULL) { log_warn("%s", __func__); close(s); blob - 602717bcefc2fd65c0cb426c4d1fe263cdbaee7f blob + a26f893929de81045bf85ad76631aa136e7d86b3 --- regress/template/runbase.c +++ regress/template/runbase.c @@ -21,44 +21,36 @@ #include "tmpl.h" int base(struct template *, const char *title); -int my_putc(struct template *, int); -int my_puts(struct template *, const char *); +int my_write(void *, const void *, size_t); int -my_putc(struct template *tp, int c) +my_write(void *arg, const void *s, size_t len) { - FILE *fp = tp->tp_arg; + FILE *fp = arg; - if (putc(c, fp) < 0) + if (fwrite(s, 1, len, fp) < 0) return (-1); return (0); } int -my_puts(struct template *tp, const char *s) -{ - FILE *fp = tp->tp_arg; - - if (fputs(s, fp) < 0) - return (-1); - - return (0); -} - -int main(int argc, char **argv) { - struct template *tp; + struct template *tp; + char buf[3]; + /* use a ridiculously small buffer in regress */ - if ((tp = template(stdout, my_puts, my_putc)) == NULL) + if ((tp = template(stdout, my_write, buf, sizeof(buf))) == NULL) err(1, "template"); - if (base(tp, " *hello* ") == -1) + if (base(tp, " *hello* ") == -1 || + template_flush(tp) == -1) return (1); puts(""); - if (base(tp, "") == -1) + if (base(tp, "") == -1 || + template_flush(tp) == -1) return (1); puts(""); blob - f6a415f000a087d0b1d41be0343b51916bebe9dd blob + 648bcc07fa18e09307682160f77408c3d644530c --- regress/template/runlist.c +++ regress/template/runlist.c @@ -22,40 +22,30 @@ #include "lists.h" int base(struct template *, struct tailhead *); -int my_putc(struct template *, int); -int my_puts(struct template *, const char *); +int my_write(void *, const void *, size_t); int -my_putc(struct template *tp, int c) +my_write(void *arg, const void *s, size_t len) { - FILE *fp = tp->tp_arg; + FILE *fp = arg; - if (putc(c, fp) < 0) + if (fwrite(s, 1, len, fp) < 0) return (-1); return (0); } int -my_puts(struct template *tp, const char *s) -{ - FILE *fp = tp->tp_arg; - - if (fputs(s, fp) < 0) - return (-1); - - return (0); -} - -int main(int argc, char **argv) { struct template *tp; struct tailhead head; struct entry *np; int i; + char buf[3]; + /* use a ridiculously small buffer in regress */ - if ((tp = template(stdout, my_puts, my_putc)) == NULL) + if ((tp = template(stdout, my_write, buf, sizeof(buf))) == NULL) err(1, "template"); TAILQ_INIT(&head); @@ -67,7 +57,8 @@ main(int argc, char **argv) TAILQ_INSERT_TAIL(&head, np, entries); } - if (base(tp, &head) == -1) + if (base(tp, &head) == -1 || + template_flush(tp) == -1) return (1); puts(""); @@ -77,7 +68,8 @@ main(int argc, char **argv) free(np); } - if (base(tp, &head) == -1) + if (base(tp, &head) == -1 || + template_flush(tp) == -1) return (1); puts(""); blob - c97478128fa19c9cddcf687a802db929cfa6bd36 blob + 59cf6888097e2e08fde77614c4ea7f9517e3f4ac --- template/parse.y +++ template/parse.y @@ -139,9 +139,10 @@ verbatims : /* empty */ raw : nstring { dbg(); - fprintf(fp, "if ((tp_ret = tp->tp_puts(tp, "); + fprintf(fp, "if ((tp_ret = tp_write(tp, "); printq($1); - fputs(")) == -1) goto err;\n", fp); + fprintf(fp, ", %zu)) == -1) goto err;\n", + strlen($1)); free($1); } @@ -189,7 +190,7 @@ special : '{' RENDER string '}' { | '{' string '|' UNSAFE '}' { dbg(); fprintf(fp, - "if ((tp_ret = tp->tp_puts(tp, %s)) == -1)\n", + "if ((tp_ret = tp_writes(tp, %s)) == -1)\n", $2); fputs("goto err;\n", fp); free($2); @@ -205,7 +206,7 @@ special : '{' RENDER string '}' { | '{' string '}' { dbg(); fprintf(fp, - "if ((tp_ret = tp->tp_escape(tp, %s)) == -1)\n", + "if ((tp_ret = tp_htmlescape(tp, %s)) == -1)\n", $2); fputs("goto err;\n", fp); free($2); @@ -218,7 +219,7 @@ printf : '{' PRINTF { } printfargs '}' { fputs(") == -1)\n", fp); fputs("goto err;\n", fp); - fputs("if ((tp_ret = tp->tp_escape(tp, tp->tp_tmp)) " + fputs("if ((tp_ret = tp_htmlescape(tp, tp->tp_tmp)) " "== -1)\n", fp); fputs("goto err;\n", fp); fputs("free(tp->tp_tmp);\n", fp); blob - 18ee428516dc3b90ef925c5ac135e90278c0d5f9 blob + 5f1093f5488dc59cedb4551421207df0fb7e025d --- template/tmpl.c +++ template/tmpl.c @@ -15,12 +15,62 @@ */ #include +#include #include #include +#include #include "tmpl.h" int +tp_write(struct template *tp, const char *str, size_t len) +{ + size_t avail; + + while (len > 0) { + avail = tp->tp_cap - tp->tp_len; + if (avail == 0) { + if (template_flush(tp) == -1) + return (-1); + avail = tp->tp_cap; + } + + if (len < avail) + avail = len; + + memcpy(tp->tp_buf + tp->tp_len, str, avail); + tp->tp_len += avail; + str += avail; + len -= avail; + } + + return (0); +} + +int +tp_writes(struct template *tp, const char *str) +{ + return (tp_write(tp, str, strlen(str))); +} + +int +tp_writef(struct template *tp, const char *fmt, ...) +{ + va_list ap; + char *str; + int r; + + va_start(ap, fmt); + r = vasprintf(&str, fmt, ap); + va_end(ap); + if (r == -1) + return (-1); + r = tp_write(tp, str, r); + free(str); + return (r); +} + +int tp_urlescape(struct template *tp, const char *str) { int r; @@ -36,10 +86,10 @@ tp_urlescape(struct template *tp, const char *str) r = snprintf(tmp, sizeof(tmp), "%%%2X", *str); if (r < 0 || (size_t)r >= sizeof(tmp)) return (0); - if (tp->tp_puts(tp, tmp) == -1) + if (tp_write(tp, tmp, r) == -1) return (-1); } else { - if (tp->tp_putc(tp, *str) == -1) + if (tp_write(tp, str, 1) == -1) return (-1); } } @@ -58,22 +108,22 @@ tp_htmlescape(struct template *tp, const char *str) for (; *str; ++str) { switch (*str) { case '<': - r = tp->tp_puts(tp, "<"); + r = tp_write(tp, "<", 4); break; case '>': - r = tp->tp_puts(tp, ">"); + r = tp_write(tp, ">", 4); break; case '&': - r = tp->tp_puts(tp, "&"); + r = tp_write(tp, "&", 5); break; case '"': - r = tp->tp_puts(tp, """); + r = tp_write(tp, """, 6); break; case '\'': - r = tp->tp_puts(tp, "'"); + r = tp_write(tp, "'", 6); break; default: - r = tp->tp_putc(tp, *str); + r = tp_write(tp, str, 1); break; } @@ -85,7 +135,7 @@ tp_htmlescape(struct template *tp, const char *str) } struct template * -template(void *arg, tmpl_puts putsfn, tmpl_putc putcfn) +template(void *arg, tmpl_write writefn, char *buf, size_t siz) { struct template *tp; @@ -93,13 +143,25 @@ template(void *arg, tmpl_puts putsfn, tmpl_putc putcfn return (NULL); tp->tp_arg = arg; - tp->tp_escape = tp_htmlescape; - tp->tp_puts = putsfn; - tp->tp_putc = putcfn; + tp->tp_write = writefn; + tp->tp_buf = buf; + tp->tp_cap = siz; return (tp); } +int +template_flush(struct template *tp) +{ + if (tp->tp_len == 0) + return (0); + + if (tp->tp_write(tp->tp_arg, tp->tp_buf, tp->tp_len) == -1) + return (-1); + tp->tp_len = 0; + return (0); +} + void template_free(struct template *tp) { blob - 4c8de903c9b7957de3c6bc1ed2058fa9a5db553b blob + df3b74c2696e16f2a591fb656220ac7957c59a56 --- template/tmpl.h +++ template/tmpl.h @@ -19,21 +19,26 @@ struct template; -typedef int (*tmpl_puts)(struct template *, const char *); -typedef int (*tmpl_putc)(struct template *, int); +typedef int (*tmpl_write)(void *, const void *, size_t); struct template { void *tp_arg; char *tp_tmp; - tmpl_puts tp_escape; - tmpl_puts tp_puts; - tmpl_putc tp_putc; + tmpl_write tp_write; + char *tp_buf; + size_t tp_len; + size_t tp_cap; }; -int tp_urlescape(struct template *, const char *); -int tp_htmlescape(struct template *, const char *); +int tp_write(struct template *, const char *, size_t); +int tp_writes(struct template *, const char *); +int tp_writef(struct template *, const char *, ...) + __attribute__((__format__ (printf, 2, 3))); +int tp_urlescape(struct template *, const char *); +int tp_htmlescape(struct template *, const char *); -struct template *template(void *, tmpl_puts, tmpl_putc); +struct template *template(void *, tmpl_write, char *, size_t); +int template_flush(struct template *); void template_free(struct template *); #endif