Refactor http_send_buf() and http_recv_header() and simplify buffer - quark - quark web server

git clone git://git.suckless.org/quark

Log

Files

Refs

LICENSE

---

commit e67eddd517d3f20a24ee14e1eab92806ce5bb20c

parent 1a93e3b22f363a0a8dffcf3c47b8d30ff207401f

Author: Laslo Hunhold 

Date:   Wed, 16 Sep 2020 10:56:45 +0200

Refactor http_send_buf() and http_recv_header() and simplify buffer

The use of "off" was problematic, because it is state that has to be
handled carefully and could potentially hit edge-cases (e.g. being
larger than sizeof(buf->data)).

The real problem is finding a way to indicate function is "done" with
something (i.e. done filling or sending the buffer). Instead of playing
around with off, which is nothing but a dirty hack, http_send_buf() now
just "drains" the buffer, which also makes it idempotent on an empty
buffer. It indicates to be "done" as soon as the length of the buffer is
zero.

The function http_recv_header() gets an additional paramter "done"
(an int-pointer) which is used to indicate completeness.

As a result, we can drop the off-variable completely from the buffer.

Signed-off-by: Laslo Hunhold 

Diffstat:
  M http.c                              |      41 +++++++++++--------------------
  M http.h                              |       2 +-
  M util.h                              |       1 -

3 files changed, 15 insertions(+), 29 deletions(-)
---

diff --git a/http.c b/http.c

@@ -101,25 +101,20 @@ err:
 enum status
 http_send_buf(int fd, struct buffer *buf)
 {
-        size_t remaining;
         ssize_t r;
 
-        if (buf == NULL || buf->off > sizeof(buf->data)) {
+        if (buf == NULL) {
                 return S_INTERNAL_SERVER_ERROR;
         }
 
-        remaining = buf->len - buf->off;
-        while (remaining > 0) {
-                if ((r = write(fd, buf->data + buf->off, remaining)) <= 0) {
+        while (buf->len > 0) {
+                if ((r = write(fd, buf->data, buf->len)) <= 0) {
                         return S_REQUEST_TIMEOUT;
                 }
-                buf->off += r;
-                remaining -= r;
+                memmove(buf->data, buf->data + r, buf->len - r);
+                buf->len -= r;
         }
 
-        /* set off to 0 to indicate that we have finished */
-        buf->off = 0;
-
         return 0;
 }
 
@@ -142,43 +137,35 @@ decode(const char src[PATH_MAX], char dest[PATH_MAX])
 }
 
 enum status
-http_recv_header(int fd, struct buffer *buf)
+http_recv_header(int fd, struct buffer *buf, int *done)
 {
         enum status s;
         ssize_t r;
 
-        if (buf->off > sizeof(buf->data)) {
-                s = S_INTERNAL_SERVER_ERROR;
-                goto err;
-        }
-
         while (1) {
-                if ((r = read(fd, buf->data + buf->off,
-                              sizeof(buf->data) - buf->off)) <= 0) {
+                if ((r = read(fd, buf->data + buf->len,
+                              sizeof(buf->data) - buf->len)) <= 0) {
                         s = S_REQUEST_TIMEOUT;
                         goto err;
                 }
-                buf->off += r;
+                buf->len += r;
 
                 /* check if we are done (header terminated) */
-                if (buf->off >= 4 && !memcmp(buf->data + buf->off - 4,
+                if (buf->len >= 4 && !memcmp(buf->data + buf->len - 4,
                                              "\r\n\r\n", 4)) {
                         break;
                 }
 
                 /* buffer is full or read over, but header is not terminated */
-                if (r == 0 || buf->off == sizeof(buf->data)) {
+                if (r == 0 || buf->len == sizeof(buf->data)) {
                         s = S_REQUEST_TOO_LARGE;
                         goto err;
                 }
         }
 
-        /* header is complete, remove last \r\n and null-terminate */
-        buf->data[buf->off - 2] = '\0';
-
-        /* set buffer length to length and offset to 0 to indicate success */
-        buf->len = buf->off - 2;
-        buf->off = 0;
+        /* header is complete, remove last \r\n and set done */
+        buf->len -= 2;
+        *done = 1;
 
         return 0;
 err:

diff --git a/http.h b/http.h

@@ -101,7 +101,7 @@ struct connection {
 
 enum status http_prepare_header_buf(const struct response *, struct buffer *);
 enum status http_send_buf(int, struct buffer *);
-enum status http_recv_header(int, struct buffer *);
+enum status http_recv_header(int, struct buffer *, int *);
 enum status http_parse_header(const char *, struct request *);
 void http_prepare_response(const struct request *, struct response *,
                            const struct server *);

diff --git a/util.h b/util.h

@@ -38,7 +38,6 @@ struct server {
 struct buffer {
         char data[BUFFER_SIZE];
         size_t len;
-        size_t off;
 };
 
 #undef MIN