From: Amos Jeffries Date: Mon, 17 Feb 2014 23:58:17 +0000 (-0800) Subject: Polish from branch audit X-Git-Tag: merge-candidate-3-v1~506^2~51 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=678451c036461d8c065b9b5649773392f8940df9;p=thirdparty%2Fsquid.git Polish from branch audit --- diff --git a/src/client_side.cc b/src/client_side.cc index 179e75e819..acbd3e78dc 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -2868,7 +2868,7 @@ ConnStateData::clientParseRequests() /* Process request */ ClientSocketContext *context = parseHttpRequest(this, *parser_); if (parser_->messageOffset()) { - // nothing but prefix garbage in the buffer. consume it. + // we are done with some of the buffer. consume it. connNoteUseOfBuffer(this, parser_->messageOffset()); parser_->noteBufferShift(parser_->messageOffset()); } diff --git a/src/http/Http1Parser.cc b/src/http/Http1Parser.cc index e3dfc2b123..98c91787b8 100644 --- a/src/http/Http1Parser.cc +++ b/src/http/Http1Parser.cc @@ -7,7 +7,7 @@ #include "SquidConfig.h" void -Http1::Parser::clear() +Http::One::Parser::clear() { completedState_ = HTTP_PARSE_NONE; buf = NULL; @@ -18,7 +18,7 @@ Http1::Parser::clear() } void -Http1::RequestParser::clear() +Http::One::RequestParser::clear() { Http1::Parser::clear(); @@ -31,7 +31,7 @@ Http1::RequestParser::clear() } void -Http1::Parser::reset(const char *aBuf, int len) +Http::One::Parser::reset(const char *aBuf, int len) { clear(); // empty the state. completedState_ = HTTP_PARSE_NEW; @@ -41,41 +41,17 @@ Http1::Parser::reset(const char *aBuf, int len) } void -Http1::RequestParser::noteBufferShift(int64_t n) +Http::One::RequestParser::noteBufferShift(int64_t n) { - bufsiz -= n; - // if parsing done, ignore buffer changes. if (completedState_ == HTTP_PARSE_DONE) return; - // shift the parser resume point to match buffer content + // shift the parser resume point to match buffer content change parseOffset_ -= n; -#if WHEN_INCREMENTAL_PARSING - - // if have not yet finished request-line - if (completedState_ == HTTP_PARSE_NEW) { - // check for and adjust the known request-line offsets. - - /* TODO: when the first-line is parsed incrementally we - * will need to recalculate the offsets for req.* - * For now, they are all re-calculated based on parserOffset_ - * with each parse attempt. - */ - } - - // if finished request-line but not mime header - // adjust the mime header states - if (completedState_ == HTTP_PARSE_FIRST) { - /* TODO: when the mime-header is parsed incrementally we - * will need to store the initial offset of mime-header block - * instead of locatign it from req.end or parseOffset_. - * Since req.end may no longer be valid, and parseOffset_ may - * have moved into the mime-block interior. - */ - } -#endif + // and remember where to stop before performing buffered data overreads + bufsiz -= n; } /** @@ -97,7 +73,7 @@ Http1::RequestParser::noteBufferShift(int64_t n) * \return true if garbage whitespace was found */ bool -Http1::RequestParser::skipGarbageLines() +Http::One::RequestParser::skipGarbageLines() { req.start = parseOffset_; // avoid re-parsing any portion we managed to complete @@ -150,7 +126,7 @@ Http1::RequestParser::skipGarbageLines() * \retval 0 more data is needed to complete the parse */ int -Http1::RequestParser::parseRequestFirstLine() +Http::One::RequestParser::parseRequestFirstLine() { int second_word = -1; // track the suspected URI start int first_whitespace = -1, last_whitespace = -1; // track the first and last SP byte @@ -383,18 +359,17 @@ Http1::RequestParser::parseRequestFirstLine() } bool -Http1::RequestParser::parse() +Http::One::RequestParser::parse() { - // stage 1: locate the request-line if (completedState_ == HTTP_PARSE_NEW) { + + // stage 1: locate the request-line if (skipGarbageLines() && (size_t)bufsiz < parseOffset_) return false; - } - // stage 2: parse the request-line - if (completedState_ == HTTP_PARSE_NEW) { + // stage 2: parse the request-line PROF_start(HttpParserParseReqLine); - int retcode = parseRequestFirstLine(); + 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)); debugs(74, 5, "request-line: method " << req.m_start << "->" << req.m_end << " (" << method_ << ")"); debugs(74, 5, "request-line: url " << req.u_start << "->" << req.u_end << " (" << uri_ << ")"); @@ -449,7 +424,7 @@ Http1::RequestParser::parse() #define GET_HDR_SZ 1024 char * -Http1::Parser::getHeaderField(const char *name) +Http::One::Parser::getHeaderField(const char *name) { LOCAL_ARRAY(char, header, GET_HDR_SZ); const char *p = NULL; diff --git a/src/http/Http1Parser.h b/src/http/Http1Parser.h index 86439e440a..1169b2a60e 100644 --- a/src/http/Http1Parser.h +++ b/src/http/Http1Parser.h @@ -12,11 +12,13 @@ namespace Http { namespace One { // Parser states -#define HTTP_PARSE_NONE 0 // nothing. completely unset state. -#define HTTP_PARSE_NEW 1 // initialized, but nothing usefully parsed yet. -#define HTTP_PARSE_FIRST 2 // have parsed request first line -#define HTTP_PARSE_MIME 3 // have located end of mime header block -#define HTTP_PARSE_DONE 99 // have done with parsing so far +enum ParseState { + HTTP_PARSE_NONE =0, ///< nothing. completely unset state. + HTTP_PARSE_NEW =1, ///< initialized, but nothing usefully parsed yet + HTTP_PARSE_FIRST, ///< HTTP/1 message first line + HTTP_PARSE_MIME, ///< mime header block + HTTP_PARSE_DONE ///< completed with parsing a full request header +}; /** HTTP protocol parser. * @@ -50,7 +52,7 @@ public: * The leftmost n bytes bytes have been dropped and all other * bytes shifted left n positions. */ - virtual void noteBufferShift(int64_t n) = 0; + virtual void noteBufferShift(const int64_t n) = 0; /** Whether the parser is already done processing the buffer. * Use to determine between incomplete data and errors results @@ -77,6 +79,8 @@ public: const char *rawHeaderBuf() {return mimeHeaderBlock_.c_str();} /// attempt to parse a message from the buffer + /// \retval true if a full message was found and parsed + /// \retval false if incomplete, invalid or no message was found virtual bool parse() = 0; /// the protocol label for this message @@ -93,7 +97,7 @@ public: protected: /// what stage the parser is currently up to - uint8_t completedState_; + ParseState completedState_; /// what protocol label has been found in the first line AnyP::ProtocolVersion msgProtocol_; @@ -120,7 +124,7 @@ public: RequestParser() : Parser() {} RequestParser(const char *aBuf, int len) : Parser(aBuf, len) {} virtual void clear(); - virtual void noteBufferShift(int64_t n); + 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(); diff --git a/src/tests/testHttp1Parser.cc b/src/tests/testHttp1Parser.cc index cefd3ac992..20ca78987a 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((uint8_t)HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); 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((uint8_t)HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); 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((uint8_t)HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); 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((uint8_t)HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); 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((uint8_t)HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); 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((uint8_t)HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); 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((uint8_t)HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); 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((uint8_t)HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); 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); @@ -702,7 +702,7 @@ testHttp1Parser::testParseRequestLineMethods() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL((uint8_t)HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); 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 +728,7 @@ testHttp1Parser::testParseRequestLineMethods() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL((uint8_t)HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); 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 +754,7 @@ testHttp1Parser::testParseRequestLineMethods() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL((uint8_t)HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); 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 +824,7 @@ testHttp1Parser::testParseRequestLineMethods() Config.onoff.relaxed_header_parser = 1; CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL((uint8_t)HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); 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); @@ -874,7 +874,7 @@ testHttp1Parser::testParseRequestLineMethods() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL((uint8_t)HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); 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 +938,7 @@ testHttp1Parser::testParseRequestLineInvalid() Config.onoff.relaxed_header_parser = 1; CPPUNIT_ASSERT_EQUAL(true, output.parse()); CPPUNIT_ASSERT_EQUAL(true, output.isDone()); -// CPPUNIT_ASSERT_EQUAL((uint8_t)HTTP_PARSE_NEW, output.completedState_); +// CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_NEW, output.completedState_); 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 +988,7 @@ testHttp1Parser::testParseRequestLineInvalid() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL((uint8_t)HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); 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 +1036,7 @@ testHttp1Parser::testParseRequestLineInvalid() output.reset(input.content(), input.contentSize()); CPPUNIT_ASSERT_EQUAL(false, output.parse()); CPPUNIT_ASSERT_EQUAL(false, output.isDone()); - CPPUNIT_ASSERT_EQUAL((uint8_t)HTTP_PARSE_FIRST, output.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, output.completedState_); 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); @@ -1215,7 +1215,7 @@ testHttp1Parser::testDripFeed() CPPUNIT_ASSERT_EQUAL(garbageEnd, (int)hp.parseOffset_); CPPUNIT_ASSERT_EQUAL(false, parseResult); CPPUNIT_ASSERT_EQUAL(false, hp.isDone()); - CPPUNIT_ASSERT_EQUAL((uint8_t)HTTP_PARSE_NEW, hp.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_NEW, hp.completedState_); continue; } @@ -1225,7 +1225,7 @@ testHttp1Parser::testDripFeed() 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((uint8_t)HTTP_PARSE_FIRST, hp.completedState_); + CPPUNIT_ASSERT_EQUAL(Http1::HTTP_PARSE_FIRST, hp.completedState_); continue; }