Tighten and rationalize checks on HTTP headers' validity.

Remove operator<<(Http::HeaderType) as it's not accepted by clang.

diff --git a/src/HttpHeader.cc b/src/HttpHeader.cc
index 4550675aabc03fa1c53ae6d318e0d73dffa4a110..1c22343cd65054be4e43e5dd68ff9bdb8c0b6cd6 100644 (file)
--- a/src/HttpHeader.cc
+++ b/src/HttpHeader.cc
@@ -683,7 +683,7 @@ HttpHeader::findEntry(Http::HdrType id) const
 {
     HttpHeaderPos pos = HttpHeaderInitPos;
     HttpHeaderEntry *e;
-    assert(any_valid_header(id));
+    assert(any_registered_header(id));
     assert(!CBIT_TEST(ListHeadersMask, id));
 
     /* check mask first */
@@ -712,7 +712,7 @@ HttpHeader::findLastEntry(Http::HdrType id) const
     HttpHeaderPos pos = HttpHeaderInitPos;
     HttpHeaderEntry *e;
     HttpHeaderEntry *result = NULL;
-    assert(any_valid_header(id));
+    assert(any_registered_header(id));
     assert(!CBIT_TEST(ListHeadersMask, id));
 
     /* check mask first */
@@ -760,8 +760,7 @@ HttpHeader::delById(Http::HdrType id)
     HttpHeaderPos pos = HttpHeaderInitPos;
     HttpHeaderEntry *e;
     debugs(55, 8, this << " del-by-id " << id);
-    assert(any_valid_header(id));
-    assert(id != Http::HdrType::OTHER);        /* does not make sense */
+    assert(any_registered_header(id));
 
     if (!CBIT_TEST(mask, id))
         return 0;
@@ -1035,7 +1034,7 @@ HttpHeader::getListMember(Http::HdrType id, const char *member, const char separ
     int ilen;
     int mlen = strlen(member);
 
-    assert(any_valid_header(id));
+    assert(any_registered_header(id));
 
     header = getStrOrList(id);
     String result;
@@ -1055,8 +1054,7 @@ HttpHeader::getListMember(Http::HdrType id, const char *member, const char separ
 int
 HttpHeader::has(Http::HdrType id) const
 {
-    assert(any_valid_header(id));
-    assert(id != Http::HdrType::OTHER);
+    assert(any_registered_header(id));
     debugs(55, 9, this << " lookup for " << id);
     return CBIT_TEST(mask, id);
 }
@@ -1064,7 +1062,7 @@ HttpHeader::has(Http::HdrType id) const
 void
 HttpHeader::putInt(Http::HdrType id, int number)
 {
-    assert(any_valid_header(id));
+    assert(any_registered_header(id));
     assert(Http::HeaderTable[id].type == Http::HdrFieldType::ftInt);  /* must be of an appropriate type */
     assert(number >= 0);
     addEntry(new HttpHeaderEntry(id, NULL, xitoa(number)));
@@ -1073,7 +1071,7 @@ HttpHeader::putInt(Http::HdrType id, int number)
 void
 HttpHeader::putInt64(Http::HdrType id, int64_t number)
 {
-    assert(any_valid_header(id));
+    assert(any_registered_header(id));
     assert(Http::HeaderTable[id].type == Http::HdrFieldType::ftInt64);    /* must be of an appropriate type */
     assert(number >= 0);
     addEntry(new HttpHeaderEntry(id, NULL, xint64toa(number)));
@@ -1082,7 +1080,7 @@ HttpHeader::putInt64(Http::HdrType id, int64_t number)
 void
 HttpHeader::putTime(Http::HdrType id, time_t htime)
 {
-    assert(any_valid_header(id));
+    assert(any_registered_header(id));
     assert(Http::HeaderTable[id].type == Http::HdrFieldType::ftDate_1123);    /* must be of an appropriate type */
     assert(htime >= 0);
     addEntry(new HttpHeaderEntry(id, NULL, mkrfc1123(htime)));
@@ -1091,7 +1089,7 @@ HttpHeader::putTime(Http::HdrType id, time_t htime)
 void
 HttpHeader::insertTime(Http::HdrType id, time_t htime)
 {
-    assert(any_valid_header(id));
+    assert(any_registered_header(id));
     assert(Http::HeaderTable[id].type == Http::HdrFieldType::ftDate_1123);    /* must be of an appropriate type */
     assert(htime >= 0);
     insertEntry(new HttpHeaderEntry(id, NULL, mkrfc1123(htime)));
@@ -1100,7 +1098,7 @@ HttpHeader::insertTime(Http::HdrType id, time_t htime)
 void
 HttpHeader::putStr(Http::HdrType id, const char *str)
 {
-    assert(any_valid_header(id));
+    assert(any_registered_header(id));
     assert(Http::HeaderTable[id].type == Http::HdrFieldType::ftStr);  /* must be of an appropriate type */
     assert(str);
     addEntry(new HttpHeaderEntry(id, NULL, str));
@@ -1197,7 +1195,7 @@ HttpHeader::putExt(const char *name, const char *value)
 int
 HttpHeader::getInt(Http::HdrType id) const
 {
-    assert(any_valid_header(id));
+    assert(any_registered_header(id));
     assert(Http::HeaderTable[id].type == Http::HdrFieldType::ftInt);  /* must be of an appropriate type */
     HttpHeaderEntry *e;
 
@@ -1210,7 +1208,7 @@ HttpHeader::getInt(Http::HdrType id) const
 int64_t
 HttpHeader::getInt64(Http::HdrType id) const
 {
-    assert(any_valid_header(id));
+    assert(any_registered_header(id));
     assert(Http::HeaderTable[id].type == Http::HdrFieldType::ftInt64);    /* must be of an appropriate type */
     HttpHeaderEntry *e;
 
@@ -1225,7 +1223,7 @@ HttpHeader::getTime(Http::HdrType id) const
 {
     HttpHeaderEntry *e;
     time_t value = -1;
-    assert(any_valid_header(id));
+    assert(any_registered_header(id));
     assert(Http::HeaderTable[id].type == Http::HdrFieldType::ftDate_1123);    /* must be of an appropriate type */
 
     if ((e = findEntry(id))) {
@@ -1241,7 +1239,7 @@ const char *
 HttpHeader::getStr(Http::HdrType id) const
 {
     HttpHeaderEntry *e;
-    assert(any_valid_header(id));
+    assert(any_registered_header(id));
     assert(Http::HeaderTable[id].type == Http::HdrFieldType::ftStr);  /* must be of an appropriate type */
 
     if ((e = findEntry(id))) {
@@ -1257,7 +1255,7 @@ const char *
 HttpHeader::getLastStr(Http::HdrType id) const
 {
     HttpHeaderEntry *e;
-    assert(any_valid_header(id));
+    assert(any_registered_header(id));
     assert(Http::HeaderTable[id].type == Http::HdrFieldType::ftStr);  /* must be of an appropriate type */
 
     if ((e = findLastEntry(id))) {
@@ -1715,7 +1713,7 @@ HttpHeader::hasListMember(Http::HdrType id, const char *member, const char separ
     int ilen;
     int mlen = strlen(member);
 
-    assert(any_valid_header(id));
+    assert(any_registered_header(id));
 
     String header (getStrOrList(id));
 

diff --git a/src/http/RegisteredHeaders.cc b/src/http/RegisteredHeaders.cc
index 4ba8cf2cfba4e5370c9b103d66bf70586555a116..baa4efcf6f44a5b7d9c4b08acd9036171cac900e 100644 (file)
--- a/src/http/RegisteredHeaders.cc
+++ b/src/http/RegisteredHeaders.cc
@@ -13,12 +13,13 @@
 
 namespace Http
 {
-
 /*
  * A table with major attributes for every known field.
  *
  * Invariant on this table:
  * for each index in HeaderTable, (int)HeaderTable[index] = index
+ *
+ * To be kept in sync with Http::HdrType
  */
 const HeaderTableRecord HeaderTable[] = {
     {"Accept", Http::HdrType::ACCEPT, Http::HdrFieldType::ftStr},
@@ -112,21 +113,11 @@ const HeaderTableRecord HeaderTable[] = {
     {"FTP-Status", Http::HdrType::FTP_STATUS, Http::HdrFieldType::ftInt},
     {"FTP-Reason", Http::HdrType::FTP_REASON, Http::HdrFieldType::ftStr},
     {"Other:", Http::HdrType::OTHER, Http::HdrFieldType::ftStr},    /* ':' will not allow matches */
-    {nullptr, Http::HdrType::ENUM_END, Http::HdrFieldType::ftInvalid},    /* end of table */
-    {nullptr, Http::HdrType::BAD_HDR, Http::HdrFieldType::ftInvalid}
+    {"*INVALID*:", Http::HdrType::BAD_HDR, Http::HdrFieldType::ftInvalid}, /* ':' will not allow matches */
+    {nullptr, Http::HdrType::ENUM_END, Http::HdrFieldType::ftInvalid}    /* end of table */
 };
 
 const LookupTable<Http::HdrType, HeaderTableRecord> HeaderLookupTable(Http::HdrType::BAD_HDR, HeaderTable);
 
 }; /* namespace Http */
 
-extern std::ostream &
-operator << (std::ostream &s , Http::HdrType id)
-{
-    if (id >= Http::HdrType::ACCEPT && id < Http::HdrType::ENUM_END)
-        s << Http::HeaderTable[id].name << '(' << static_cast<int>(id) << ')';
-    else
-        s << "invalid" << '(' << static_cast<int>(id) << ')';
-    return s;
-}
-

diff --git a/src/http/RegisteredHeaders.h b/src/http/RegisteredHeaders.h
index 8555f3f8ac03fe841c6d47b3a36a4f6e9a0e97c8..27dd835aaaed4cbf41f72e191fe19b0b6e9ac3a2 100644 (file)
--- a/src/http/RegisteredHeaders.h
+++ b/src/http/RegisteredHeaders.h
@@ -10,7 +10,6 @@
 #define SQUID_HTTP_REGISTEREDHEADERS_H
 
 #include "base/LookupTable.h"
-#include <iosfwd>
 
 namespace Http
 {
@@ -120,8 +119,8 @@ enum HdrType {
     FTP_STATUS,                     /**< Internal header for FTP reply status */
     FTP_REASON,                     /**< Internal header for FTP reply reason */
     OTHER,                          /**< internal tag value for "unknown" headers */
-    ENUM_END,                       /**< internal tag for end-of-valid headers */
-    BAD_HDR                         /**< Invalid header. Must be after ENUM_END */
+    BAD_HDR,                        /**< Invalid header. Must be after ENUM_END */
+    ENUM_END                        /**< internal tag for end-of-headers */
 };
 
 /** possible types for http header fields */
@@ -159,22 +158,29 @@ extern const HeaderTableRecord HeaderTable[];
  */
 extern const LookupTable<Http::HdrType, HeaderTableRecord> HeaderLookupTable;
 
+/// match any known header type, including OTHER and BAD
 inline bool
 any_HdrType_enum_value (const Http::HdrType id)
 {
-    return (id == Http::HdrType::BAD_HDR || (id >= Http::HdrType::ACCEPT && id < Http::HdrType::ENUM_END));
+    return (id >= Http::HdrType::ACCEPT && id < Http::HdrType::ENUM_END);
 }
 
+/// match any valid header type, including OTHER but not BAD
 inline bool
 any_valid_header (const Http::HdrType id)
 {
-    return (id >= Http::HdrType::ACCEPT && id < Http::HdrType::ENUM_END);
+    return (id >= Http::HdrType::ACCEPT && id < Http::HdrType::BAD_HDR);
 }
 
-}; /* namespace Http */
+/// match any registered header type (headers squid knows how to handle),
+///  thus excluding OTHER and BAD
+inline bool
+any_registered_header (const Http::HdrType id)
+{
+    return (id >= Http::HdrType::ACCEPT && id < Http::HdrType::OTHER);
+}
 
-std::ostream &
-operator << (std::ostream &, Http::HdrType);
+}; /* namespace Http */
 
 #endif /* SQUID_HTTP_REGISTEREDHEADERS_H */