Abstract some stuff into fastcgi.[ch] + add fastcgi params parser

8 files changed, 407 insertions, 67 deletions

diff --git a/Makefile.am b/Makefile.am
index a950b47..47ce969 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -39,6 +39,7 @@ bin_PROGRAMS=fcgy
 fcgy_LDADD=libdeps.a -lm -lpthread $(EV_LIBS)
 fcgy_SOURCES=\
 	src/app.c\
+	src/fastcgi.c\
 	src/fconn_fastcgi.c\
 	src/main.c\
 	src/util.c
@@ -47,6 +48,7 @@ nodist_fcgy_SOURCES=version.c
 
 noinst_HEADERS=\
 	src/app.h\
+	src/fastcgi.h\
 	src/fcgy.h\
 	src/fconn_fastcgi.h\
 	src/main.h\
@@ -54,6 +56,14 @@ noinst_HEADERS=\
 
 MOSTLYCLEANFILES+=version.c
 
+
+# Tests
+TESTS=test/fastcgi_param_parse
+check_PROGRAMS=$(TESTS)
+test_fastcgi_param_parse_SOURCES=test/fastcgi_param_parse.c src/fastcgi.c
+test_fastcgi_param_parse_LDADD=libdeps.a -lm -lpthread $(EV_LIBS)
+
+
 if USE_GIT_VERSION
 version.c: $(srcdir)/.git/logs/HEAD
 	$(AM_V_GEN)echo 'const char *fcgy_version = "'"`git describe --abbrev=8 --dirty=-d`"'";' >version.c
diff --git a/src/app.c b/src/app.c
index f81a6e4..ac3d217 100644
--- a/src/app.c
+++ b/src/app.c
@@ -47,8 +47,7 @@ static void front_accept_cb(EV_P_ ev_io *w, int revents) {
         return;
     }
 
-    fconn_fastcgi *c = fconn_fastcgi_create(fd, fd);
-    hlist_prepend(f->app->fconns, c);
+    fconn_fastcgi_create(f->app, fd, fd);
     fprintf(stderr, "HI!\n");
 }
 
@@ -173,9 +172,11 @@ void app_run(fcgy_app *app) {
 void app_destroy(fcgy_app *app) {
     while(app->fronts) {
         fcgy_front *n = app->fronts;
-        front_stop(n);
         hlist_remove(app->fronts, n);
+        front_stop(n);
         free(n);
     }
+    while(app->fconns)
+        fconn_fastcgi_destroy(app->fconns);
     free(app);
 }
diff --git a/src/fastcgi.c b/src/fastcgi.c
new file mode 100644
index 0000000..31b72fa
--- /dev/null
+++ b/src/fastcgi.c
@@ -0,0 +1,182 @@
+/* Copyright (c) 2015-2016 Yoran Heling
+
+  Permission is hereby granted, free of charge, to any person obtaining
+  a copy of this software and associated documentation files (the
+  "Software"), to deal in the Software without restriction, including
+  without limitation the rights to use, copy, modify, merge, publish,
+  distribute, sublicense, and/or sell copies of the Software, and to
+  permit persons to whom the Software is furnished to do so, subject to
+  the following conditions:
+
+  The above copyright notice and this permission notice shall be included
+  in all copies or substantial portions of the Software.
+
+  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+  EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+  MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
+  CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+  TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+  SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+*/
+
+#include "fcgy.h"
+
+
+/* Returns -1 if no complete packet has been received, otherwise writes to *h
+ * and returns the length of the first packet in the buffer. */
+static ssize_t parse_header(fastcgi_header *h, char *buf, size_t buflen) {
+    if(buflen < sizeof(fastcgi_header))
+        return -1;
+    memcpy(h, buf, sizeof(fastcgi_header));
+    /* TODO: A proper byteswap in the struct itself seems more efficient and clear */
+    h->requestId     = (((uint8_t)buf[2])<<8) + ((uint8_t)buf[3]);
+    h->contentLength = (((uint8_t)buf[4])<<8) + ((uint8_t)buf[5]);
+
+    size_t len = sizeof(fastcgi_header) + h->contentLength + h->paddingLength;
+    return buflen < len ? -1 : (ssize_t)len;
+}
+
+
+static void reader_consume(fastcgi_reader *r) {
+    size_t len = r->buf_len;
+    char *buf = r->buf;
+    while(true) {
+        fastcgi_header h;
+        ssize_t n = parse_header(&h, buf, len);
+        if(n < 0)
+            break;
+        r->cb(r, n, h, buf);
+        len -= n;
+        buf += n;
+    }
+    memmove(r->buf, buf, len);
+    r->buf_len = len;
+}
+
+
+static void reader_io_cb(EV_P_ ev_io *w, int revents) {
+    fastcgi_reader *r = w->data;
+
+    ssize_t n = read(r->fd, r->buf+r->buf_len, sizeof(r->buf)-r->buf_len);
+    if(n < 0 && (errno == EINTR || errno == EWOULDBLOCK || errno == EAGAIN))
+        return;
+
+    if(n <= 0) {
+        if(n == 0)
+            errno = ECONNRESET;
+        fastcgi_header h = {0};
+        r->cb(r, -1, h, NULL);
+        return;
+    }
+
+    r->buf_len += n;
+    reader_consume(r);
+}
+
+
+void fastcgi_reader_init(fastcgi_reader *r, int fd, fastcgi_reader_cb cb, void *data) {
+    r->fd = fd;
+    r->buf_len = 0;
+    r->data = data;
+    r->cb = cb;
+    ev_io_init(r->io, reader_io_cb, r->fd, EV_READ);
+    r->io->data = r;
+}
+
+
+
+
+static ssize_t param_parse_len(fastcgi_param_parser *p, uint8_t c) {
+    bool next = false;
+    uint32_t *val   = p->state == 0 ? &p->namelen : &p->valuelen;
+    uint32_t maxval = p->state == 0 ? 1<<10 : 1<<20;
+
+    if(p->off == 0)
+        *val = 0;
+
+    if(p->off == 0 && !(c & 0x80)) { /* Single byte */
+        *val = c;
+        next = true;
+    } else { /* 4 bytes */
+        *val = p->off == 0 ? (c & ~0x80) : (*val << 8) + c;
+        next = ++p->off == 4;
+    }
+
+    if(next) {
+        p->state++;
+        p->off = 0;
+        if(*val > maxval)
+            return -1;
+    }
+    return 0;
+}
+
+
+ssize_t fastcgi_param_parse(fastcgi_param_parser *p, const char *buf, size_t len) {
+    /* TODO: Using memcpy() for processing name/value data is likely faster
+     * than this byte-for-byte copying. Also a bit more complex. */
+    while(len > 0) {
+        switch(p->state) {
+
+        case 0: /* Name length */
+        case 1: /* Value length */
+            if(param_parse_len(p, *buf) < 0)
+                return -1;
+            len--;
+            buf++;
+            if(p->state == 2) {
+                p->buf = malloc(p->namelen + p->valuelen + 2);
+                /* Need to special-case zero-length name/value. This is ugly... */
+                if(p->namelen == 0) {
+                    p->buf[p->off++] = 0;
+                    p->state = 3;
+                }
+                if(p->state == 3 && p->valuelen == 0) {
+                    p->buf[p->off] = 0;
+                    p->state = 0;
+                    p->off = 0;
+                    return 2;
+                }
+            }
+            break;
+
+        case 2: /* Name data */
+            if(*buf == 0 || *buf == '=') {
+                free(p->buf);
+                return -1;
+            }
+            p->buf[p->off++] = *buf;
+            len--;
+            buf++;
+            if(p->off == p->namelen) {
+                p->buf[p->off++] = 0;
+                p->state++;
+                if(p->valuelen == 0) {
+                    p->buf[p->off] = 0;
+                    p->state = 0;
+                    p->off = 0;
+                    return p->namelen + 2;
+                }
+            }
+            break;
+
+        case 3: /* Value data */
+            if(*buf == 0) {
+                free(p->buf);
+                return -1;
+            }
+            p->buf[p->off++] = *buf;
+            len--;
+            buf++;
+            if(p->off == p->namelen + p->valuelen + 1) {
+                p->buf[p->off] = 0;
+                p->state = 0;
+                p->off = 0;
+                return p->namelen + p->valuelen + 2;
+            }
+            break;
+        }
+    }
+    return 0;
+}
diff --git a/src/fastcgi.h b/src/fastcgi.h
new file mode 100644
index 0000000..f32a91d
--- /dev/null
+++ b/src/fastcgi.h
@@ -0,0 +1,93 @@
+/* Copyright (c) 2015-2016 Yoran Heling
+
+  Permission is hereby granted, free of charge, to any person obtaining
+  a copy of this software and associated documentation files (the
+  "Software"), to deal in the Software without restriction, including
+  without limitation the rights to use, copy, modify, merge, publish,
+  distribute, sublicense, and/or sell copies of the Software, and to
+  permit persons to whom the Software is furnished to do so, subject to
+  the following conditions:
+
+  The above copyright notice and this permission notice shall be included
+  in all copies or substantial portions of the Software.
+
+  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+  EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+  MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
+  CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+  TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+  SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+*/
+
+#ifndef FCGY_FASTCGI_H
+#define FCGY_FASTCGI_H
+
+typedef struct {
+    uint8_t  version;
+    uint8_t  type;
+    uint16_t requestId;
+    uint16_t contentLength;
+    uint8_t  paddingLength;
+    uint8_t  reserved;
+} fastcgi_header;
+
+typedef struct fastcgi_reader fastcgi_reader;
+
+/* Called when a complete FastCGI packet has arrived. Arguments:
+ *   ssize_t -> -1 on error (see errno), otherwise total length of the packet.
+ *   char *  -> packet buffer, including header.
+ */
+typedef void(*fastcgi_reader_cb)(fastcgi_reader *, ssize_t, fastcgi_header, const char *);
+
+struct fastcgi_reader {
+    int fd;
+    size_t buf_len;
+    /* Read buffer. Large enough to hold a single FastCGI packet.
+     * (should this be a dynamic buffer? Might save some memory and/or optimize read() calls) */
+    char buf[8+65535+255];
+    ev_io io[1];
+    fastcgi_reader_cb cb;
+    void *data;
+};
+
+void fastcgi_reader_init(fastcgi_reader *, int, fastcgi_reader_cb, void *);
+static inline void fastcgi_reader_start(fastcgi_reader *r) { ev_io_start(EV_DEFAULT_UC_ r->io); }
+static inline void fastcgi_reader_stop (fastcgi_reader *r) { ev_io_stop (EV_DEFAULT_UC_ r->io); }
+
+
+
+typedef struct {
+    uint8_t state;
+    uint32_t namelen;
+    uint32_t valuelen;
+    uint32_t off; /* Offset into the current 4-byte length specification or in the name/value buffer */
+    char *buf;
+} fastcgi_param_parser;
+
+/* Consumes the first name/value pair from the given buffer. The parser state
+ * should be zero-initialized before the first call to this function and before
+ * re-using the state struct after an error.
+ *
+ * Returns 0 and updates its internal state if no complete pair is in the
+ * buffer. It can then be called again later with the continuation of the
+ * buffer.
+ *
+ * Returns -1 on error. The following things are considered an error:
+ * - Name contains '='
+ * - Name or value contains \0
+ * - Name is larger than 1 KiB
+ * - Value is larger than 1 MiB
+ * (The length limits are completely arbitrary, but 2 GiB param names/values
+ * are a security nightmare waiting to happen)
+ *
+ * On success it returns the length (including header, name and value) of the
+ * first pair found. The *buf field then points to a newly allocated buffer
+ * starting with the \0-terminated name, and followed by the \0-terminated
+ * value. Ownership of this buffer is passed to the user. The namelen and
+ * valuelen fields are equivalent to a strlen() on the name and value.
+ * (Redundant note: The value starts at buf+namelen+1)
+ */
+ssize_t fastcgi_param_parse(fastcgi_param_parser *, const char *, size_t);
+
+#endif
diff --git a/src/fcgy.h b/src/fcgy.h
index 3b9e946..b3175eb 100644
--- a/src/fcgy.h
+++ b/src/fcgy.h
@@ -61,6 +61,7 @@ typedef struct fcgy_front fcgy_front;
 typedef struct fconn_fastcgi fconn_fastcgi;
 
 #include "util.h"
+#include "fastcgi.h"
 #include "main.h"
 #include "app.h"
 #include "fconn_fastcgi.h"
diff --git a/src/fconn_fastcgi.c b/src/fconn_fastcgi.c
index d39c39e..bba353e 100644
--- a/src/fconn_fastcgi.c
+++ b/src/fconn_fastcgi.c
@@ -22,71 +22,36 @@
 
 #include "fcgy.h"
 
-typedef struct {
-    uint8_t  version;
-    uint8_t  type;
-    uint16_t requestId;
-    uint16_t contentLength;
-    uint8_t  paddingLength;
-    uint8_t  reserved;
-} fastcgi_header;
 
+void read_cb(fastcgi_reader *r, ssize_t len, fastcgi_header h, const char *buf) {
+    fconn_fastcgi *c = r->data;
 
-/* Returns -1 if no complete packet has been received, otherwise writes to *h
- * and returns the length of the first packet in the buffer. */
-static ssize_t fastcgi_parse_header(fastcgi_header *h, char *buf, size_t buflen) {
-    if(buflen < sizeof(fastcgi_header))
-        return -1;
-    memcpy(h, buf, sizeof(fastcgi_header));
-    /* TODO: A proper byteswap in the struct itself seems more efficient and clear */
-    h->requestId     = (((uint8_t)buf[2])<<8) + ((uint8_t)buf[3]);
-    h->contentLength = (((uint8_t)buf[4])<<8) + ((uint8_t)buf[5]);
-
-    size_t len = sizeof(fastcgi_header) + h->contentLength + h->paddingLength;
-    return buflen < len ? -1 : (ssize_t)len;
-}
-
-
-static void io_read_cb(EV_P_ ev_io *w, int revents) {
-    fconn_fastcgi *c = w->data;
-
-    ssize_t r = read(c->rfd, c->rbuf+c->rbuf_len, sizeof(c->rbuf)-c->rbuf_len);
-    if(r < 0 && (errno == EINTR || errno == EWOULDBLOCK || errno == EAGAIN))
-        return;
-
-    if(r <= 0) {
-        fprintf(stderr, "Read error: %s\n", strerror(r == 0 ? ECONNRESET : errno));
-        /* TODO: Destroy connection */
+    if(len < 0) {
+        fconn_fastcgi_destroy(c);
+        fprintf(stderr, "Read error: %s\n", strerror(errno));
         return;
     }
 
-    size_t len = c->rbuf_len + r;
-    char *buf = c->rbuf;
-    while(true) {
-        fastcgi_header h;
-        r = fastcgi_parse_header(&h, buf, len);
-        if(r < 0)
-            break;
-        fprintf(stderr, "Got FastCGI packet: len = %4u, version = %u, type = %2u, requestId = %2u, contentLength = %4u\n",
-            (unsigned)r, (unsigned)h.version, (unsigned)h.type, (unsigned)h.requestId, (unsigned)h.contentLength);
-        /* TODO: Handle packet */
-        len -= r;
-        buf += r;
-    }
-    memmove(c->rbuf, buf, len);
-    c->rbuf_len = len;
+    fprintf(stderr, "Got FastCGI packet: len = %4u, version = %u, type = %2u, requestId = %2u, contentLength = %4u\n",
+        (unsigned)len, (unsigned)h.version, (unsigned)h.type, (unsigned)h.requestId, (unsigned)h.contentLength);
 }
 
 
-fconn_fastcgi *fconn_fastcgi_create(int rfd, int wfd) {
+void fconn_fastcgi_create(fcgy_app *app, int rfd, int wfd) {
     fconn_fastcgi *c = malloc(sizeof(fconn_fastcgi));
+    c->app = app;
 
-    c->rfd = rfd;
-    c->rbuf_len = 0;
-    ev_io_init(c->rio, io_read_cb, c->rfd, EV_READ);
-    ev_io_start(EV_DEFAULT_UC_ c->rio);
-    c->rio->data = c;
+    fastcgi_reader_init(c->rd, rfd, read_cb, c);
+    fastcgi_reader_start(c->rd);
 
     c->wfd = wfd;
-    return c;
+
+    hlist_prepend(app->fconns, c);
+}
+
+
+void fconn_fastcgi_destroy(fconn_fastcgi *c) {
+    fastcgi_reader_stop(c->rd);
+    hlist_remove(c->app->fconns, c);
+    free(c);
 }
diff --git a/src/fconn_fastcgi.h b/src/fconn_fastcgi.h
index 9fac641..8a313af 100644
--- a/src/fconn_fastcgi.h
+++ b/src/fconn_fastcgi.h
@@ -25,16 +25,16 @@
 
 struct fconn_fastcgi {
     fconn_fastcgi *next, *prev;
-    int rfd,
-        wfd;
-
-    /* Read buffer. Large enough to hold a single FastCGI packet.
-     * (should this be a dynamic buffer? Might save some memory and/or optimize read() calls) */
-    size_t rbuf_len;
-    char rbuf[8+65535+255];
-    ev_io rio[1];
+    fcgy_app *app;
+    int wfd;
+
+    fastcgi_reader rd[1];
 };
 
-fconn_fastcgi *fconn_fastcgi_create(int, int);
+
+/* These functions are also responsible for adding/removing the connection in
+ * the app->fconns list */
+void fconn_fastcgi_create(fcgy_app *, int, int);
+void fconn_fastcgi_destroy(fconn_fastcgi *);
 
 #endif
diff --git a/test/fastcgi_param_parse.c b/test/fastcgi_param_parse.c
new file mode 100644
index 0000000..ca3639f
--- /dev/null
+++ b/test/fastcgi_param_parse.c
@@ -0,0 +1,88 @@
+/* Copyright (c) 2015-2016 Yoran Heling
+
+  Permission is hereby granted, free of charge, to any person obtaining
+  a copy of this software and associated documentation files (the
+  "Software"), to deal in the Software without restriction, including
+  without limitation the rights to use, copy, modify, merge, publish,
+  distribute, sublicense, and/or sell copies of the Software, and to
+  permit persons to whom the Software is furnished to do so, subject to
+  the following conditions:
+
+  The above copyright notice and this permission notice shall be included
+  in all copies or substantial portions of the Software.
+
+  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+  EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+  MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
+  CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+  TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+  SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+*/
+
+#include "fcgy.h"
+
+#undef NDEBUG
+#include <assert.h>
+
+#define T(in, nl, vl, b) do {\
+        fastcgi_param_parser p = {0};\
+        assert(fastcgi_param_parse(&p, in, sizeof(in)-1) == sizeof(b)-1);\
+        assert(p.namelen == nl);\
+        assert(p.valuelen == vl);\
+        assert(memcmp(p.buf, b, sizeof(b)-1) == 0);\
+        assert(p.state == 0);\
+        assert(p.off == 0);\
+        free(p.buf);\
+        size_t off = 0;\
+        for(; off<sizeof(in)-2; off++)\
+            assert(fastcgi_param_parse(&p, in+off, 1) == 0);\
+        assert(fastcgi_param_parse(&p, in+off, 1) == sizeof(b)-1);\
+        assert(p.namelen == nl);\
+        assert(p.valuelen == vl);\
+        assert(memcmp(p.buf, b, sizeof(b)-1) == 0);\
+        assert(p.state == 0);\
+        assert(p.off == 0);\
+        free(p.buf);\
+    } while(0)
+
+#define F(in) do {\
+        fastcgi_param_parser p = {0};\
+        assert(fastcgi_param_parse(&p, in, sizeof(in)-1) == -1);\
+        p.state = 0;\
+        p.off = 0;\
+        size_t off = 0;\
+        ssize_t n = 0;\
+        for(; off<sizeof(in)-1; off++)\
+            if((n = fastcgi_param_parse(&p, in+off, 1)) != 0)\
+                break;\
+        assert(n == -1);\
+    } while(0)
+
+#define x127 "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+#define x255 x127 x127 "x"
+
+int main(int argc, char **argv) {
+    T("\013\002SERVER_PORT80", 11, 2, "SERVER_PORT\080\0");
+    T("\x80\0\0\1\x80\0\0\1XY", 1, 1, "X\0Y\0");
+    T("\0\0", 0, 0, "\0\0");
+    T("\1\0x", 1, 0, "x\0\0");
+    T("\0\1x", 0, 1, "\0x\0");
+    T("\1\x80\0\0\0x", 1, 0, "x\0\0");
+    T("\x80\0\0\0\1x", 0, 1, "\0x\0");
+    T("\0\x7f"x127, 0, 127, "\0"x127"\0");
+    T("\x7f\0"x127, 127, 0, x127"\0\0");
+    T("\0\x80\0\0\xff"x255, 0, 255, "\0"x255"\0");
+    T("\x80\0\0\xff\0"x255, 255, 0, x255"\0\0");
+    T("\0\x80\0\1\0x"x255, 0, 256, "\0x"x255"\0");
+    T("\x80\0\1\0\0x"x255, 256, 0, x255"x\0\0");
+    T("\5\6./,';=./,';", 5, 6, "./,';\0=./,';\0");
+
+    F("\2\0x=");
+    F("\2\0x\0");
+    F("\0\2x\0");
+    F("\0\2x\0");
+    F("\x80\0\4\1\0");
+    F("\0\x80\x10\0\1");
+    return 0;
+}