From: Amos Jeffries Date: Thu, 3 Apr 2014 09:11:45 +0000 (-0700) Subject: Restructure parser state management X-Git-Tag: merge-candidate-3-v1~506^2~50 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=cbcd99df7bc872c89683411c56ee8a32249ff72c;p=thirdparty%2Fsquid.git Restructure parser state management Old parser tracked completed states. Which led to several bugs in resuming parse of a 'block' of data, end of block detection. Instead track the state/block currently being processed. Thanks to Alex Rousskov for identifying this as the source of several bugs. --- diff --git a/src/client_side.cc b/src/client_side.cc index acbd3e78dc..67eab966a5 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -2792,15 +2792,6 @@ finish: } } -static void -connStripBufferWhitespace (ConnStateData * conn) -{ - while (conn->in.notYetUsed > 0 && xisspace(conn->in.buf[0])) { - memmove(conn->in.buf, conn->in.buf + 1, conn->in.notYetUsed - 1); - -- conn->in.notYetUsed; - } -} - /** * Limit the number of concurrent requests. * \return true when there are available position(s) in the pipeline queue for another request. @@ -2840,7 +2831,6 @@ ConnStateData::clientParseRequests() // Loop while we have read bytes that are not needed for producing the body // On errors, bodyPipe may become nil, but readMore will be cleared while (in.notYetUsed > 0 && !bodyPipe && flags.readMore) { - connStripBufferWhitespace(this); // XXX: should not be needed anymore. /* Don't try to parse if the buffer is empty */ if (in.notYetUsed == 0) @@ -2850,10 +2840,6 @@ ConnStateData::clientParseRequests() if (concurrentRequestQueueFilled()) break; - /* Should not be needed anymore */ - /* Terminate the string */ - in.buf[in.notYetUsed] = '\0'; - /* Begin the parsing */ PROF_start(parseHttpRequest); @@ -2867,10 +2853,10 @@ ConnStateData::clientParseRequests() /* Process request */ ClientSocketContext *context = parseHttpRequest(this, *parser_); - if (parser_->messageOffset()) { - // we are done with some of the buffer. consume it. - connNoteUseOfBuffer(this, parser_->messageOffset()); - parser_->noteBufferShift(parser_->messageOffset()); + if (parser_->doneBytes()) { + // we are done with some of the buffer. consume it now. + connNoteUseOfBuffer(this, parser_->doneBytes()); + parser_->noteBufferShift(parser_->doneBytes()); } PROF_stop(parseHttpRequest); @@ -2881,9 +2867,6 @@ ConnStateData::clientParseRequests() CommTimeoutCbPtrFun(clientLifetimeTimeout, context->http)); commSetConnTimeout(clientConnection, Config.Timeout.lifetime, timeoutCall); - // Request has now been shifted out of the buffer. - // Consume header early so that next action starts with just the next bytes. - connNoteUseOfBuffer(this, parser_->messageHeaderSize() + parser_->messageOffset()); clientProcessRequest(this, *parser_, context); parsed_req = true; // XXX: do we really need to parse everything right NOW ? diff --git a/src/http/Http1Parser.cc b/src/http/Http1Parser.cc index 98c91787b8..d89423775c 100644 --- a/src/http/Http1Parser.cc +++ b/src/http/Http1Parser.cc @@ -9,7 +9,7 @@ void Http::One::Parser::clear() { - completedState_ = HTTP_PARSE_NONE; + parsingStage_ = HTTP_PARSE_NONE; buf = NULL; bufsiz = 0; parseOffset_ = 0; @@ -34,7 +34,8 @@ void Http::One::Parser::reset(const char *aBuf, int len) { clear(); // empty the state. - completedState_ = HTTP_PARSE_NEW; + parsingStage_ = HTTP_PARSE_NEW; + parseOffset_ = 0; buf = aBuf; bufsiz = len; debugs(74, DBG_DATA, "Parse " << Raw("buf", buf, bufsiz)); @@ -44,7 +45,7 @@ void Http::One::RequestParser::noteBufferShift(int64_t n) { // if parsing done, ignore buffer changes. - if (completedState_ == HTTP_PARSE_DONE) + if (parsingStage_ == HTTP_PARSE_DONE) return; // shift the parser resume point to match buffer content change @@ -70,22 +71,20 @@ Http::One::RequestParser::noteBufferShift(int64_t n) * " * * Parsing state is stored between calls to avoid repeating buffer scans. - * \return true if garbage whitespace was found + * If garbage is found the parsing offset is incremented. */ -bool +void Http::One::RequestParser::skipGarbageLines() { - req.start = parseOffset_; // avoid re-parsing any portion we managed to complete - #if WHEN_RFC_COMPLIANT // CRLF or bare-LF is what RFC 2616 tolerant parsers do ... if (Config.onoff.relaxed_header_parser) { - if (Config.onoff.relaxed_header_parser < 0 && (buf[req.start] == '\r' || buf[req.start] == '\n')) + if (Config.onoff.relaxed_header_parser < 0 && (buf[parseOffset_] == '\r' || buf[parseOffset_] == '\n')) debugs(74, DBG_IMPORTANT, "WARNING: Invalid HTTP Request: " << "CRLF bytes received ahead of request-line. " << "Ignored due to relaxed_header_parser."); // Be tolerant of prefix empty lines - for (; req.start < bufsiz && (buf[req.start] == '\n' || ((buf[req.start] == '\r' && (buf[req.start+1] == '\n')); ++req.start); - parseOffset_ = req.start; + // ie any series of either \n or \r\n with no other characters and no repeated \r + for (; parseOffset_ < (size_t)bufsiz && (buf[parseOffset_] == '\n' || ((buf[parseOffset_] == '\r' && (buf[parseOffset_+1] == '\n')); ++parseOffset_); } #endif @@ -97,17 +96,14 @@ Http::One::RequestParser::skipGarbageLines() */ #if USE_HTTP_VIOLATIONS if (Config.onoff.relaxed_header_parser) { - if (Config.onoff.relaxed_header_parser < 0 && buf[req.start] == ' ') + if (Config.onoff.relaxed_header_parser < 0 && buf[parseOffset_] == ' ') debugs(74, DBG_IMPORTANT, "WARNING: Invalid HTTP Request: " << "Whitespace bytes received ahead of method. " << "Ignored due to relaxed_header_parser."); // Be tolerant of prefix spaces (other bytes are valid method values) - for (; req.start < bufsiz && buf[req.start] == ' '; ++req.start); - parseOffset_ = req.start; + for (; parseOffset_ < (size_t)bufsiz && buf[parseOffset_] == ' '; ++parseOffset_); } #endif - - return (parseOffset_ > 0); } /** @@ -253,8 +249,6 @@ Http::One::RequestParser::parseRequestFirstLine() msgProtocol_ = Http::ProtocolVersion(0,9); req.u_end = line_end; request_parse_status = Http::scOkay; // HTTP/0.9 - completedState_ = HTTP_PARSE_FIRST; - parseOffset_ = line_end; return 1; } else { // otherwise last whitespace is somewhere after end of URI. @@ -285,8 +279,6 @@ Http::One::RequestParser::parseRequestFirstLine() msgProtocol_ = Http::ProtocolVersion(0,9); req.u_end = line_end; request_parse_status = Http::scOkay; // treat as HTTP/0.9 - completedState_ = HTTP_PARSE_FIRST; - parseOffset_ = req.end; return 1; #else // protocol not supported / implemented. @@ -353,21 +345,28 @@ Http::One::RequestParser::parseRequestFirstLine() * Rightio - we have all the schtuff. Return true; we've got enough. */ request_parse_status = Http::scOkay; - parseOffset_ = req.end+1; // req.end is the \n byte. Next parse step needs to start *after* that byte. - completedState_ = HTTP_PARSE_FIRST; return 1; } - +#include bool Http::One::RequestParser::parse() { - if (completedState_ == HTTP_PARSE_NEW) { - - // stage 1: locate the request-line - if (skipGarbageLines() && (size_t)bufsiz < parseOffset_) + // stage 1: locate the request-line + if (parsingStage_ == HTTP_PARSE_NEW) { +fprintf(stderr, "parse GARBAGE: '%s'\n", buf); + skipGarbageLines(); +fprintf(stderr, "parse GBG A(%d) < B(%u)\n", bufsiz, parseOffset_); + + // if we hit something before EOS treat it as a message + if ((size_t)bufsiz > parseOffset_) + parsingStage_ = HTTP_PARSE_FIRST; + else return false; + } - // stage 2: parse the request-line + // stage 2: parse the request-line + if (parsingStage_ == HTTP_PARSE_FIRST) { +fprintf(stderr, "parse FIRST: '%s'\n", buf); PROF_start(HttpParserParseReqLine); const int retcode = parseRequestFirstLine(); debugs(74, 5, "request-line: retval " << retcode << ": from " << req.start << "->" << req.end << " " << Raw("line", &buf[req.start], req.end-req.start)); @@ -376,14 +375,26 @@ Http::One::RequestParser::parse() debugs(74, 5, "request-line: proto " << req.v_start << "->" << req.v_end << " (" << msgProtocol_ << ")"); debugs(74, 5, "Parser: parse-offset=" << parseOffset_); PROF_stop(HttpParserParseReqLine); + + // syntax errors already if (retcode < 0) { - completedState_ = HTTP_PARSE_DONE; + parsingStage_ = HTTP_PARSE_DONE; +fprintf(stderr, "parse FIRST DONE (error)\n"); return false; } + + // first-line (or a look-alike) found successfully. + if (retcode > 0) { + parseOffset_ += firstLineSize(); // first line bytes including CRLF terminator are now done. + parsingStage_ = HTTP_PARSE_MIME; +fprintf(stderr, "parse FIRST (next: MIME)\n"); + } +else fprintf(stderr, "parse FIRST: ret=%d\n",retcode); } // stage 3: locate the mime header block - if (completedState_ == HTTP_PARSE_FIRST) { + if (parsingStage_ == HTTP_PARSE_MIME) { +fprintf(stderr, "parse MIME: '%s'\n", buf); // HTTP/1.x request-line is valid and parsing completed. if (msgProtocol_.major == 1) { /* NOTE: HTTP/0.9 requests do not have a mime header block. @@ -395,19 +406,22 @@ Http::One::RequestParser::parse() if (bufsiz-parseOffset_ >= Config.maxRequestHeaderSize) { debugs(33, 5, "Too large request"); request_parse_status = Http::scHeaderTooLarge; - completedState_ = HTTP_PARSE_DONE; - } else + parsingStage_ = HTTP_PARSE_DONE; +fprintf(stderr, "parse DONE: HTTP/1.x\n"); + } else { debugs(33, 5, "Incomplete request, waiting for end of headers"); - return false; +fprintf(stderr, "parse MIME incomplete\n"); +} return false; } mimeHeaderBlock_.assign(&buf[req.end+1], mimeHeaderBytes); + parseOffset_ += mimeHeaderBytes; // done with these bytes now. - } else + } else { debugs(33, 3, "Missing HTTP/1.x identifier"); - - // NP: planned name for this stage is HTTP_PARSE_MIME - // but we do not do any further stages here yet so go straight to DONE - completedState_ = HTTP_PARSE_DONE; +fprintf(stderr, "parse MIME: HTTP/0.9\n"); +} + // NP: we do not do any further stages here yet so go straight to DONE + parsingStage_ = HTTP_PARSE_DONE; // Squid could handle these headers, but admin does not want to if (messageHeaderSize() >= Config.maxRequestHeaderSize) { diff --git a/src/http/Http1Parser.h b/src/http/Http1Parser.h index 1169b2a60e..22140df02e 100644 --- a/src/http/Http1Parser.h +++ b/src/http/Http1Parser.h @@ -58,10 +58,10 @@ public: * Use to determine between incomplete data and errors results * from the parse. */ - bool isDone() const {return completedState_==HTTP_PARSE_DONE;} + bool isDone() const {return parsingStage_==HTTP_PARSE_DONE;} - /// number of bytes in buffer before the message - virtual int64_t messageOffset() const = 0; + /// number of bytes at the start of the buffer which are no longer needed + int64_t doneBytes() const {return (int64_t)parseOffset_;} /// size in bytes of the first line including CRLF terminator virtual int64_t firstLineSize() const = 0; @@ -97,7 +97,7 @@ public: protected: /// what stage the parser is currently up to - ParseState completedState_; + ParseState parsingStage_; /// what protocol label has been found in the first line AnyP::ProtocolVersion msgProtocol_; @@ -125,7 +125,6 @@ public: RequestParser(const char *aBuf, int len) : Parser(aBuf, len) {} virtual void clear(); virtual void noteBufferShift(const int64_t n); - virtual int64_t messageOffset() const {return req.start;} virtual int64_t firstLineSize() const {return req.end - req.start + 1;} virtual bool parse(); @@ -141,10 +140,11 @@ public: Http::StatusCode request_parse_status; private: - bool skipGarbageLines(); + void skipGarbageLines(); int parseRequestFirstLine(); /// Offsets for pieces of the (HTTP request) Request-Line as per RFC 2616 + /// only valid before and during parse stage HTTP_PARSE_FIRST struct request_offsets { int start, end; int m_start, m_end; // method diff --git a/src/tests/testHttp1Parser.cc b/src/tests/testHttp1Parser.cc index 20ca78987a..fee350b5ad 100644 --- a/src/tests/testHttp1Parser.cc +++ b/src/tests/testHttp1Parser.cc @@ -99,7 +99,7 @@ testHttp1Parser::testParseRequestLineProtocols() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_MIME, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scOkay, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL((int)input.contentSize()-1, output.req.end); @@ -124,7 +124,7 @@ testHttp1Parser::testParseRequestLineProtocols() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_MIME, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scOkay, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL((int)input.contentSize()-1, output.req.end); @@ -150,7 +150,7 @@ testHttp1Parser::testParseRequestLineProtocols() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_MIME, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scOkay, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL((int)input.contentSize()-1, output.req.end); @@ -399,7 +399,7 @@ testHttp1Parser::testParseRequestLineStrange() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_MIME, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scOkay, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL((int)input.contentSize()-1, output.req.end); @@ -425,7 +425,7 @@ testHttp1Parser::testParseRequestLineStrange() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_MIME, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scOkay, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL((int)input.contentSize()-1, output.req.end); @@ -451,7 +451,7 @@ testHttp1Parser::testParseRequestLineStrange() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_MIME, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scOkay, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL((int)input.contentSize()-5, output.req.end); @@ -488,7 +488,7 @@ testHttp1Parser::testParseRequestLineTerminators() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_MIME, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scOkay, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL((int)input.contentSize()-1, output.req.end); @@ -541,7 +541,7 @@ testHttp1Parser::testParseRequestLineTerminators() // Being tolerant we can ignore and elide these apparently benign CR CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_MIME, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scOkay, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL((int)input.contentSize()-1, output.req.end); @@ -616,6 +616,7 @@ testHttp1Parser::testParseRequestLineTerminators() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scNone, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL(-1, output.req.end); @@ -634,6 +635,7 @@ testHttp1Parser::testParseRequestLineTerminators() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scNone, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL(-1, output.req.end); @@ -652,6 +654,7 @@ testHttp1Parser::testParseRequestLineTerminators() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scNone, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL(-1, output.req.end); @@ -670,6 +673,7 @@ testHttp1Parser::testParseRequestLineTerminators() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scNone, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL(-1, output.req.end); @@ -702,7 +706,7 @@ testHttp1Parser::testParseRequestLineMethods() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_MIME, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scOkay, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL((int)input.contentSize()-1, output.req.end); @@ -728,7 +732,7 @@ testHttp1Parser::testParseRequestLineMethods() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_MIME, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scOkay, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL((int)input.contentSize()-1, output.req.end); @@ -754,7 +758,7 @@ testHttp1Parser::testParseRequestLineMethods() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_MIME, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scOkay, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL((int)input.contentSize()-1, output.req.end); @@ -824,7 +828,7 @@ testHttp1Parser::testParseRequestLineMethods() Config.onoff.relaxed_header_parser = 1; CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_MIME, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scOkay, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(1, output.req.start); CPPUNIT_ASSERT_EQUAL((int)input.contentSize()-1, output.req.end); @@ -868,13 +872,14 @@ testHttp1Parser::testParseRequestLineMethods() } // tab padded method (NP: tab is not SP so treated as any other binary) + // XXX: binary codes are non-compliant { input.append("\tGET / HTTP/1.1\n", 16); //printf("TEST: '%s'\n",input.content()); output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_MIME, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scOkay, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL((int)input.contentSize()-1, output.req.end); @@ -938,7 +943,7 @@ testHttp1Parser::testParseRequestLineInvalid() Config.onoff.relaxed_header_parser = 1; CPPUNIT_ASSERT_EQUAL(true, output.parse()); CPPUNIT_ASSERT_EQUAL(true, output.isDone()); -// CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_NEW, output.completedState_); +// CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_DONE, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scOkay, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(1, output.req.start); CPPUNIT_ASSERT_EQUAL((int)input.contentSize()-1, output.req.end); @@ -988,7 +993,7 @@ testHttp1Parser::testParseRequestLineInvalid() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_MIME, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scOkay, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL((int)input.contentSize()-1, output.req.end); @@ -1036,7 +1041,7 @@ testHttp1Parser::testParseRequestLineInvalid() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_MIME, output.parsingStage_); CPPUNIT_ASSERT_EQUAL(Http::scOkay, output.request_parse_status); CPPUNIT_ASSERT_EQUAL(0, output.req.start); CPPUNIT_ASSERT_EQUAL((int)input.contentSize()-1, output.req.end); @@ -1187,7 +1192,9 @@ testHttp1Parser::testDripFeed() int garbageEnd = mb.contentSize(); mb.append("GET http://example.com/ HTTP/1.1\r\n", 34); int reqLineEnd = mb.contentSize(); - mb.append("Host: example.com\r\n\r\n.", 22); + mb.append("Host: example.com\r\n\r\n", 21); + int mimeEnd = mb.contentSize(); + mb.append("...", 3); // trailer to catch mime EOS errors. Http1::RequestParser hp(mb.content(), 0); @@ -1204,9 +1211,10 @@ testHttp1Parser::testDripFeed() #endif // before end of garbage found its a moving offset. - if (hp.bufsiz < garbageEnd) { + if (hp.bufsiz <= garbageEnd) { CPPUNIT_ASSERT_EQUAL(hp.bufsiz, (int)hp.parseOffset_); CPPUNIT_ASSERT_EQUAL(false, hp.isDone()); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_NEW, hp.parsingStage_); continue; } @@ -1215,22 +1223,22 @@ testHttp1Parser::testDripFeed() CPPUNIT_ASSERT_EQUAL(garbageEnd, (int)hp.parseOffset_); CPPUNIT_ASSERT_EQUAL(false, parseResult); CPPUNIT_ASSERT_EQUAL(false, hp.isDone()); - CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_NEW, hp.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, hp.parsingStage_); continue; } // before request headers entirely found, parse announces incomplete - if (hp.bufsiz < mb.contentSize()-1) { + if (hp.bufsiz < mimeEnd) { CPPUNIT_ASSERT_EQUAL(reqLineEnd, (int)hp.parseOffset_); CPPUNIT_ASSERT_EQUAL(false, parseResult); CPPUNIT_ASSERT_EQUAL(false, hp.isDone()); // TODO: add all the other usual tests for request-line details - CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, hp.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_MIME, hp.parsingStage_); continue; } // once request line is found (AND the following \n) current parser announces success - CPPUNIT_ASSERT_EQUAL(reqLineEnd, (int)hp.parseOffset_); + CPPUNIT_ASSERT_EQUAL(mimeEnd, (int)hp.parseOffset_); CPPUNIT_ASSERT_EQUAL(true, parseResult); CPPUNIT_ASSERT_EQUAL(true, hp.isDone()); }