From: Eduard Bagdasaryan Date: Tue, 15 Nov 2016 05:08:59 +0000 (+1300) Subject: ICAP: Initial support for trailers X-Git-Tag: M-staged-PR71~375 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=69c698a38b5b015c6f98710a44a6c7fb95a86040;p=thirdparty%2Fsquid.git ICAP: Initial support for trailers ICAP trailers are currently specified by https://datatracker.ietf.org/doc/draft-rousskov-icap-trailers/ This implementation complies with version -01 of that draft: - Squid unconditionally announces ICAP Trailer support via the ICAP Allow request header field. - Squid parses the ICAP response trailer if and only if the ICAP server signals its presence by sending both Trailer header and Allow/trailers in the ICAP response. Squid logs and ignores all parsed ICAP header fields (for now). Also refactored HttpHeader parsing methods in order to reuse them for ICAP trailer parsing. Also simplified and fixed headers isolating code while dealing with empty (i.e. zero header fields) headers. Old httpMsgIsolateHeaders() tried to re-implement header end detection/processing logic that is actually covered by headersEnd(). Old httpMsgIsolateHeaders() could return success for some garbage input (e.g., a buffer of several CRs) even if no end of headers was found. --- diff --git a/src/HttpHeader.cc b/src/HttpHeader.cc index 2191f91474..de2e1631ca 100644 --- a/src/HttpHeader.cc +++ b/src/HttpHeader.cc @@ -22,6 +22,7 @@ #include "HttpHeaderTools.h" #include "MemBuf.h" #include "mgr/Registration.h" +#include "mime_header.h" #include "profiler/Profiler.h" #include "rfc1123.h" #include "SquidConfig.h" @@ -316,6 +317,51 @@ HttpHeader::update(HttpHeader const *fresh) return true; } +bool +HttpHeader::Isolate(const char **parse_start, size_t l, const char **blk_start, const char **blk_end) +{ + /* + * parse_start points to the first line of HTTP message *headers*, + * not including the request or status lines + */ + const size_t end = headersEnd(*parse_start, l); + + if (end) { + *blk_start = *parse_start; + *blk_end = *parse_start + end - 1; + assert(**blk_end == '\n'); + // Point blk_end to the first character after the last header field. + // In other words, blk_end should point to the CR?LF header terminator. + if (end > 1 && *(*blk_end - 1) == '\r') + --(*blk_end); + *parse_start += end; + } + return end; +} + +int +HttpHeader::parse(const char *buf, size_t buf_len, bool atEnd, size_t &hdr_sz) +{ + const char *parse_start = buf; + const char *blk_start, *blk_end; + hdr_sz = 0; + + if (!Isolate(&parse_start, buf_len, &blk_start, &blk_end)) { + // XXX: do not parse non-isolated headers even if the connection is closed. + // Treat unterminated headers as "partial headers" framing errors. + if (!atEnd) + return 0; + blk_start = parse_start; + blk_end = blk_start + strlen(blk_start); + } + + if (parse(blk_start, blk_end - blk_start)) { + hdr_sz = parse_start - buf; + return 1; + } + return -1; +} + int HttpHeader::parse(const char *header_start, size_t hdrLen) { diff --git a/src/HttpHeader.h b/src/HttpHeader.h index 4ecb9b3748..f28c32c1f3 100644 --- a/src/HttpHeader.h +++ b/src/HttpHeader.h @@ -84,6 +84,11 @@ public: bool update(HttpHeader const *fresh); void compact(); int parse(const char *header_start, size_t len); + /// Parses headers stored in a buffer. + /// \returns 1 and sets hdr_sz on success + /// \returns 0 when needs more data + /// \returns -1 on error + int parse(const char *buf, size_t buf_len, bool atEnd, size_t &hdr_sz); void packInto(Packable * p, bool mask_sensitive_info=false) const; HttpHeaderEntry *getEntry(HttpHeaderPos * pos) const; HttpHeaderEntry *findEntry(Http::HdrType id) const; @@ -145,6 +150,13 @@ public: protected: /** \deprecated Public access replaced by removeHopByHopEntries() */ void removeConnectionHeaderEntries(); + /// either finds the end of headers or returns false + /// If the end was found: + /// *parse_start points to the first character after the header delimiter + /// *blk_start points to the first header character (i.e. old parse_start value) + /// *blk_end points to the first header delimiter character (CR or LF in CR?LF). + /// If block starts where it ends, then there are no fields in the header. + static bool Isolate(const char **parse_start, size_t l, const char **blk_start, const char **blk_end); bool needUpdate(const HttpHeader *fresh) const; bool skipUpdateHeader(const Http::HdrType id) const; void updateWarnings(); diff --git a/src/HttpMsg.cc b/src/HttpMsg.cc index 7f57717a66..3bdf1b32d8 100644 --- a/src/HttpMsg.cc +++ b/src/HttpMsg.cc @@ -60,65 +60,6 @@ HttpMsgParseState &operator++ (HttpMsgParseState &aState) return aState; } -/* find end of headers */ -static int -httpMsgIsolateHeaders(const char **parse_start, int l, const char **blk_start, const char **blk_end) -{ - /* - * parse_start points to the first line of HTTP message *headers*, - * not including the request or status lines - */ - size_t end = headersEnd(*parse_start, l); - int nnl; - - if (end) { - *blk_start = *parse_start; - *blk_end = *parse_start + end - 1; - /* - * leave blk_end pointing to the first character after the - * first newline which terminates the headers - */ - assert(**blk_end == '\n'); - - while (*(*blk_end - 1) == '\r') - --(*blk_end); - - assert(*(*blk_end - 1) == '\n'); - - *parse_start += end; - - return 1; - } - - /* - * If we didn't find the end of headers, and parse_start does - * NOT point to a CR or NL character, then return failure - */ - if (**parse_start != '\r' && **parse_start != '\n') - return 0; /* failure */ - - /* - * If we didn't find the end of headers, and parse_start does point - * to an empty line, then we have empty headers. Skip all CR and - * NL characters up to the first NL. Leave parse_start pointing at - * the first character after the first NL. - */ - *blk_start = *parse_start; - - *blk_end = *blk_start; - - for (nnl = 0; nnl == 0; ++(*parse_start)) { - if (**parse_start == '\r') - (void) 0; - else if (**parse_start == '\n') - ++nnl; - else - break; - } - - return 1; -} - /* find first CRLF */ static int httpMsgIsolateStart(const char **parse_start, const char **blk_start, const char **blk_end) @@ -275,27 +216,14 @@ HttpMsg::httpMsgParseStep(const char *buf, int len, int atEnd) * after headers.) Grr. */ if (pstate == psReadyToParseHeaders) { - if (!httpMsgIsolateHeaders(&parse_start, parse_len, &blk_start, &blk_end)) { - if (atEnd) { - blk_start = parse_start; - blk_end = blk_start + strlen(blk_start); - } else { - PROF_stop(HttpMsg_httpMsgParseStep); - return 0; - } - } - - if (!header.parse(blk_start, blk_end-blk_start)) { + size_t hsize = 0; + const int parsed = header.parse(parse_start, parse_len, atEnd, hsize); + if (parsed <= 0) { PROF_stop(HttpMsg_httpMsgParseStep); - return httpMsgParseError(); + return !parsed ? 0 : httpMsgParseError(); } - + hdr_sz += hsize; hdrCacheInit(); - - *parse_end_ptr = parse_start; - - hdr_sz = *parse_end_ptr - buf; - ++pstate; } diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc index b5f3fe09dc..549a78ec45 100644 --- a/src/adaptation/icap/ModXact.cc +++ b/src/adaptation/icap/ModXact.cc @@ -60,6 +60,7 @@ Adaptation::Icap::ModXact::ModXact(HttpMsg *virginHeader, replyHttpHeaderSize(-1), replyHttpBodySize(-1), adaptHistoryId(-1), + trailerParser(nullptr), alMaster(alp) { assert(virginHeader); @@ -657,6 +658,9 @@ void Adaptation::Icap::ModXact::parseMore() if (state.parsing == State::psBody) parseBody(); + + if (state.parsing == State::psIcapTrailer) + parseIcapTrailer(); } void Adaptation::Icap::ModXact::callException(const std::exception &e) @@ -698,7 +702,7 @@ void Adaptation::Icap::ModXact::bypassFailure() // end all activities associated with the ICAP server - stopParsing(); + stopParsing(false); stopWriting(true); // or should we force it? if (haveConnection()) { @@ -790,6 +794,11 @@ void Adaptation::Icap::ModXact::parseIcapHead() if (!parseHead(icapReply.getRaw())) return; + if (expectIcapTrailers()) { + Must(!trailerParser); + trailerParser = new TrailerParser; + } + if (httpHeaderHasConnDir(&icapReply->header, "close")) { debugs(93, 5, HERE << "found connection close"); reuseConnection = false; @@ -864,23 +873,24 @@ void Adaptation::Icap::ModXact::parseIcapHead() stopWriting(true); } -bool Adaptation::Icap::ModXact::validate200Ok() -{ - if (ICAP::methodRespmod == service().cfg().method) { - if (!gotEncapsulated("res-hdr")) - return false; +/// Parses ICAP trailers and stops parsing, if all trailer data +/// have been received. +void Adaptation::Icap::ModXact::parseIcapTrailer() { - return true; + if (parsePart(trailerParser, "trailer")) { + for (const auto &e: trailerParser->trailer.entries) + debugs(93, 5, "ICAP trailer: " << e->name << ": " << e->value); + stopParsing(); } +} - if (ICAP::methodReqmod == service().cfg().method) { - if (!gotEncapsulated("res-hdr") && !gotEncapsulated("req-hdr")) - return false; - - return true; - } +bool Adaptation::Icap::ModXact::validate200Ok() +{ + if (service().cfg().method == ICAP::methodRespmod) + return gotEncapsulated("res-hdr"); - return false; + return service().cfg().method == ICAP::methodReqmod && + expectHttpHeader(); } void Adaptation::Icap::ModXact::handle100Continue() @@ -1040,7 +1050,7 @@ void Adaptation::Icap::ModXact::prepPartialBodyEchoing(uint64_t pos) void Adaptation::Icap::ModXact::handleUnknownScode() { - stopParsing(); + stopParsing(false); stopBackup(); // TODO: mark connection as "bad" @@ -1050,7 +1060,7 @@ void Adaptation::Icap::ModXact::handleUnknownScode() void Adaptation::Icap::ModXact::parseHttpHead() { - if (gotEncapsulated("res-hdr") || gotEncapsulated("req-hdr")) { + if (expectHttpHeader()) { replyHttpHeaderSize = 0; maybeAllocateHttpMsg(); @@ -1077,33 +1087,59 @@ void Adaptation::Icap::ModXact::parseHttpHead() decideOnParsingBody(); } -// parses both HTTP and ICAP headers -bool Adaptation::Icap::ModXact::parseHead(HttpMsg *head) +template +bool Adaptation::Icap::ModXact::parsePart(Part *part, const char *description) { - Must(head); - debugs(93, 5, "have " << readBuf.length() << " head bytes to parse; state: " << state.parsing); - + Must(part); + debugs(93, 5, "have " << readBuf.length() << ' ' << description << " bytes to parse; state: " << state.parsing); Http::StatusCode error = Http::scNone; // XXX: performance regression. c_str() data copies // XXX: HttpMsg::parse requires a terminated string buffer const char *tmpBuf = readBuf.c_str(); - const bool parsed = head->parse(tmpBuf, readBuf.length(), commEof, &error); - Must(parsed || !error); // success or need more data + const bool parsed = part->parse(tmpBuf, readBuf.length(), commEof, &error); + debugs(93, (!parsed && error) ? 2 : 5, description << " parsing result: " << parsed << " detail: " << error); + Must(parsed || !error); + if (parsed) + readBuf.consume(part->hdr_sz); + return parsed; +} - if (!parsed) { // need more data - debugs(93, 5, HERE << "parse failed, need more data, return false"); +// parses both HTTP and ICAP headers +bool Adaptation::Icap::ModXact::parseHead(HttpMsg *head) +{ + if (!parsePart(head, "head")) { head->reset(); return false; } - - debugs(93, 5, HERE << "parse success, consume " << head->hdr_sz << " bytes, return true"); - readBuf.consume(head->hdr_sz); return true; } +bool Adaptation::Icap::ModXact::expectHttpHeader() const +{ + return gotEncapsulated("res-hdr") || gotEncapsulated("req-hdr"); +} + +bool Adaptation::Icap::ModXact::expectHttpBody() const +{ + return gotEncapsulated("res-body") || gotEncapsulated("req-body"); +} + +bool Adaptation::Icap::ModXact::expectIcapTrailers() const +{ + String trailers; + const bool promisesToSendTrailer = icapReply->header.getByIdIfPresent(Http::HdrType::TRAILER, trailers); + const bool supportsTrailers = icapReply->header.hasListMember(Http::HdrType::ALLOW, "trailers", ','); + // ICAP Trailer specs require us to reject transactions having either Trailer + // header or Allow:trailers + Must((promisesToSendTrailer == supportsTrailers) || (!promisesToSendTrailer && supportsTrailers)); + if (promisesToSendTrailer && !trailers.size()) + debugs(93, DBG_IMPORTANT, "ERROR: ICAP Trailer response header field must not be empty (salvaged)"); + return promisesToSendTrailer; +} + void Adaptation::Icap::ModXact::decideOnParsingBody() { - if (gotEncapsulated("res-body") || gotEncapsulated("req-body")) { + if (expectHttpBody()) { debugs(93, 5, HERE << "expecting a body"); state.parsing = State::psBody; replyHttpBodySize = 0; @@ -1112,7 +1148,10 @@ void Adaptation::Icap::ModXact::decideOnParsingBody() Must(state.sending == State::sendingAdapted); } else { debugs(93, 5, HERE << "not expecting a body"); - stopParsing(); + if (trailerParser) + state.parsing = State::psIcapTrailer; + else + stopParsing(); stopSending(true); } } @@ -1142,15 +1181,14 @@ void Adaptation::Icap::ModXact::parseBody() } if (parsed) { - if (state.readyForUob && bodyParser->useOriginBody >= 0) { - prepPartialBodyEchoing( - static_cast(bodyParser->useOriginBody)); + if (state.readyForUob && bodyParser->useOriginBody >= 0) + prepPartialBodyEchoing(static_cast(bodyParser->useOriginBody)); + else + stopSending(true); // the parser succeeds only if all parsed data fits + if (trailerParser) + state.parsing = State::psIcapTrailer; + else stopParsing(); - return; - } - - stopParsing(); - stopSending(true); // the parser succeeds only if all parsed data fits return; } @@ -1170,16 +1208,21 @@ void Adaptation::Icap::ModXact::parseBody() } } -void Adaptation::Icap::ModXact::stopParsing() +void Adaptation::Icap::ModXact::stopParsing(const bool checkUnparsedData) { if (state.parsing == State::psDone) return; - debugs(93, 7, HERE << "will no longer parse" << status()); + if (checkUnparsedData) + Must(readBuf.isEmpty()); + + debugs(93, 7, "will no longer parse" << status()); delete bodyParser; + bodyParser = nullptr; - bodyParser = NULL; + delete trailerParser; + trailerParser = nullptr; state.parsing = State::psDone; } @@ -1240,6 +1283,7 @@ void Adaptation::Icap::ModXact::noteBodyConsumerAborted(BodyPipe::Pointer) Adaptation::Icap::ModXact::~ModXact() { delete bodyParser; + delete trailerParser; } // internal cleanup @@ -1482,9 +1526,10 @@ void Adaptation::Icap::ModXact::makeAllowHeader(MemBuf &buf) const bool allow204out = state.allowedPostview204 = shouldAllow204(); const bool allow206in = state.allowedPreview206 = shouldAllow206in(); const bool allow206out = state.allowedPostview206 = shouldAllow206out(); + const bool allowTrailers = true; // TODO: make configurable debugs(93,9, HERE << "Allows: " << allow204in << allow204out << - allow206in << allow206out); + allow206in << allow206out << allowTrailers); const bool allow204 = allow204in || allow204out; const bool allow206 = allow206in || allow206out; @@ -1499,17 +1544,15 @@ void Adaptation::Icap::ModXact::makeAllowHeader(MemBuf &buf) // writing Allow/204 means we will honor 204 outside preview // writing Allow:206 means we will honor 206 inside preview // writing Allow:204,206 means we will honor 206 outside preview - const char *allowHeader = NULL; - if (allow204out && allow206) - allowHeader = "Allow: 204, 206\r\n"; - else if (allow204out) - allowHeader = "Allow: 204\r\n"; - else if (allow206) - allowHeader = "Allow: 206\r\n"; - - if (allowHeader) { // may be nil if only allow204in is true - buf.append(allowHeader, strlen(allowHeader)); - debugs(93,5, HERE << "Will write " << allowHeader); + if (allow204 || allow206 || allowTrailers) { + buf.appendf("Allow: "); + if (allow204out) + buf.appendf("204, "); + if (allow206) + buf.appendf("206, "); + if (allowTrailers) + buf.appendf("trailers"); + buf.appendf("\r\n"); } } @@ -2015,3 +2058,10 @@ void Adaptation::Icap::ModXactLauncher::updateHistory(bool doStart) } } +bool Adaptation::Icap::TrailerParser::parse(const char *buf, int len, int atEnd, Http::StatusCode *error) { + const int parsed = trailer.parse(buf, len, atEnd, hdr_sz); + if (parsed < 0) + *error = Http::scInvalidHeader; // TODO: should we add a new Http::scInvalidTrailer? + return parsed > 0; +} + diff --git a/src/adaptation/icap/ModXact.h b/src/adaptation/icap/ModXact.h index 0dedb27535..89acf8e726 100644 --- a/src/adaptation/icap/ModXact.h +++ b/src/adaptation/icap/ModXact.h @@ -105,6 +105,21 @@ private: enum State { stDisabled, stWriting, stIeof, stDone } theState; }; +/// Parses and stores ICAP trailer header block. +class TrailerParser +{ +public: + TrailerParser() : trailer(hoReply), hdr_sz(0) {} + /// Parses trailers stored in a buffer. + /// \returns true and sets hdr_sz on success + /// \returns false and sets *error to zero when needs more data + /// \returns false and sets *error to a positive Http::StatusCode on error + bool parse(const char *buf, int len, int atEnd, Http::StatusCode *error); + HttpHeader trailer; + /// parsed trailer size if parse() was successful + size_t hdr_sz; // pedantic XXX: wrong type dictated by HttpHeader::parse() API +}; + class ModXact: public Xaction, public BodyProducer, public BodyConsumer { CBDATA_CLASS(ModXact); @@ -206,6 +221,7 @@ private: void decideOnParsingBody(); void parseBody(); + void parseIcapTrailer(); void maybeAllocateHttpMsg(); void handle100Continue(); @@ -231,7 +247,7 @@ private: void stopReceiving(); void stopSending(bool nicely); void stopWriting(bool nicely); - void stopParsing(); + void stopParsing(const bool checkUnparsedData = true); void stopBackup(); virtual void fillPendingStatus(MemBuf &buf) const; @@ -239,9 +255,22 @@ private: virtual bool fillVirginHttpHeader(MemBuf&) const; private: + /// parses a message header or trailer + /// \returns true on success + /// \returns false if more data is needed + /// \throw TextException on unrecoverable error + template + bool parsePart(Part *part, const char *description); + void packHead(MemBuf &httpBuf, const HttpMsg *head); void encapsulateHead(MemBuf &icapBuf, const char *section, MemBuf &httpBuf, const HttpMsg *head); bool gotEncapsulated(const char *section) const; + /// whether ICAP response header indicates HTTP header presence + bool expectHttpHeader() const; + /// whether ICAP response header indicates HTTP body presence + bool expectHttpBody() const; + /// whether ICAP response header indicates ICAP trailers presence + bool expectIcapTrailers() const; void checkConsuming(); virtual void finalizeLogInfo(); @@ -270,6 +299,8 @@ private: int adaptHistoryId; ///< adaptation history slot reservation + TrailerParser *trailerParser; + class State { @@ -304,7 +335,7 @@ private: parsing == psHttpHeader; } - enum Parsing { psIcapHeader, psHttpHeader, psBody, psDone } parsing; + enum Parsing { psIcapHeader, psHttpHeader, psBody, psIcapTrailer, psDone } parsing; // measures ICAP request writing progress enum Writing { writingInit, writingConnect, writingHeaders, diff --git a/src/adaptation/icap/OptXact.cc b/src/adaptation/icap/OptXact.cc index e89e68fda2..b0194af914 100644 --- a/src/adaptation/icap/OptXact.cc +++ b/src/adaptation/icap/OptXact.cc @@ -61,8 +61,10 @@ void Adaptation::Icap::OptXact::makeRequest(MemBuf &buf) if (!TheConfig.reuse_connections) buf.append("Connection: close\r\n", 19); + buf.append("Allow: ", 7); if (TheConfig.allow206_enable) - buf.append("Allow: 206\r\n", 12); + buf.append("206, ", 5); + buf.append("trailers\r\n", 10); buf.append(ICAP::crlf, 2); // XXX: HttpRequest cannot fully parse ICAP Request-Line diff --git a/src/mime_header.h b/src/mime_header.h index ca176f329c..e3589d0224 100644 --- a/src/mime_header.h +++ b/src/mime_header.h @@ -18,7 +18,10 @@ * - CRLF CRLF, or * - CRLF LF, or * - LF CRLF, or - * - LF LF + * - LF LF or, + * if mime header block is empty: + * - LF or + * - CRLF * * Also detects whether a obf-fold pattern exists within the mime block * - CR*LF (SP / HTAB)