From 98f853c8ac5965c8c9bd8729f90668b54a599878 Mon Sep 17 00:00:00 2001 From: Neale Pickett Date: Fri, 24 Feb 2012 17:03:09 -0700 Subject: [PATCH] found another fnord bug --- README | 2 +- break-fnord.sh | 14 +++- eris.c | 179 ++++++++++++++++++++++++++----------------------- tests.sh | 116 ++++++++++++++++++++++++++++++++ tests/test.py | 5 ++ 5 files changed, 229 insertions(+), 87 deletions(-) create mode 100755 tests.sh diff --git a/README b/README index a49f505..d8f51d9 100644 --- a/README +++ b/README @@ -12,7 +12,7 @@ Significant differences between eris and fnord are: * elimination of user switching (you can use tcpserver -[ug]) * elimination of chroot code (you can use chroot) * several bugfixes (sent to the fnord mail list) -* removal of (non-functional) content-negotiation +* ignores Accept header (fnord does too) ---- diff --git a/break-fnord.sh b/break-fnord.sh index 70131e6..1992717 100755 --- a/break-fnord.sh +++ b/break-fnord.sh @@ -39,6 +39,10 @@ fail () { failures=$(expr $failures + 1) } +d () { + tr '\r\n' '#%' +} + if [ ! -f fnord-1.10.tar.bz2 ]; then wget http://www.fefe.de/fnord/fnord-1.10.tar.bz2 @@ -82,7 +86,7 @@ printf 'GET / HTTP/1.0\r\nHost: empty\r\n\r\n' | $HTTPD_IDX 2>/dev/null | grep - # 2. Should output \r\n\r\n; instead outputs \r\n\n title "CGI output bare newlines" -printf 'GET /a.cgi HTTP/1.0\r\n\r\n' | $HTTPD_CGI 2>/dev/null | grep -qU '^\r$' && pass || fail +printf 'GET /a.cgi HTTP/1.0\r\n\r\n' | $HTTPD_CGI 2>/dev/null | d | grep -q '#%#%' && pass || fail # 3. Should process both requests; instead drops second title "Multiple requests in one packet" @@ -102,7 +106,13 @@ title "Second MIME-Type" title "POST to static HTML" (printf 'POST / HTTP/1.1\r\nHost: a\r\nConnection: keep-alive\r\nContent-Type: text/plain\r\nContent-Length: 1\r\n\r\n'; ls / > /dev/null - printf 'aPOST / HTTP/1.1\r\nHost: a\r\nConnection: keep-alive\r\nContent-Type: text/plain\r\nContent-Length: 1\r\n\r\na') | $HTTPD 2>/dev/null | grep -c '^HTTP/1.' | grep -q 2 && pass || fail + printf 'aPOST / HTTP/1.1\r\nHost: a\r\nConnection: keep-alive\r\nContent-Type: text/plain\r\nContent-Length: 1\r\n\r\na') | $HTTPD 2>/dev/null | grep -c '200 OK' | grep -q 2 && pass || fail + +# 7. HTTP/1.1 should default to keepalive; instead connection is closed +title "HTTP/1.1 default keepalive" +(printf 'GET / HTTP/1.1\r\nHost: a\r\n\r\n' + ls / >/dev/null + printf 'GET / HTTP/1.1\r\nHost: a\r\n\r\n') | $HTTPD 2>/dev/null | grep -c '^HTTP/' | grep -q 2 && pass || fail cat < (1ul << 31)) ? 1ul << 31 : l; if (sendfile(1, fd, &offset, c) == -1) -#ifdef USE_MMAP +# ifdef USE_MMAP return serve_mmap(fd); -#else +# else return serve_read_write(fd); -#endif +# endif l -= c; } while (l); } @@ -1317,18 +1319,14 @@ serve_static_data(int fd) } #else fflush(stdout); -#ifdef TCP_CORK - { - int one = 1; - setsockopt(1, IPPROTO_TCP, TCP_CORK, &one, sizeof(one)); - corked = 1; - } -#endif -#ifdef USE_MMAP + + /* XXX: Is this really the right place to uncork? */ + cork(0); +# ifdef USE_MMAP return serve_mmap(fd); -#else +# else return serve_read_write(fd); -#endif +# endif #endif } @@ -1337,10 +1335,11 @@ int main(int argc, char *argv[], const char *const *envp) { char headerbuf[MAXHEADERLEN]; - char *nurl, - *origurl; + char *url, + *nurl; int doauth = 0; int docgi = 0; + int dokeepalive = 1; int dirlist = 0; int redirect = 0; int portappend = 0; @@ -1349,7 +1348,7 @@ main(int argc, char *argv[], const char *const *envp) { int opt; - while (-1 != (opt = getopt(argc, argv, "acdrp"))) { + while (-1 != (opt = getopt(argc, argv, "acdhkprv"))) { switch (opt) { case 'a': doauth = 1; @@ -1360,15 +1359,29 @@ main(int argc, char *argv[], const char *const *envp) case 'd': dirlist = 1; break; - case 'r': - redirect = 1; + case 'k': + dokeepalive = 0; break; case 'p': portappend = 1; break; + case 'r': + redirect = 1; + break; + case 'v': + printf(FNORD "\n"); + return 0; default: - fprintf(stderr, "Usage: %s [-c] [-d] [-r] [-p]\n", + fprintf(stderr, "Usage: %s [OPTIONS]\n", argv[0]); + fprintf(stderr, "\n"); + fprintf(stderr, "-a Enable authentication\n"); + fprintf(stderr, "-c Enable CGI\n"); + fprintf(stderr, "-d Enable directory listing\n"); + fprintf(stderr, "-k No default keepalive with HTTP/1.1\n"); + fprintf(stderr, "-p Append port to hostname directory\n"); + fprintf(stderr, "-r Enable symlink redirection\n"); + fprintf(stderr, "-v Print version and exit\n"); return 69; } } @@ -1381,10 +1394,12 @@ main(int argc, char *argv[], const char *const *envp) handlenext: + /* This is cancelled by later calls to alarm */ alarm(READTIMEOUT); headerlen = sizeof headerbuf; - // XXX: I need a way to signal that this is a new read with the same buffer. + /* Clear static variables. This is such a kludge. I should fix it. */ + read_header(NULL, NULL, NULL); switch (read_header(stdin, headerbuf, &headerlen)) { case -1: return 0; @@ -1394,24 +1409,22 @@ main(int argc, char *argv[], const char *const *envp) break; } - alarm(0); - if (headerlen < 10) badrequest(400, "Bad Request", "That does not look like HTTP"); if (!memcmp(headerbuf, "GET /", 5)) { method = GET; - url = headerbuf + 4; + path = headerbuf + 4; } else if (!memcmp(headerbuf, "POST /", 6)) { method = POST; - url = headerbuf + 5; + path = headerbuf + 5; } else if (!memcmp(headerbuf, "HEAD /", 6)) { method = HEAD; - url = headerbuf + 5; + path = headerbuf + 5; } else badrequest(405, "Method Not Allowed", "Unsupported HTTP method."); - origurl = url; + url = path; { /* @@ -1431,7 +1444,11 @@ main(int argc, char *argv[], const char *const *envp) if (httpversion > 1) { badrequest(400, "Bad Request", "HTTP Version not supported"); } - keepalive = 0; + if (httpversion > 0) { + keepalive = dokeepalive; + } else { + keepalive = 0; + } /* * demangle path in-place @@ -1470,10 +1487,7 @@ main(int argc, char *argv[], const char *const *envp) *d = 0; } - /* - * XXX: check use of uri to see if it needs to be duplicated - */ - uri = strdup(url); + strncpy(rpath, url, sizeof rpath); } { @@ -1665,12 +1679,7 @@ main(int argc, char *argv[], const char *const *envp) if ((method == HEAD)) badrequest(400, "Bad Request", "Illegal HTTP method for Gateway call."); -#ifdef TCP_CORK - { - int one = 1; - setsockopt(1, IPPROTO_TCP, TCP_CORK, &one, sizeof(one)); - } -#endif + cork(1); for (i = nurl - url; i > -1; --i) { if ((nurl[0] == '/') && (nurl[1] == 'n') && (nurl[2] == 'p') && (nurl[3] == 'h') @@ -1732,6 +1741,11 @@ main(int argc, char *argv[], const char *const *envp) (unsigned long long) st.st_size); } fputs("\r\n", stdout); + + for (; post_len; post_len -= 1) { + getchar(); + } + if (method == GET || method == POST) { int ret; @@ -1747,13 +1761,7 @@ main(int argc, char *argv[], const char *const *envp) case 1: return 1; } -#ifdef TCP_CORK - if (corked) { - int zero = 0; - setsockopt(1, IPPROTO_TCP, TCP_CORK, &zero, - sizeof(zero)); - } -#endif + cork(0); if (keepalive > 0) { close(fd); fchdir(rootdir); @@ -1763,8 +1771,11 @@ main(int argc, char *argv[], const char *const *envp) exit(0); error500: retcode = 500; - } else + } else { fflush(stdout); + } + } else { + retcode = 404; } } switch (retcode) { @@ -1772,11 +1783,11 @@ main(int argc, char *argv[], const char *const *envp) { char *space = alloca(strlen(url) + 2); - if (handleindexcgi(url, origurl, space)) + if (handleindexcgi(url, path, space)) goto indexcgi; - handleredirect(url, origurl); + handleredirect(url, path); if (dirlist) { - handledirlist(origurl); + handledirlist(path); } badrequest(404, "Not Found", "No such file or directory."); } @@ -1789,5 +1800,5 @@ main(int argc, char *argv[], const char *const *envp) case 500: badrequest(500, "Internal Server Error", ""); } - return 1; + return 0; } diff --git a/tests.sh b/tests.sh new file mode 100755 index 0000000..0caee20 --- /dev/null +++ b/tests.sh @@ -0,0 +1,116 @@ +#! /bin/sh + +: ${HTTPD:=./eris} +: ${HTTPD_CGI:=./eris -c} +: ${HTTPD_IDX:=./eris -d} + +H () { + echo + echo "$@" + echo "===================================" + echo +} + +BR () { + echo + echo "-----------------------------------" + echo +} + +title() { + printf "* %-50s: " "$1" + tests=$(expr $tests + 1) +} + +successes=0 +pass () { + echo 'ok' + successes=$(expr $successes + 1) +} + +failures=0 +fail () { + echo 'FAIL' + failures=$(expr $failures + 1) +} + +d () { + tr '\r\n' '#%' +} + + +if [ ! -d default ]; then + mkdir default + echo james > default/index.html + touch default/a + cat < default/a.cgi +#! /bin/sh +echo 'Content-type: text/plain' +ls / > /dev/null # delay a little +echo +echo james +EOD + chmod +x default/a.cgi + mkdir empty:80 +fi + +echo "HTTPD: $HTTPD " +echo "CGI: $HTTPD_CGI " +echo "IDX: $HTTPD_IDX " + +H "Basic tests" + +title "GET" +printf 'GET / HTTP/1.0\r\n\r\n' | $HTTPD 2>/dev/null | d | grep -q 'HTTP/1.0 200 OK#%Server: [a-z]*/[0-9.]*#%Content-Type: text/html; charset=UTF-8#%Content-Length: 6#%Last-Modified: ..., .. ... 20.. ..:..:.. GMT#%#%james%' && pass || fail + +title "POST" +printf 'POST / HTTP/1.0\r\nContent-Type: a\r\nContent-Length: 5\r\n\r\njames' | $HTTPD 2>/dev/null | d | grep -q 'HTTP/1.0 200 OK#%Server: [a-z]*/[0-9.]*#%Content-Type: text/html; charset=UTF-8#%Content-Length: 6#%Last-Modified: ..., .. ... 20.. ..:..:.. GMT#%#%james%' && pass || fail + +title "Logging /" +(printf 'GET / HTTP/1.1\r\nHost: host\r\n\r\n' | + PROTO=TCP TCPREMOTEPORT=1234 TCPREMOTEIP=10.0.0.2 $HTTPD >/dev/null) 2>&1 | grep -q '^10.0.0.2 200 6 host (null) (null) /$' && pass || fail + +title "Logging /index.html" +(printf 'GET /index.html HTTP/1.1\r\nHost: host\r\n\r\n' | + PROTO=TCP TCPREMOTEPORT=1234 TCPREMOTEIP=10.0.0.2 $HTTPD >/dev/null) 2>&1 | grep -q '^10.0.0.2 200 6 host (null) (null) /index.html$' && pass || fail + +H "fnord bugs" + +# 1. Should return directory listing of /; instead segfaults +title "Directory indexing of /" +printf 'GET / HTTP/1.0\r\nHost: empty\r\n\r\n' | $HTTPD_IDX 2>/dev/null | grep -q 200 && pass || fail + +# 2. Should output \r\n\r\n; instead outputs \r\n\n +title "CGI output bare newlines" +printf 'GET /a.cgi HTTP/1.0\r\n\r\n' | $HTTPD_CGI 2>/dev/null | d | grep -q '#%#%' && pass || fail + +# 3. Should process both requests; instead drops second +title "Multiple requests in one packet" +printf 'GET / HTTP/1.1\r\nHost: a\r\nConnection: keep-alive\r\n\r\nGET / HTTP/1.1\r\nHost: a\r\nConnection: keep-alive\r\n\r\n' | $HTTPD 2>/dev/null | grep -c '^HTTP/1.' | grep -q 2 && pass || fail + +# 4. Should return 406 Not Acceptable; instead ignores Accept header +title "Accept header" +printf 'GET / HTTP/1.0\r\nAccept: nothing\r\n\r\n' | $HTTPD 2>/dev/null | grep 406 && pass || fail + +# 5. Should serve second request as default MIME-Type (text/plain); instead uses previous mime type +title "Second MIME-Type" +(printf 'GET / HTTP/1.1\r\nHost: a\r\nConnection: keep-alive\r\n\r\n' + ls / > /dev/null # Delay required to work around test #3 + printf 'GET /a HTTP/1.1\r\nHost: a\r\nConnection: keep-alive\r\n\r\n') | $HTTPD 2>/dev/null | grep -q 'text/plain\|application/octet-stream' && pass || fail + +# 6. Should consume POST data; instead tries to read POST data as second request +title "POST to static HTML" +(printf 'POST / HTTP/1.1\r\nHost: a\r\nConnection: keep-alive\r\nContent-Type: text/plain\r\nContent-Length: 1\r\n\r\n'; + ls / > /dev/null + printf 'aPOST / HTTP/1.1\r\nHost: a\r\nConnection: keep-alive\r\nContent-Type: text/plain\r\nContent-Length: 1\r\n\r\na') | $HTTPD 2>/dev/null | grep -c '200 OK' | grep -q 2 && pass || fail + +# 7. HTTP/1.1 should default to keepalive; instead connection is closed +title "HTTP/1.1 default keepalive" +(printf 'GET / HTTP/1.1\r\nHost: a\r\n\r\n' + ls / >/dev/null + printf 'GET / HTTP/1.1\r\nHost: a\r\n\r\n') | $HTTPD 2>/dev/null | grep -c '^HTTP/' | grep -q 2 && pass || fail + +BR +echo "$successes of $tests tests passed ($failures failed)." + +exit $failures diff --git a/tests/test.py b/tests/test.py index 2818daf..017577d 100755 --- a/tests/test.py +++ b/tests/test.py @@ -110,6 +110,11 @@ class CGITests(BasicTests): self.assertLinesEqual(se, b'10.1.2.3 200 306 default (null) (null) /cgi/set.cgi\n') self.assertLinesEqual(so, b'HTTP/1.0 200 OK\r\nServer: eris/2.0\r\nPragma: no-cache\r\nConnection: close\r\nContent-Type: text/plain\r\n\r\nGATEWAY_INTERFACE:CGI/1.1\nSERVER_PROTOCOL:HTTP/1.0\nSERVER_SOFTWARE:eris/2.0\nSERVER_NAME:default\nSERVER_PORT:80\nREQUEST_METHOD:POST\nREQUEST_URI:/cgi/set.cgi\nSCRIPT_NAME:/cgi/set.cgi\nREMOTE_ADDR:10.1.2.3\nREMOTE_PORT:5858\nCONTENT_TYPE:application/x-www-form-urlencoded\nCONTENT_LENGTH:11\nForm data: a=1&b=2&c=3') +class BufferingTests(BasicTests): + def testDoubleGet(self): + so, se = self.p.communicate(b'GET / HTTP/1.0\r\n\r\nGET / HTTP/1.0\r\n\r\n') + self.assertLinesEqual(so, b'') + # XXX: Test posting to static html with keepalive # (it probably won't discard content-length octets)