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 d22c3fe775baa61ba4dfa5da5e72a6d032875d23..bfb189eeee7dfd4721eb4ad6fce51992be6705fd 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;
     }
@@ -825,7 +825,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;
 }
 
@@ -834,7 +834,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;
 }
 
@@ -842,29 +842,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;
@@ -885,7 +886,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 0f3dd93192d2aff8cd9fcc3cdbb5788f0c3387f1..1d0eee6ab6fe1283fcb9a52071fda7749dfa5a04 100644 (file)
--- a/src/HttpHeader.h
+++ b/src/HttpHeader.h
@@ -101,11 +101,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..99a1c151dcd088f6137ccddfeb34cb6230a4a22b 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