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 85f5126b7c47050aee3af6e55c005b63afd1da9b..ac53ee1223cc730ee13e48c4a81a5ab46cbfeb9b 100644 (file)
--- 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<String::size_type>(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 b85cdd0f2cfbe7e0945bbbd17335508a3bb9aa08..0a06802ad7b38c3a5a84898f6462d4b2ff253766 100644 (file)
--- 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 12bd6441171f5892493e9bd6ec35e5252d5fa207..28a55ea6f4fe05ab20e919972d5abd0d31db472f 100644 (file)
--- 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 bd7bc74ba76cfd4b96bfb841fd0e18bd4be81210..34d99095e9732ee80666321a7de465c7f057adf1 100644 (file)
--- 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<bool>(theHeader.has(squidId));
 }
 
 Adaptation::Ecap::HeaderRep::Value

diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc
index e7c4fe8bbf151896e8905efa7106191e359d3552..cbfc9ac41234218172603e112c551a616be7909e 100644 (file)
--- 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