From: Eduard Bagdasaryan Date: Wed, 15 Feb 2017 18:54:02 +0000 (-0700) Subject: Fix empty header handling in Ecap::HeaderRep::hasAny(). X-Git-Tag: M-staged-PR71~269 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f29d429e7df8879f147f87d6e00b0b0936d82b8d;p=thirdparty%2Fsquid.git Fix empty header handling in Ecap::HeaderRep::hasAny(). The method returned false for present but empty "unknown" headers. The fixed version is also faster, closing an old optimization XXX. --- diff --git a/src/HttpHeader.cc b/src/HttpHeader.cc index 85f5126b7c..ac53ee1223 100644 --- a/src/HttpHeader.cc +++ b/src/HttpHeader.cc @@ -248,7 +248,7 @@ HttpHeader::needUpdate(HttpHeader const *fresh) const continue; String value; const char *name = e->name.termedBuf(); - if (!getByNameIfPresent(name, strlen(name), value) || + if (!hasNamed(name, strlen(name), &value) || (value != fresh->getByName(name))) return true; } @@ -870,7 +870,7 @@ HttpHeader::getByName(const char *name) const { String result; // ignore presence: return undefined string if an empty header is present - (void)getByNameIfPresent(name, strlen(name), result); + (void)hasNamed(name, strlen(name), &result); return result; } @@ -879,7 +879,7 @@ HttpHeader::getByName(const SBuf &name) const { String result; // ignore presence: return undefined string if an empty header is present - (void)getByNameIfPresent(name, result); + (void)hasNamed(name, &result); return result; } @@ -887,29 +887,30 @@ String HttpHeader::getById(Http::HdrType id) const { String result; - (void)getByIdIfPresent(id,result); + (void)getByIdIfPresent(id, &result); return result; } bool -HttpHeader::getByNameIfPresent(const SBuf &s, String &result) const +HttpHeader::hasNamed(const SBuf &s, String *result) const { - return getByNameIfPresent(s.rawContent(), s.length(), result); + return hasNamed(s.rawContent(), s.length(), result); } bool -HttpHeader::getByIdIfPresent(Http::HdrType id, String &result) const +HttpHeader::getByIdIfPresent(Http::HdrType id, String *result) const { if (id == Http::HdrType::BAD_HDR) return false; if (!has(id)) return false; - result = getStrOrList(id); + if (result) + *result = getStrOrList(id); return true; } bool -HttpHeader::getByNameIfPresent(const char *name, int namelen, String &result) const +HttpHeader::hasNamed(const char *name, int namelen, String *result) const { Http::HdrType id; HttpHeaderPos pos = HttpHeaderInitPos; @@ -930,7 +931,9 @@ HttpHeader::getByNameIfPresent(const char *name, int namelen, String &result) co while ((e = getEntry(&pos))) { if (e->id == Http::HdrType::OTHER && e->name.size() == static_cast(namelen) && e->name.caseCmp(name, namelen) == 0) { found = true; - strListAdd(&result, e->value.termedBuf(), ','); + if (!result) + break; + strListAdd(result, e->value.termedBuf(), ','); } } diff --git a/src/HttpHeader.h b/src/HttpHeader.h index b85cdd0f2c..0a06802ad7 100644 --- a/src/HttpHeader.h +++ b/src/HttpHeader.h @@ -105,11 +105,13 @@ public: String getByName(const SBuf &name) const; String getByName(const char *name) const; String getById(Http::HdrType id) const; - /// sets value and returns true iff a [possibly empty] field identified by id is there - bool getByIdIfPresent(Http::HdrType id, String &result) const; - /// sets value and returns true iff a [possibly empty] named field is there - bool getByNameIfPresent(const SBuf &s, String &value) const; - bool getByNameIfPresent(const char *name, int namelen, String &value) const; + /// returns true iff a [possibly empty] field identified by id is there + /// when returning true, also sets the `result` parameter (if it is not nil) + bool getByIdIfPresent(Http::HdrType id, String *result) const; + /// returns true iff a [possibly empty] named field is there + /// when returning true, also sets the `value` parameter (if it is not nil) + bool hasNamed(const SBuf &s, String *value = 0) const; + bool hasNamed(const char *name, int namelen, String *value = 0) const; String getByNameListMember(const char *name, const char *member, const char separator) const; String getListMember(Http::HdrType id, const char *member, const char separator) const; int has(Http::HdrType id) const; diff --git a/src/acl/HttpHeaderData.cc b/src/acl/HttpHeaderData.cc index 12bd644117..28a55ea6f4 100644 --- a/src/acl/HttpHeaderData.cc +++ b/src/acl/HttpHeaderData.cc @@ -48,7 +48,7 @@ ACLHTTPHeaderData::match(HttpHeader* hdr) return false; value = hdr->getStrOrList(hdrId); } else { - if (!hdr->getByNameIfPresent(hdrName, value)) + if (!hdr->hasNamed(hdrName, &value)) return false; } diff --git a/src/adaptation/ecap/MessageRep.cc b/src/adaptation/ecap/MessageRep.cc index bd7bc74ba7..34d99095e9 100644 --- a/src/adaptation/ecap/MessageRep.cc +++ b/src/adaptation/ecap/MessageRep.cc @@ -33,10 +33,9 @@ bool Adaptation::Ecap::HeaderRep::hasAny(const Name &name) const { const Http::HdrType squidId = TranslateHeaderId(name); - // XXX: optimize to remove getByName: we do not need the value here return squidId == Http::HdrType::OTHER ? - theHeader.getByName(name.image().c_str()).size() > 0: - (bool)theHeader.has(squidId); + theHeader.hasNamed(name.image().c_str(), name.image().size()) : + static_cast(theHeader.has(squidId)); } Adaptation::Ecap::HeaderRep::Value diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc index e7c4fe8bbf..cbfc9ac412 100644 --- a/src/adaptation/icap/ModXact.cc +++ b/src/adaptation/icap/ModXact.cc @@ -1127,7 +1127,7 @@ bool Adaptation::Icap::ModXact::expectHttpBody() const bool Adaptation::Icap::ModXact::expectIcapTrailers() const { String trailers; - const bool promisesToSendTrailer = icapReply->header.getByIdIfPresent(Http::HdrType::TRAILER, 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