From: Francesco Chemolli Date: Tue, 27 Sep 2011 21:48:12 +0000 (+0200) Subject: avoided temporary in HttpHdrCc name-to-id lookup X-Git-Tag: BumpSslServerFirst.take01~126^2~6 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=cf7c2e948f9041a56336f52cd594da33479d7309;p=thirdparty%2Fsquid.git avoided temporary in HttpHdrCc name-to-id lookup Started implementing have/set/get/clear combo for all members of HttpHdrCc Fixed zero-pointer-dereference in http.cc --- diff --git a/src/HttpHdrCc.cc b/src/HttpHdrCc.cc index f06b6e68c3..6a9cb7e585 100644 --- a/src/HttpHdrCc.cc +++ b/src/HttpHdrCc.cc @@ -125,8 +125,7 @@ HttpHdrCc::parse(const String & str) nlen = ilen; /* find type */ - const StringArea tmpstr(item,nlen); - const CcNameToIdMap_t::const_iterator i=CcNameToIdMap.find(tmpstr); + const CcNameToIdMap_t::const_iterator i=CcNameToIdMap.find(StringArea(item,nlen)); if (i==CcNameToIdMap.end()) type=CC_OTHER; else @@ -139,20 +138,18 @@ HttpHdrCc::parse(const String & str) ++CcAttrs[type].stat.repCount; continue; } - } else { - set(type); } - /* post-processing special cases */ + /* post-processing, including special cases */ switch (type) { case CC_MAX_AGE: int32_t ma; if (!p || !httpHeaderParseInt(p, &ma)) { debugs(65, 2, "cc: invalid max-age specs near '" << item << "'"); - setMaxAge(MAX_AGE_UNSET); + maxAge(MAX_AGE_UNKNOWN); } else { - setMaxAge(ma); + maxAge(ma); } break; @@ -161,7 +158,7 @@ HttpHdrCc::parse(const String & str) if (!p || !httpHeaderParseInt(p, &s_maxage)) { debugs(65, 2, "cc: invalid s-maxage specs near '" << item << "'"); - setSMaxAge(S_MAXAGE_UNSET); + sMaxAge(S_MAXAGE_UNKNOWN); } break; @@ -179,7 +176,7 @@ HttpHdrCc::parse(const String & str) if (!p || !httpHeaderParseInt(p, &min_fresh)) { debugs(65, 2, "cc: invalid min-fresh specs near '" << item << "'"); - setMinFresh(MIN_FRESH_UNSET); + setMinFresh(MIN_FRESH_UNKNOWN); } break; @@ -187,7 +184,7 @@ HttpHdrCc::parse(const String & str) case CC_STALE_IF_ERROR: if (!p || !httpHeaderParseInt(p, &stale_if_error)) { debugs(65, 2, "cc: invalid stale-if-error specs near '" << item << "'"); - setStaleIfError(STALE_IF_ERROR_UNSET); + setStaleIfError(STALE_IF_ERROR_UNKNOWN); } break; @@ -201,6 +198,8 @@ HttpHdrCc::parse(const String & str) break; default: + + set(type); /* note that we ignore most of '=' specs (RFCVIOLATION) */ break; } @@ -225,10 +224,10 @@ httpHdrCcPackInto(const HttpHdrCc * cc, Packer * p) /* handle options with values */ if (flag == CC_MAX_AGE) - packerPrintf(p, "=%d", (int) cc->getMaxAge()); + packerPrintf(p, "=%d", (int) cc->maxAge()); if (flag == CC_S_MAXAGE) - packerPrintf(p, "=%d", (int) cc->getSMaxAge()); + packerPrintf(p, "=%d", (int) cc->sMaxAge()); if (flag == CC_MAX_STALE && cc->getMaxStale() >= 0) packerPrintf(p, "=%d", (int) cc->getMaxStale()); diff --git a/src/HttpHdrCc.cci b/src/HttpHdrCc.cci index adbb0bcf77..861b83698f 100644 --- a/src/HttpHdrCc.cci +++ b/src/HttpHdrCc.cci @@ -32,38 +32,19 @@ void -HttpHdrCc::setMaxAge(int max_age_) +HttpHdrCc::sMaxAge(int32_t s_maxage) { - /* keeping the mask in sync is needed e.g. for the packer functions */ - if (max_age_ >= 0) { - set(CC_MAX_AGE); - max_age = max_age_; - } else { - set(CC_MAX_AGE,false); - max_age=MAX_AGE_UNSET; - } -} - -int32_t -HttpHdrCc::getMaxAge() const -{ - return max_age; -} - -void -HttpHdrCc::setSMaxAge(int32_t s_maxage) -{ - if (s_maxage >= 0) { - set(CC_S_MAXAGE); + if (s_maxage>=0) { + setMask(CC_S_MAXAGE); this->s_maxage=s_maxage; } else { - set(CC_S_MAXAGE,false); - this->s_maxage=S_MAXAGE_UNSET; + setMask(CC_S_MAXAGE,false); + this->s_maxage=S_MAXAGE_UNKNOWN; } } int32_t -HttpHdrCc::getSMaxAge() const +HttpHdrCc::sMaxAge() const { return s_maxage; } @@ -72,11 +53,11 @@ void HttpHdrCc::setMaxStale(int32_t max_stale) { if (max_stale>=0 || max_stale==MAX_STALE_ALWAYS) { - set(CC_MAX_STALE); + setMask(CC_MAX_STALE); this->max_stale=max_stale; } else { - set(CC_MAX_STALE,false); - this->max_stale=MAX_STALE_UNSET; + setMask(CC_MAX_STALE,false); + this->max_stale=MAX_STALE_UNKNOWN; } } int32_t @@ -89,11 +70,11 @@ void HttpHdrCc::setStaleIfError(int32_t stale_if_error) { if (stale_if_error >= 0) { - set(CC_STALE_IF_ERROR); + setMask(CC_STALE_IF_ERROR); this->stale_if_error=stale_if_error; } else { - set(CC_STALE_IF_ERROR,false); - this->stale_if_error=STALE_IF_ERROR_UNSET; + setMask(CC_STALE_IF_ERROR,false); + this->stale_if_error=STALE_IF_ERROR_UNKNOWN; } } @@ -107,11 +88,11 @@ void HttpHdrCc::setMinFresh(int32_t min_fresh) { if (min_fresh >= 0) { - set(CC_MIN_FRESH); + setMask(CC_MIN_FRESH); this->min_fresh=min_fresh; } else { - set(CC_MIN_FRESH,false); - this->min_fresh=MIN_FRESH_UNSET; + setMask(CC_MIN_FRESH,false); + this->min_fresh=MIN_FRESH_UNKNOWN; } } @@ -132,6 +113,15 @@ void HttpHdrCc::set(http_hdr_cc_type id, bool newval) { assert(id>=CC_PUBLIC && id < CC_ENUM_END); + assert(id!=CC_MAX_AGE && id != CC_S_MAXAGE && + id != CC_MAX_STALE && id != CC_STALE_IF_ERROR && + id != CC_MIN_FRESH); + setMask(id,newval); +} + +void +HttpHdrCc::setMask(http_hdr_cc_type id, bool newval) +{ if (newval) EBIT_SET(mask,id); else diff --git a/src/HttpHdrCc.h b/src/HttpHdrCc.h index 4a0ca70870..18ad00d296 100644 --- a/src/HttpHdrCc.h +++ b/src/HttpHdrCc.h @@ -44,17 +44,17 @@ class HttpHdrCc { public: - static const int32_t MAX_AGE_UNSET=-1; //max-age is unset - static const int32_t S_MAXAGE_UNSET=-1; //s-maxage is unset - static const int32_t MAX_STALE_UNSET=-1; //max-stale is unset + static const int32_t MAX_AGE_UNKNOWN=-1; //max-age is unset + static const int32_t S_MAXAGE_UNKNOWN=-1; //s-maxage is unset + static const int32_t MAX_STALE_UNKNOWN=-1; //max-stale is unset static const int32_t MAX_STALE_ALWAYS=-2; //max-stale is set to no value - static const int32_t STALE_IF_ERROR_UNSET=-1; //stale_if_error is unset - static const int32_t MIN_FRESH_UNSET=-1; //min_fresh is unset + static const int32_t STALE_IF_ERROR_UNKNOWN=-1; //stale_if_error is unset + static const int32_t MIN_FRESH_UNKNOWN=-1; //min_fresh is unset HttpHdrCc() : - mask(0), max_age(MAX_AGE_UNSET), s_maxage(S_MAXAGE_UNSET), - max_stale(MAX_STALE_UNSET), stale_if_error(STALE_IF_ERROR_UNSET), - min_fresh(MIN_FRESH_UNSET) {} + mask(0), max_age(MAX_AGE_UNKNOWN), s_maxage(S_MAXAGE_UNKNOWN), + max_stale(MAX_STALE_UNKNOWN), stale_if_error(STALE_IF_ERROR_UNKNOWN), + min_fresh(MIN_FRESH_UNKNOWN) {} /// reset data-members to default state void clear(); @@ -62,31 +62,82 @@ public: /// parse a header-string and fill in appropriate values. bool parse(const String & s); - /// set an attribute value or clear it (by supplying false as the second argument) - _SQUID_INLINE_ void set(http_hdr_cc_type id, bool newval=true); - /// check whether the attribute value supplied by id is set - _SQUID_INLINE_ bool isSet(http_hdr_cc_type id) const; - - /// max-age setter. Clear by setting to MAX_AGE_UNSET - _SQUID_INLINE_ void setMaxAge(int32_t max_age); - _SQUID_INLINE_ int32_t getMaxAge() const; - - /// s-maxage setter. Clear by setting to S_MAXAGE_UNSET - _SQUID_INLINE_ void setSMaxAge(int32_t s_maxage); - _SQUID_INLINE_ int32_t getSMaxAge() const; - - /// max-stale setter. Clear by setting to MAX_STALE_UNSET + //manipulation for Cache-Control: XXX header + //inline bool haveXXX() const {return isSet();} + //inline bool XXX() const {return isSet();} + //inline void XXX(bool newval) {setMask(,newval);} + //inline void clearXXX() {setMask(,false);} + + //manipulation for Cache-Control: public header + inline bool havePublic() const {return isSet(CC_PUBLIC);} + inline bool getPublic() const {return isSet(CC_PUBLIC);} + inline void setPublic(bool newval) {setMask(CC_PUBLIC,newval);} + inline void clearPublic() {setMask(CC_PUBLIC,false);} + + //manipulation for Cache-Control: private header + inline bool havePrivate() const {return isSet(CC_PRIVATE);} + inline bool getPrivate() const {return isSet(CC_PRIVATE);} + inline void setPrivate(bool newval) {setMask(CC_PRIVATE,newval);} + inline void clearPrivate() {setMask(CC_PRIVATE,false);} + + //manipulation for Cache-Control: no-cache header + inline bool haveNoCache() const {return isSet(CC_NO_CACHE);} + inline bool noCache() const {return isSet(CC_NO_CACHE);} + inline void noCache(bool newval) {setMask(CC_NO_CACHE,newval);} + inline void clearNoCache() {setMask(CC_NO_CACHE,false);} + + //manipulation for Cache-Control: no-store header + inline bool haveNoStore() const {return isSet(CC_NO_STORE);} + inline bool noStore() const {return isSet(CC_NO_STORE);} + inline void noStore(bool newval) {setMask(CC_NO_STORE,newval);} + inline void clearNoStore() {setMask(CC_NO_STORE,false);} + + //manipulation for Cache-Control: no-transform header + inline bool haveNoTransform() const {return isSet(CC_NO_TRANSFORM);} + inline bool noTransform() const {return isSet(CC_NO_TRANSFORM);} + inline void noTransform(bool newval) {setMask(CC_NO_TRANSFORM,newval);} + inline void clearNoTransform() {setMask(CC_NO_TRANSFORM,false);} + + //manipulation for Cache-Control: must-revalidate header + inline bool haveMustRevalidate() const {return isSet(CC_MUST_REVALIDATE);} + inline bool mustRevalidate() const {return isSet(CC_MUST_REVALIDATE);} + inline void mustRevalidate(bool newval) {setMask(CC_MUST_REVALIDATE,newval);} + inline void clearMustRevalidate() {setMask(CC_MUST_REVALIDATE,false);} + + //manipulation for Cache-Control: proxy-revalidate header + inline bool haveProxyRevalidate() const {return isSet(CC_PROXY_REVALIDATE);} + inline bool proxyRevalidate() const {return isSet(CC_PROXY_REVALIDATE);} + inline void proxyRevalidate(bool newval) {setMask(CC_PROXY_REVALIDATE,newval);} + inline void clearProxyRevalidate() {setMask(CC_PROXY_REVALIDATE,false);} + + //manipulation for Cache-Control: max-age header + inline bool haveMaxAge() const {return isSet(CC_MAX_AGE);} + inline int32_t maxAge() const { return max_age;} + inline void maxAge(int32_t max_age) {this->max_age = max_age; setMask(CC_MAX_AGE); } + inline void clearMaxAge() {max_age=MAX_AGE_UNKNOWN; setMask(CC_MAX_AGE,false);} + + + /// s-maxage setter. Clear by setting to S_MAXAGE_UNKNOWN + _SQUID_INLINE_ void sMaxAge(int32_t s_maxage); + _SQUID_INLINE_ int32_t sMaxAge() const; + + /// max-stale setter. Clear by setting to MAX_STALE_UNKNOWN _SQUID_INLINE_ void setMaxStale(int32_t max_stale); _SQUID_INLINE_ int32_t getMaxStale() const; - /// stale-if-error setter. Clear by setting to STALE_IF_ERROR_UNSET + /// stale-if-error setter. Clear by setting to STALE_IF_ERROR_UNKNOWN _SQUID_INLINE_ void setStaleIfError(int32_t stale_if_error); _SQUID_INLINE_ int32_t getStaleIfError() const; - /// min-fresh setter. Clear by setting to MIN_FRESH_UNSET + /// min-fresh setter. Clear by setting to MIN_FRESH_UNKNOWN _SQUID_INLINE_ void setMinFresh(int32_t min_fresh); _SQUID_INLINE_ int32_t getMinFresh() const; + /// set an attribute value or clear it (by supplying false as the second argument) + _SQUID_INLINE_ void set(http_hdr_cc_type id, bool newval=true); + /// check whether the attribute value supplied by id is set + _SQUID_INLINE_ bool isSet(http_hdr_cc_type id) const; + MEMPROXY_CLASS(HttpHdrCc); /** bit-mask representing what header values are set among those @@ -101,6 +152,9 @@ private: int32_t max_stale; int32_t stale_if_error; int32_t min_fresh; + /// low-level part of the public set method, performs no checks + _SQUID_INLINE_ void setMask(http_hdr_cc_type id, bool newval=true); + public: /**comma-separated representation of the header values which were * received but are not recognized. diff --git a/src/HttpReply.cc b/src/HttpReply.cc index 6ff6694561..1559c1d206 100644 --- a/src/HttpReply.cc +++ b/src/HttpReply.cc @@ -331,21 +331,21 @@ HttpReply::hdrExpirationTime() if (cache_control) { if (date >= 0) { - if (cache_control->getSMaxAge() != HttpHdrCc::S_MAXAGE_UNSET) - return date + cache_control->getSMaxAge(); + if (cache_control->sMaxAge() != HttpHdrCc::S_MAXAGE_UNKNOWN) + return date + cache_control->sMaxAge(); - if (cache_control->getMaxAge() != HttpHdrCc::MAX_AGE_UNSET) - return date + cache_control->getMaxAge(); + if (cache_control->isSet(CC_MAX_AGE)) + return date + cache_control->maxAge(); } else { /* * Conservatively handle the case when we have a max-age * header, but no Date for reference? */ - if (cache_control->getSMaxAge() != HttpHdrCc::S_MAXAGE_UNSET) + if (cache_control->sMaxAge() != HttpHdrCc::S_MAXAGE_UNKNOWN) return squid_curtime; - if (cache_control->getMaxAge() != HttpHdrCc::MAX_AGE_UNSET) + if (cache_control->isSet(CC_MAX_AGE)) return squid_curtime; } } diff --git a/src/base/StringArea.h b/src/base/StringArea.h index 1ffda12716..719bd1f850 100644 --- a/src/base/StringArea.h +++ b/src/base/StringArea.h @@ -47,13 +47,20 @@ */ class StringArea { public: + /// build a StringArea by explicitly assigning pointed-to area and and length StringArea(const char * ptr, size_t len): theStart(ptr), theLen(len) {} - bool operator==(const StringArea &s) const { return theLen==s.theLen && strncmp(theStart,s.theStart,theLen)==0; } + bool operator==(const StringArea &s) const { return theLen==s.theLen && memcmp(theStart,s.theStart,theLen)==0; } bool operator!=(const StringArea &s) const { return !operator==(s); } - bool operator< ( const StringArea &s) const { return theLen < s.theLen || strncmp(theStart,s.theStart,theLen) < 0; } + bool operator< ( const StringArea &s) const { + return (theLen < s.theLen || (theLen == s.theLen && memcmp(theStart,s.theStart,theLen) < 0)) ; } private: + // default constructor is disabled + StringArea(); + + /// pointed to the externally-managed memory area const char *theStart; + /// length of the string size_t theLen; }; diff --git a/src/http.cc b/src/http.cc index 96c1f3690a..e826ce3ae4 100644 --- a/src/http.cc +++ b/src/http.cc @@ -357,7 +357,7 @@ HttpStateData::cacheableReply() !REFRESH_OVERRIDE(ignore_no_store)) return 0; - if (!ignoreCacheControl) { + if (!ignoreCacheControl && request->cache_control != NULL) { const HttpHdrCc* cc=request->cache_control; if (cc->isSet(CC_PRIVATE)) { if (!REFRESH_OVERRIDE(ignore_private)) @@ -928,7 +928,7 @@ no_cache: if (!ignoreCacheControl && rep->cache_control) { if (rep->cache_control->isSet(CC_PROXY_REVALIDATE) || rep->cache_control->isSet(CC_MUST_REVALIDATE) || - rep->cache_control->getSMaxAge() != HttpHdrCc::S_MAXAGE_UNSET + rep->cache_control->sMaxAge() != HttpHdrCc::S_MAXAGE_UNKNOWN ) EBIT_SET(entry->flags, ENTRY_REVALIDATE); } @@ -1771,10 +1771,10 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request, #endif /* Add max-age only without no-cache */ - if (cc->getMaxAge()==HttpHdrCc::MAX_AGE_UNSET && !cc->isSet(CC_NO_CACHE)) { + if (cc->isSet(CC_MAX_AGE) && !cc->isSet(CC_NO_CACHE)) { const char *url = entry ? entry->url() : urlCanonical(request); - cc->setMaxAge(getMaxAge(url)); + cc->maxAge(getMaxAge(url)); } diff --git a/src/mime.cc b/src/mime.cc index 1d8c4b485b..7c7af825c4 100644 --- a/src/mime.cc +++ b/src/mime.cc @@ -465,7 +465,7 @@ MimeIcon::created (StoreEntry *newEntry) reply->cache_control = new HttpHdrCc(); - reply->cache_control->setMaxAge(86400); + reply->cache_control->maxAge(86400); reply->header.putCc(reply->cache_control); diff --git a/src/refresh.cc b/src/refresh.cc index af4fdc7afa..97450d5673 100644 --- a/src/refresh.cc +++ b/src/refresh.cc @@ -268,7 +268,7 @@ refreshCheck(const StoreEntry * entry, HttpRequest * request, time_t delta) if (request && !request->flags.ignore_cc) { const HttpHdrCc *const cc = request->cache_control; - if (cc && cc->getMinFresh()!=HttpHdrCc::MIN_FRESH_UNSET) { + if (cc && cc->getMinFresh()!=HttpHdrCc::MIN_FRESH_UNKNOWN) { const int32_t minFresh=cc->getMinFresh(); debugs(22, 3, "\tage + min-fresh:\t" << age << " + " << minFresh << " = " << age + minFresh); @@ -288,7 +288,7 @@ refreshCheck(const StoreEntry * entry, HttpRequest * request, time_t delta) // stale-if-error requires any failure be passed thru when its period is over. if (request && entry->mem_obj && entry->mem_obj->getReply() && entry->mem_obj->getReply()->cache_control && - entry->mem_obj->getReply()->cache_control->getStaleIfError() != HttpHdrCc::STALE_IF_ERROR_UNSET && + entry->mem_obj->getReply()->cache_control->getStaleIfError() != HttpHdrCc::STALE_IF_ERROR_UNKNOWN && entry->mem_obj->getReply()->cache_control->getStaleIfError() < staleness) { debugs(22, 3, "refreshCheck: stale-if-error period expired."); @@ -336,19 +336,19 @@ refreshCheck(const StoreEntry * entry, HttpRequest * request, time_t delta) #endif if (NULL != cc) { - if (cc->getMaxAge() != HttpHdrCc::MAX_AGE_UNSET) { + if (cc->isSet(CC_MAX_AGE)) { #if USE_HTTP_VIOLATIONS - if (R->flags.ignore_reload && cc->getMaxAge() == 0) { + if (R->flags.ignore_reload && cc->maxAge() == 0) { debugs(22, 3, "refreshCheck: MAYBE: client-max-age = 0 and ignore-reload"); } else #endif { - if (cc->getMaxAge() == 0) { + if (cc->maxAge() == 0) { debugs(22, 3, "refreshCheck: YES: client-max-age = 0"); return STALE_EXCEEDS_REQUEST_MAX_AGE_VALUE; } - if (age > cc->getMaxAge()) { + if (age > cc->maxAge()) { debugs(22, 3, "refreshCheck: YES: age > client-max-age"); return STALE_EXCEEDS_REQUEST_MAX_AGE_VALUE; }