[hackers] [quark] Refactor response-generation || Laslo Hunhold

From: <git_AT_suckless.org>
Date: Wed, 5 Aug 2020 13:55:43 +0200 (CEST)

commit c51b31d7ac2c8bb8eeb6cb058c2d5d95fc4a52b8
Author: Laslo Hunhold <dev_AT_frign.de>
AuthorDate: Wed Aug 5 13:41:44 2020 +0200
Commit: Laslo Hunhold <dev_AT_frign.de>
CommitDate: Wed Aug 5 13:41:44 2020 +0200

    Refactor response-generation
    
    I wasn't happy with how responses were generated. HTTP-headers were
    handled by hand and it was duplicated in multiple parts of the code.
    Due to the duplication, some functions like timestamp() had really
    ugly semantics.
    
    The HTTP requests are parsed much better: We have an enum of fields
    we care about that are automatically read into our request struct. This
    commit adapts this idea to the response: We have an enum of fields
    we might put into our response, and a response-struct holds the
    content of these fields. A function http_send_header() automatically
    sends a header based on the entries in response. In case we don't
    use a field, we just leave the field in the response-struct empty.
    
    With this commit, some logical changes came with it:
    
      - timestamp() now has a sane signature, TIMESTAMP_LEN is no more and
        it can now return proper errors and is also reentrant by using
        gmtime_r() instead of gmtime()
      - No more use of a static timestamp-array, making all the methods
        also reentrant
      - Better internal-error-reporting: Because the fields are filled
        before and not during sending the response-headers, we can better
        report any internal errors as status 500 instead of sending a
        partial non-500-header and then dying.
    
    These improved data structures make it easier to read and hack the code
    and implement new features, if desired.
    
    Signed-off-by: Laslo Hunhold <dev_AT_frign.de>

diff --git a/http.c b/http.c
index 2235c5d..7fe14f1 100644
--- a/http.c
+++ b/http.c
_AT_@ -48,23 +48,76 @@ const char *status_str[] = {
         [S_VERSION_NOT_SUPPORTED] = "HTTP Version not supported",
 };
 
+const char *res_field_str[] = {
+ [RES_ACCEPT_RANGES] = "Accept-Ranges",
+ [RES_ALLOW] = "Allow",
+ [RES_LOCATION] = "Location",
+ [RES_LAST_MODIFIED] = "Last-Modified",
+ [RES_CONTENT_LENGTH] = "Content-Length",
+ [RES_CONTENT_RANGE] = "Content-Range",
+ [RES_CONTENT_TYPE] = "Content-Type",
+};
+
 enum status
-http_send_status(int fd, enum status s)
+http_send_header(int fd, const struct response *res)
 {
- static char t[TIMESTAMP_LEN];
+ char t[FIELD_MAX];
+ size_t i;
+
+ if (timestamp(t, sizeof(t), time(NULL))) {
+ return S_INTERNAL_SERVER_ERROR;
+ }
 
         if (dprintf(fd,
                     "HTTP/1.1 %d %s\r\n"
                     "Date: %s\r\n"
- "Connection: close\r\n"
- "%s"
- "Content-Type: text/html; charset=utf-8\r\n"
- "\r\n"
+ "Connection: close\r\n",
+ res->status, status_str[res->status], t) < 0) {
+ return S_REQUEST_TIMEOUT;
+ }
+
+ for (i = 0; i < NUM_RES_FIELDS; i++) {
+ if (res->field[i][0] != '\0') {
+ if (dprintf(fd, "%s: %s\r\n", res_field_str[i],
+ res->field[i]) < 0) {
+ return S_REQUEST_TIMEOUT;
+ }
+ }
+ }
+
+ if (dprintf(fd, "\r\n") < 0) {
+ return S_REQUEST_TIMEOUT;
+ }
+
+ return res->status;
+}
+
+enum status
+http_send_status(int fd, enum status s)
+{
+ enum status sendstatus;
+
+ struct response res = {
+ .status = s,
+ .field[RES_CONTENT_TYPE] = "text/html; charset=utf-8",
+ };
+
+ if (s == S_METHOD_NOT_ALLOWED) {
+ if (esnprintf(res.field[RES_ALLOW],
+ sizeof(res.field[RES_ALLOW]), "%s",
+ "Allow: GET, HEAD")) {
+ return S_INTERNAL_SERVER_ERROR;
+ }
+ }
+
+ if ((sendstatus = http_send_header(fd, &res)) != s) {
+ return sendstatus;
+ }
+
+ if (dprintf(fd,
                     "<!DOCTYPE html>\n<html>\n\t<head>\n"
                     "\t\t<title>%d %s</title>\n\t</head>\n\t<body>\n"
                     "\t\t<h1>%d %s</h1>\n\t</body>\n</html>\n",
- s, status_str[s], timestamp(time(NULL), t),
- (s == S_METHOD_NOT_ALLOWED) ? "Allow: HEAD, GET\r\n" : "",
                     s, status_str[s], s, status_str[s]) < 0) {
                 return S_REQUEST_TIMEOUT;
         }
_AT_@ -93,7 +146,7 @@ decode(char src[PATH_MAX], char dest[PATH_MAX])
 int
 http_get_request(int fd, struct request *r)
 {
- struct in6_addr res;
+ struct in6_addr addr;
         size_t hlen, i, mlen;
         ssize_t off;
         char h[HEADER_MAX], *p, *q;
_AT_@ -260,7 +313,7 @@ http_get_request(int fd, struct request *r)
                 p = r->field[REQ_HOST] + 1;
 
                 /* validate the contained IPv6 address */
- if (inet_pton(AF_INET6, p, &res) != 1) {
+ if (inet_pton(AF_INET6, p, &addr) != 1) {
                         return http_send_status(fd, S_BAD_REQUEST);
                 }
 
_AT_@ -451,13 +504,14 @@ parse_range(char *s, off_t size, off_t *lower, off_t *upper)
 enum status
 http_send_response(int fd, struct request *r)
 {
- struct in6_addr res;
+ struct in6_addr addr;
+ struct response res = { 0 };
         struct stat st;
         struct tm tm = { 0 };
         size_t len, i;
         off_t lower, upper;
         int hasport, ipv6host;
- static char realtarget[PATH_MAX], tmptarget[PATH_MAX], t[TIMESTAMP_LEN];
+ static char realtarget[PATH_MAX], tmptarget[PATH_MAX];
         char *p, *mime;
         const char *vhostmatch, *targethost;
 
_AT_@ -545,10 +599,12 @@ http_send_response(int fd, struct request *r)
         /* redirect if targets differ, host is non-canonical or we prefixed */
         if (strcmp(r->target, realtarget) || (s.vhost && vhostmatch &&
             strcmp(r->field[REQ_HOST], vhostmatch))) {
+ res.status = S_MOVED_PERMANENTLY;
+
                 /* encode realtarget */
                 encode(realtarget, tmptarget);
 
- /* send redirection header */
+ /* determine target location */
                 if (s.vhost) {
                         /* absolute redirection URL */
                         targethost = r->field[REQ_HOST][0] ? vhostmatch ?
_AT_@ -562,43 +618,31 @@ http_send_response(int fd, struct request *r)
                          * in URLs, so we need to check if our host is one and
                          * honor that later when we fill the "Location"-field */
                         if ((ipv6host = inet_pton(AF_INET6, targethost,
- &res)) < 0) {
+ &addr)) < 0) {
                                 return http_send_status(fd,
                                                         S_INTERNAL_SERVER_ERROR);
                         }
 
- if (dprintf(fd,
- "HTTP/1.1 %d %s\r\n"
- "Date: %s\r\n"
- "Connection: close\r\n"
- "Location: //%s%s%s%s%s%s\r\n"
- "\r\n",
- S_MOVED_PERMANENTLY,
- status_str[S_MOVED_PERMANENTLY],
- timestamp(time(NULL), t),
- ipv6host ? "[" : "",
- targethost,
- ipv6host ? "]" : "", hasport ? ":" : "",
- hasport ? s.port : "", tmptarget) < 0) {
- return S_REQUEST_TIMEOUT;
+ /* write location to response struct */
+ if (esnprintf(res.field[RES_LOCATION],
+ sizeof(res.field[RES_LOCATION]),
+ "//%s%s%s%s%s%s",
+ ipv6host ? "[" : "",
+ targethost,
+ ipv6host ? "]" : "", hasport ? ":" : "",
+ hasport ? s.port : "", tmptarget)) {
+ return http_send_status(fd, S_REQUEST_TOO_LARGE);
                         }
                 } else {
- /* relative redirection URL */
- if (dprintf(fd,
- "HTTP/1.1 %d %s\r\n"
- "Date: %s\r\n"
- "Connection: close\r\n"
- "Location: %s\r\n"
- "\r\n",
- S_MOVED_PERMANENTLY,
- status_str[S_MOVED_PERMANENTLY],
- timestamp(time(NULL), t),
- tmptarget) < 0) {
- return S_REQUEST_TIMEOUT;
+ /* write relative redirection URL to response struct */
+ if (esnprintf(res.field[RES_LOCATION],
+ sizeof(res.field[RES_LOCATION]),
+ tmptarget)) {
+ return http_send_status(fd, S_REQUEST_TOO_LARGE);
                         }
                 }
 
- return S_MOVED_PERMANENTLY;
+ return http_send_header(fd, &res);
         }
 
         if (S_ISDIR(st.st_mode)) {
_AT_@ -635,35 +679,23 @@ http_send_response(int fd, struct request *r)
 
                 /* compare with last modification date of the file */
                 if (difftime(st.st_mtim.tv_sec, timegm(&tm)) <= 0) {
- if (dprintf(fd,
- "HTTP/1.1 %d %s\r\n"
- "Date: %s\r\n"
- "Connection: close\r\n"
- "\r\n",
- S_NOT_MODIFIED, status_str[S_NOT_MODIFIED],
- timestamp(time(NULL), t)) < 0) {
- return S_REQUEST_TIMEOUT;
- }
- return S_NOT_MODIFIED;
+ res.status = S_NOT_MODIFIED;
+ return http_send_header(fd, &res);
                 }
         }
 
         /* range */
         switch (parse_range(r->field[REQ_RANGE], st.st_size, &lower, &upper)) {
         case S_RANGE_NOT_SATISFIABLE:
- if (dprintf(fd,
- "HTTP/1.1 %d %s\r\n"
- "Date: %s\r\n"
- "Content-Range: bytes */%zu\r\n"
- "Connection: close\r\n"
- "\r\n",
- S_RANGE_NOT_SATISFIABLE,
- status_str[S_RANGE_NOT_SATISFIABLE],
- timestamp(time(NULL), t),
- st.st_size) < 0) {
- return S_REQUEST_TIMEOUT;
+ res.status = S_RANGE_NOT_SATISFIABLE;
+
+ if (esnprintf(res.field[RES_CONTENT_RANGE],
+ sizeof(res.field[RES_CONTENT_RANGE]),
+ "bytes */%zu", st.st_size)) {
+ return http_send_status(fd, S_INTERNAL_SERVER_ERROR);
                 }
- return S_RANGE_NOT_SATISFIABLE;
+
+ return http_send_header(fd, &res);
         case S_BAD_REQUEST:
                 return http_send_status(fd, S_BAD_REQUEST);
         default:
diff --git a/http.h b/http.h
index cd1ba22..81f84a6 100644
--- a/http.h
+++ b/http.h
_AT_@ -48,6 +48,25 @@ enum status {
 
 extern const char *status_str[];
 
+enum res_field {
+ RES_ACCEPT_RANGES,
+ RES_ALLOW,
+ RES_LOCATION,
+ RES_LAST_MODIFIED,
+ RES_CONTENT_LENGTH,
+ RES_CONTENT_RANGE,
+ RES_CONTENT_TYPE,
+ NUM_RES_FIELDS,
+};
+
+extern const char *res_field_str[];
+
+struct response {
+ enum status status;
+ char field[NUM_RES_FIELDS][FIELD_MAX];
+};
+
+enum status http_send_header(int, const struct response *);
 enum status http_send_status(int, enum status);
 int http_get_request(int, struct request *);
 enum status http_send_response(int, struct request *);
diff --git a/resp.c b/resp.c
index 4c3d112..63fe11e 100644
--- a/resp.c
+++ b/resp.c
_AT_@ -86,10 +86,14 @@ html_escape(char *src, char *dst, size_t dst_siz)
 enum status
 resp_dir(int fd, char *name, struct request *r)
 {
+ enum status sendstatus;
         struct dirent **e;
+ struct response res = {
+ .status = S_OK,
+ .field[RES_CONTENT_TYPE] = "text/html; charset=utf-8",
+ };
         size_t i;
- int dirlen, s;
- static char t[TIMESTAMP_LEN];
+ int dirlen;
         char esc[PATH_MAX /* > NAME_MAX */ * 6]; /* strlen("&...;") <= 6 */
 
         /* read directory */
_AT_@ -98,14 +102,8 @@ resp_dir(int fd, char *name, struct request *r)
         }
 
         /* send header as late as possible */
- if (dprintf(fd,
- "HTTP/1.1 %d %s\r\n"
- "Date: %s\r\n"
- "Connection: close\r\n"
- "Content-Type: text/html; charset=utf-8\r\n"
- "\r\n",
- S_OK, status_str[S_OK], timestamp(time(NULL), t)) < 0) {
- s = S_REQUEST_TIMEOUT;
+ if ((sendstatus = http_send_header(fd, &res)) != res.status) {
+ res.status = sendstatus;
                 goto cleanup;
         }
 
_AT_@ -117,7 +115,7 @@ resp_dir(int fd, char *name, struct request *r)
                             "<title>Index of %s</title></head>\n"
                             "\t<body>\n\t\t<a href=\"..\">..</a>",
                             esc) < 0) {
- s = S_REQUEST_TIMEOUT;
+ res.status = S_REQUEST_TIMEOUT;
                         goto cleanup;
                 }
 
_AT_@ -135,18 +133,17 @@ resp_dir(int fd, char *name, struct request *r)
                                     (e[i]->d_type == DT_DIR) ? "/" : "",
                                     esc,
                                     suffix(e[i]->d_type)) < 0) {
- s = S_REQUEST_TIMEOUT;
+ res.status = S_REQUEST_TIMEOUT;
                                 goto cleanup;
                         }
                 }
 
                 /* listing footer */
                 if (dprintf(fd, "\n\t</body>\n</html>\n") < 0) {
- s = S_REQUEST_TIMEOUT;
+ res.status = S_REQUEST_TIMEOUT;
                         goto cleanup;
                 }
         }
- s = S_OK;
 
 cleanup:
         while (dirlen--) {
_AT_@ -154,7 +151,7 @@ cleanup:
         }
         free(e);
 
- return s;
+ return res.status;
 }
 
 enum status
_AT_@ -162,51 +159,56 @@ resp_file(int fd, char *name, struct request *r, struct stat *st, char *mime,
           off_t lower, off_t upper)
 {
         FILE *fp;
- enum status s;
+ enum status sendstatus;
+ struct response res = {
+ .status = (r->field[REQ_RANGE][0] != '\0') ?
+ S_PARTIAL_CONTENT : S_OK,
+ .field[RES_ACCEPT_RANGES] = "bytes",
+ };
         ssize_t bread, bwritten;
         off_t remaining;
- int range;
- static char buf[BUFSIZ], *p, t1[TIMESTAMP_LEN], t2[TIMESTAMP_LEN];
+ static char buf[BUFSIZ], *p;
 
         /* open file */
         if (!(fp = fopen(name, "r"))) {
- s = http_send_status(fd, S_FORBIDDEN);
+ res.status = http_send_status(fd, S_FORBIDDEN);
                 goto cleanup;
         }
 
         /* seek to lower bound */
         if (fseek(fp, lower, SEEK_SET)) {
- s = http_send_status(fd, S_INTERNAL_SERVER_ERROR);
+ res.status = http_send_status(fd, S_INTERNAL_SERVER_ERROR);
                 goto cleanup;
         }
 
- /* send header as late as possible */
- range = r->field[REQ_RANGE][0];
- s = range ? S_PARTIAL_CONTENT : S_OK;
-
- if (dprintf(fd,
- "HTTP/1.1 %d %s\r\n"
- "Date: %s\r\n"
- "Connection: close\r\n"
- "Last-Modified: %s\r\n"
- "Content-Type: %s\r\n"
- "Content-Length: %zu\r\n"
- "Accept-Ranges: bytes\r\n",
- s, status_str[s], timestamp(time(NULL), t1),
- timestamp(st->st_mtim.tv_sec, t2), mime,
- upper - lower + 1) < 0) {
- s = S_REQUEST_TIMEOUT;
- goto cleanup;
+ /* build header */
+ if (esnprintf(res.field[RES_CONTENT_LENGTH],
+ sizeof(res.field[RES_CONTENT_LENGTH]),
+ "%zu", upper - lower + 1)) {
+ return http_send_status(fd, S_INTERNAL_SERVER_ERROR);
         }
- if (range) {
- if (dprintf(fd, "Content-Range: bytes %zd-%zd/%zu\r\n",
- lower, upper + (upper < 0), st->st_size) < 0) {
- s = S_REQUEST_TIMEOUT;
- goto cleanup;
+ if (r->field[REQ_RANGE][0] != '\0') {
+ if (esnprintf(res.field[RES_CONTENT_RANGE],
+ sizeof(res.field[RES_CONTENT_RANGE]),
+ "bytes %zd-%zd/%zu", lower, upper,
+ st->st_size)) {
+ return http_send_status(fd, S_INTERNAL_SERVER_ERROR);
                 }
         }
- if (dprintf(fd, "\r\n") < 0) {
- s = S_REQUEST_TIMEOUT;
+ if (esnprintf(res.field[RES_CONTENT_TYPE],
+ sizeof(res.field[RES_CONTENT_TYPE]),
+ "%s", mime)) {
+ return http_send_status(fd, S_INTERNAL_SERVER_ERROR);
+ }
+ if (timestamp(res.field[RES_LAST_MODIFIED],
+ sizeof(res.field[RES_LAST_MODIFIED]),
+ st->st_mtim.tv_sec)) {
+ return http_send_status(fd, S_INTERNAL_SERVER_ERROR);
+ }
+
+ /* send header as late as possible */
+ if ((sendstatus = http_send_header(fd, &res)) != res.status) {
+ res.status = sendstatus;
                 goto cleanup;
         }
 
_AT_@ -217,7 +219,7 @@ resp_file(int fd, char *name, struct request *r, struct stat *st, char *mime,
                 while ((bread = fread(buf, 1, MIN(sizeof(buf),
                                       (size_t)remaining), fp))) {
                         if (bread < 0) {
- s = S_INTERNAL_SERVER_ERROR;
+ res.status = S_INTERNAL_SERVER_ERROR;
                                 goto cleanup;
                         }
                         remaining -= bread;
_AT_@ -225,7 +227,7 @@ resp_file(int fd, char *name, struct request *r, struct stat *st, char *mime,
                         while (bread > 0) {
                                 bwritten = write(fd, p, bread);
                                 if (bwritten <= 0) {
- s = S_REQUEST_TIMEOUT;
+ res.status = S_REQUEST_TIMEOUT;
                                         goto cleanup;
                                 }
                                 bread -= bwritten;
_AT_@ -238,5 +240,5 @@ cleanup:
                 fclose(fp);
         }
 
- return s;
+ return res.status;
 }
diff --git a/util.c b/util.c
index 06dcff3..7ad512f 100644
--- a/util.c
+++ b/util.c
_AT_@ -83,12 +83,17 @@ eunveil(const char *path, const char *permissions)
 #endif /* __OpenBSD__ */
 }
 
-char *
-timestamp(time_t t, char buf[TIMESTAMP_LEN])
+int
+timestamp(char *buf, size_t len, time_t t)
 {
- strftime(buf, TIMESTAMP_LEN, "%a, %d %b %Y %T GMT", gmtime(&t));
+ struct tm tm;
+
+ if (gmtime_r(&t, &tm) == NULL ||
+ strftime(buf, len, "%a, %d %b %Y %T GMT", &tm) == 0) {
+ return 1;
+ }
 
- return buf;
+ return 0;
 }
 
 int
diff --git a/util.h b/util.h
index 7fe65d6..b23a192 100644
--- a/util.h
+++ b/util.h
_AT_@ -49,9 +49,7 @@ void die(const char *, ...);
 void epledge(const char *, const char *);
 void eunveil(const char *, const char *);
 
-#define TIMESTAMP_LEN 30
-
-char *timestamp(time_t, char buf[TIMESTAMP_LEN]);
+int timestamp(char *, size_t, time_t);
 int esnprintf(char *, size_t, const char *, ...);
 
 void *reallocarray(void *, size_t, size_t);
Received on Wed Aug 05 2020 - 13:55:43 CEST

This archive was generated by hypermail 2.3.0 : Wed Aug 05 2020 - 14:00:34 CEST