From 8cccf1f8b06c9ec8ac966ffc30c65e2c644838a6 Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Wed, 5 Aug 2015 18:30:48 +0200 Subject: [PATCH] Tighten and rationalize checks on HTTP headers' validity. Remove operator<<(Http::HeaderType) as it's not accepted by clang. --- src/HttpHeader.cc | 34 ++++++++++++++++------------------ src/http/RegisteredHeaders.cc | 17 ++++------------- src/http/RegisteredHeaders.h | 22 ++++++++++++++-------- 3 files changed, 34 insertions(+), 39 deletions(-) diff --git a/src/HttpHeader.cc b/src/HttpHeader.cc index 4550675aab..1c22343cd6 100644 --- 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 4ba8cf2cfb..baa4efcf6f 100644 --- 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 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(id) << ')'; - else - s << "invalid" << '(' << static_cast(id) << ')'; - return s; -} - diff --git a/src/http/RegisteredHeaders.h b/src/http/RegisteredHeaders.h index 8555f3f8ac..27dd835aaa 100644 --- a/src/http/RegisteredHeaders.h +++ b/src/http/RegisteredHeaders.h @@ -10,7 +10,6 @@ #define SQUID_HTTP_REGISTEREDHEADERS_H #include "base/LookupTable.h" -#include 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 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 */ -- 2.47.2