=== modified file 'src/HttpMsg.cc' --- src/HttpMsg.cc 2010-08-24 20:35:02 +0000 +++ src/HttpMsg.cc 2010-08-29 06:17:10 +0000 @@ -394,15 +394,28 @@ void HttpParserInit(HttpParser *hdr, const char *buf, int bufsiz) { + hdr->clear(); hdr->state = 1; - hdr->request_parse_status = HTTP_STATUS_NONE; hdr->buf = buf; hdr->bufsiz = bufsiz; - hdr->req_start = hdr->req_end = -1; - hdr->hdr_start = hdr->hdr_end = -1; debugs(74, 5, "httpParseInit: Request buffer is " << buf); } +void +HttpParser::clear() +{ + state = 0; // ? + buf = NULL; + bufsiz = 0; + req_start = req_end = -1; + hdr_start = hdr_end = -1; + m_start = m_end = -1; + u_start = u_end = -1; + v_start = v_end = -1; + v_maj = v_min = 0; + request_parse_status = HTTP_STATUS_NONE; +} + #if MSGDODEBUG /* XXX This should eventually turn into something inlined or #define'd */ int @@ -484,9 +497,15 @@ } } if (hmsg->req_end == -1) { - retcode = 0; - goto finish; + PROF_stop(HttpParserParseReqLine); + debugs(74, 5, "Parser: retval 0: from " << hmsg->req_start << + "->" << hmsg->req_end << ": needs more data to complete first line."); + return 0; // This is the only spot in this function where (0) is valid result. } + + // NP: we have now seen EOL, more-data (0) cannot occur. + // From here on any failure is -1, success is 1 + assert(hmsg->buf[hmsg->req_end] == '\n'); /* Start at the beginning again */ i = 0; @@ -494,7 +513,7 @@ /* Find first non-whitespace - beginning of method */ for (; i < hmsg->req_end && (xisspace(hmsg->buf[i])); i++); if (i >= hmsg->req_end) { - retcode = 0; + retcode = -1; goto finish; } hmsg->m_start = i; @@ -503,7 +522,7 @@ /* Find first whitespace - end of method */ for (; i < hmsg->req_end && (! xisspace(hmsg->buf[i])); i++); if (i >= hmsg->req_end) { - retcode = 0; + retcode = -1; goto finish; } hmsg->m_end = i - 1; @@ -511,7 +530,7 @@ /* Find first non-whitespace - beginning of URL+Version */ for (; i < hmsg->req_end && (xisspace(hmsg->buf[i])); i++); if (i >= hmsg->req_end) { - retcode = 0; + retcode = -1; goto finish; } hmsg->u_start = i; @@ -534,7 +553,7 @@ } } if (i > hmsg->req_end) { - retcode = 0; + retcode = -1; goto finish; } @@ -555,7 +574,7 @@ /* XXX why <= vs < ? I do need to really re-audit all of this ..*/ for (i = last_whitespace; i <= hmsg->req_end && xisspace(hmsg->buf[i]); i++); if (i > hmsg->req_end) { - retcode = 0; + retcode = -1; goto finish; } @@ -581,17 +600,17 @@ goto finish; } if (i >= hmsg->req_end) { - retcode = 0; + retcode = -1; goto finish; } /* next should be .; we -have- to have this as we have a whole line.. */ if (hmsg->buf[i] != '.') { - retcode = 0; + retcode = -1; goto finish; } if (i + 1 >= hmsg->req_end) { - retcode = 0; + retcode = -1; goto finish; } @@ -631,4 +650,3 @@ return retcode; } - === modified file 'src/HttpMsg.h' --- src/HttpMsg.h 2010-08-19 00:12:43 +0000 +++ src/HttpMsg.h 2010-08-29 03:04:53 +0000 @@ -130,6 +130,10 @@ class HttpParser { public: + /// reset all fields to empty values. + void clear(); + +public: char state; const char *buf; int bufsiz; === modified file 'src/tests/testHttpRequest.cc' --- src/tests/testHttpRequest.cc 2010-08-16 14:47:39 +0000 +++ src/tests/testHttpRequest.cc 2010-08-29 04:03:49 +0000 @@ -199,3 +199,338 @@ input.reset(); error = HTTP_STATUS_NONE; } + +void +testHttpRequest::testParseRequestLine() +{ + MemBuf input; + HttpParser output; + size_t hdr_len; + input.init(); + + // Valid States + + // HTTP/0.9 simple-request + input.append("GET /\r\n", 7); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == 1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); + CPPUNIT_ASSERT_EQUAL(output.v_maj, 0); + CPPUNIT_ASSERT_EQUAL(output.v_min, 9); +// TODO : check "GET" +// TODO : check "/" + input.reset(); + output.clear(); + + // HTTP/1.0 full-request + input.append("GET / HTTP/1.0\r\n", 16); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == 1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); + CPPUNIT_ASSERT_EQUAL(output.v_maj, 1); + CPPUNIT_ASSERT_EQUAL(output.v_min, 0); +// TODO : check "GET" +// TODO : check "/" + input.reset(); + output.clear(); + + // HTTP/1.1 full-request + input.append("GET / HTTP/1.1\r\n", 16); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == 1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); + CPPUNIT_ASSERT_EQUAL(output.v_maj, 1); + CPPUNIT_ASSERT_EQUAL(output.v_min, 1); +// TODO : check "GET" +// TODO : check "/" + input.reset(); + output.clear(); + + // future version full-request + input.append("GET / HTTP/10.12\r\n", 18); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == 1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); +// TODO : check "GET" +// TODO : check "/" +// TODO : check "HTTP" + CPPUNIT_ASSERT_EQUAL(output.v_maj, 10); + CPPUNIT_ASSERT_EQUAL(output.v_min, 12); + input.reset(); + output.clear(); + + // extra fluffy whitespace. + input.append("GET / HTTP/1.1\r\n", 21); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == 1); + CPPUNIT_ASSERT_EQUAL(output.req_start, 0); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.contentSize()); +// TODO : check "GET" +// TODO : check "/" +// TODO : check "HTTP" + CPPUNIT_ASSERT_EQUAL(output.v_maj, 1); + CPPUNIT_ASSERT_EQUAL(output.v_min, 1); + input.reset(); + output.clear(); + + // additional data in buffer + input.append("GET / HTTP/1.1\nboo!", 24); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == 1); + CPPUNIT_ASSERT_EQUAL(output.req_start, 0); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.contentSize() - 4); +// TODO : check "GET" +// TODO : check "/" +// TODO : check "HTTP" + CPPUNIT_ASSERT_EQUAL(output.v_maj, 1); + CPPUNIT_ASSERT_EQUAL(output.v_min, 1); + input.reset(); + output.clear(); + + // alternative EOL sequence: NL-only + input.append("GET / HTTP/1.1\n", 15); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == 1); + CPPUNIT_ASSERT_EQUAL(output.req_start, 0); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.contentSize()); +// TODO : check "GET" +// TODO : check "/" +// TODO : check "HTTP" + CPPUNIT_ASSERT_EQUAL(output.v_maj, 1); + CPPUNIT_ASSERT_EQUAL(output.v_min, 1); + input.reset(); + output.clear(); + + // alternative EOL sequence: double-NL-only + input.append("GET / HTTP/1.1\n\n", 16); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == 1); + CPPUNIT_ASSERT_EQUAL(output.req_start, 0); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.contentSize() - 1); +// TODO : check "GET" +// TODO : check "/" +// TODO : check "HTTP" + CPPUNIT_ASSERT_EQUAL(output.v_maj, 1); + CPPUNIT_ASSERT_EQUAL(output.v_min, 1); + input.reset(); + output.clear(); + + // . method + input.append(". / HTTP/1.1\n", 13); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == 1); + CPPUNIT_ASSERT_EQUAL(output.req_start, 0); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.contentSize()); +// TODO : check "." +// TODO : check "/" +// TODO : check "HTTP" + CPPUNIT_ASSERT_EQUAL(output.v_maj, 1); + CPPUNIT_ASSERT_EQUAL(output.v_min, 1); + input.reset(); + output.clear(); + + // OPTIONS with * URL + input.append("OPTIONS * HTTP/1.1\n", 19); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == 1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); +// TODO : check "OPTIONS" +// TODO : check "*" +// TODO : check "HTTP" + CPPUNIT_ASSERT_EQUAL(output.v_maj, 1); + CPPUNIT_ASSERT_EQUAL(output.v_min, 1); + input.reset(); + output.clear(); + + // unknown method + input.append("HELLOWORLD / HTTP/1.1\n", 22); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == 1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); +// TODO : check "HELLOWORLD" +// TODO : check "/" +// TODO : check "HTTP" + CPPUNIT_ASSERT_EQUAL(output.v_maj, 1); + CPPUNIT_ASSERT_EQUAL(output.v_min, 1); + input.reset(); + output.clear(); + + // This stage of the parser also accepts non-HTTP protocol names + input.append("GET / FOO/1.0\n", 14); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == 1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); +// TODO : check "GET" +// TODO : check "/" +// TODO : check "FOO" + CPPUNIT_ASSERT_EQUAL(output.v_maj, 1); + CPPUNIT_ASSERT_EQUAL(output.v_min, 0); + input.reset(); + output.clear(); + + +// Error States. + + // method-only + input.append("A\n", 2); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == -1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); +// TODO : check "A" + // other fields should be in unset state still... + CPPUNIT_ASSERT_EQUAL(output.v_maj, 0); + CPPUNIT_ASSERT_EQUAL(output.v_min, 0); + CPPUNIT_ASSERT_EQUAL(output.v_start, -1); + CPPUNIT_ASSERT_EQUAL(output.v_end, -1); + input.reset(); + output.clear(); + + // no method + input.append("/ HTTP/1.0\n", 11); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == -1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); +// TODO : check "/" (request) + // other fields should be in unset state still... + CPPUNIT_ASSERT_EQUAL(output.v_maj, 0); + CPPUNIT_ASSERT_EQUAL(output.v_min, 0); + CPPUNIT_ASSERT_EQUAL(output.v_start, -1); + CPPUNIT_ASSERT_EQUAL(output.v_end, -1); + input.reset(); + output.clear(); + + // binary code in method (error or not?) + input.append("GET\0xA / HTTP/1.1\n", 16); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == -1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); +// TODO : check "GET\0xA" +// TODO : check "/" +// TODO : check "HTTP" + CPPUNIT_ASSERT_EQUAL(output.v_maj, 1); + CPPUNIT_ASSERT_EQUAL(output.v_min, 1); + input.reset(); + output.clear(); + + // no URL (grammer otherwise correct) + input.append("GET HTTP/1.1\n", 14); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == -1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); +// TODO : check "GET" + // other fields should be in unset state still... + CPPUNIT_ASSERT_EQUAL(output.v_maj, 0); + CPPUNIT_ASSERT_EQUAL(output.v_min, 0); + input.reset(); + output.clear(); + + // no URL (grammer invalid) + input.append("GET HTTP/1.1\n", 13); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == -1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); +// TODO : check "GET" + // other fields should be in unset state still... + CPPUNIT_ASSERT_EQUAL(output.v_maj, 0); + CPPUNIT_ASSERT_EQUAL(output.v_min, 0); + CPPUNIT_ASSERT_EQUAL(output.v_start, -1); + CPPUNIT_ASSERT_EQUAL(output.v_end, -1); + input.reset(); + output.clear(); + + // no protocol + input.append("GET HTTP/1.1\n", 14); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == -1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); +// TODO : check "GET" + // other fields should be in unset state still... + CPPUNIT_ASSERT_EQUAL(output.v_maj, 0); + CPPUNIT_ASSERT_EQUAL(output.v_min, 0); + input.reset(); + output.clear(); + + // no version + input.append("GET / HTTP/\n", 12); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == -1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); +// TODO : check "GET" +// TODO : check "/" +// TODO : check "HTTP" + // other fields should be in unset state still... + CPPUNIT_ASSERT_EQUAL(output.v_maj, 0); + CPPUNIT_ASSERT_EQUAL(output.v_min, 0); + input.reset(); + output.clear(); + + // no major version + input.append("GET / HTTP/.1\n", 14); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == -1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); +// TODO : check "GET" +// TODO : check "/" +// TODO : check "HTTP" + // other fields should be in unset state still... + CPPUNIT_ASSERT_EQUAL(output.v_maj, 0); + CPPUNIT_ASSERT_EQUAL(output.v_min, 0); + input.reset(); + output.clear(); + + // no minor version + input.append("GET / HTTP/1.\n", 14); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == -1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); +// TODO : check "GET" +// TODO : check "/" +// TODO : check "HTTP" + CPPUNIT_ASSERT_EQUAL(output.v_maj, 1); + CPPUNIT_ASSERT_EQUAL(output.v_min, 0); + input.reset(); + output.clear(); + + // binary line + input.append("\0xA\0xB\0xC\0xD\0xE\0xF\n", 7); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == -1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); +// TODO : check "\0xA\0xB\0xC\0xD\0xE\0xF" + // other fields should be in unset state still... + CPPUNIT_ASSERT_EQUAL(output.v_maj, 0); + CPPUNIT_ASSERT_EQUAL(output.v_min, 0); + input.reset(); + output.clear(); + + // mixed whitespace line + input.append(" \t \r \n", 9); + HttpParserInit(&output, input.content(), input.contentSize()); + CPPUNIT_ASSERT(HttpParserParseReqLine(&input, &output) == -1); + CPPUNIT_ASSERT_EQUAL(output.req_start, input.content()); + CPPUNIT_ASSERT_EQUAL(output.req_end, input.content() + input.contentSize()); + // other fields should be in unset state still... + CPPUNIT_ASSERT_EQUAL(output.v_maj, 0); + CPPUNIT_ASSERT_EQUAL(output.v_min, 0); + CPPUNIT_ASSERT_EQUAL(output.v_start, -1); + CPPUNIT_ASSERT_EQUAL(output.v_end, -1); + input.reset(); + output.clear(); +} === modified file 'src/tests/testHttpRequest.h' --- src/tests/testHttpRequest.h 2009-07-26 09:24:07 +0000 +++ src/tests/testHttpRequest.h 2010-08-29 02:50:26 +0000 @@ -15,6 +15,7 @@ CPPUNIT_TEST( testCreateFromUrl ); CPPUNIT_TEST( testIPv6HostColonBug ); CPPUNIT_TEST( testSanityCheckStartLine ); + CPPUNIT_TEST( testParseRequestLine ); CPPUNIT_TEST_SUITE_END(); public: @@ -25,6 +26,7 @@ void testCreateFromUrl(); void testIPv6HostColonBug(); void testSanityCheckStartLine(); + void testParseRequestLine(); }; #endif