From: Francesco Chemolli Date: Wed, 28 Sep 2011 22:42:19 +0000 (+0200) Subject: Explicitly check that integer-carrying Cache-control directives are positive. X-Git-Tag: BumpSslServerFirst.take01~126^2~4 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d74ad83fac327bce0bbc91aa7fa932de5ae34dc8;p=thirdparty%2Fsquid.git Explicitly check that integer-carrying Cache-control directives are positive. Rename MAX_STALE_ALWAYS to MAX_STALE_ANY and changed its representation to very large positive integer. Implemented internal HttpHdrCc::setValue call. Removed unnecessarily explicitly-masked StringArea default constructor. --- diff --git a/src/HttpHdrCc.cc b/src/HttpHdrCc.cc index 4ec42a3582..d6c18e565c 100644 --- a/src/HttpHdrCc.cc +++ b/src/HttpHdrCc.cc @@ -69,7 +69,7 @@ static HttpHeaderCcFields CcAttrs[CC_ENUM_END] = { typedef std::map CcNameToIdMap_t; static CcNameToIdMap_t CcNameToIdMap; -/// iterate over a table of http_header_cc_type structs +/// used to walk a table of http_header_cc_type structs http_hdr_cc_type &operator++ (http_hdr_cc_type &aHeader) { int tmp = (int)aHeader; @@ -144,40 +144,47 @@ HttpHdrCc::parse(const String & str) switch (type) { case CC_MAX_AGE: - int32_t ma; - if (!p || !httpHeaderParseInt(p, &ma)) { + if (!p || !httpHeaderParseInt(p, &max_age) || max_age < 0) { debugs(65, 2, "cc: invalid max-age specs near '" << item << "'"); clearMaxAge(); } else { - maxAge(ma); + setMask(type,true); } break; case CC_S_MAXAGE: - if (!p || !httpHeaderParseInt(p, &s_maxage)) { + if (!p || !httpHeaderParseInt(p, &s_maxage) || s_maxage < 0) { debugs(65, 2, "cc: invalid s-maxage specs near '" << item << "'"); clearSMaxAge(); + } else { + setMask(type,true); } break; case CC_MAX_STALE: - if (!p || !httpHeaderParseInt(p, &max_stale)) { + if (!p || !httpHeaderParseInt(p, &max_stale) || max_stale < 0) { debugs(65, 2, "cc: max-stale directive is valid without value"); - maxStale(MAX_STALE_ALWAYS); + maxStale(MAX_STALE_ANY); + } else { + setMask(type,true); } break; case CC_MIN_FRESH: - if (!p || !httpHeaderParseInt(p, &min_fresh)) { + if (!p || !httpHeaderParseInt(p, &min_fresh) || min_fresh < 0) { debugs(65, 2, "cc: invalid min-fresh specs near '" << item << "'"); clearMinFresh(); + } else { + setMask(type,true); } break; case CC_STALE_IF_ERROR: - if (!p || !httpHeaderParseInt(p, &stale_if_error)) { + if (!p || !httpHeaderParseInt(p, &stale_if_error) || stale_if_error < 0) { debugs(65, 2, "cc: invalid stale-if-error specs near '" << item << "'"); clearStaleIfError(); + } else { + setMask(type,true); } break; @@ -227,7 +234,7 @@ HttpHdrCc::packInto(Packer * p) const if (flag == CC_S_MAXAGE) packerPrintf(p, "=%d", (int) sMaxAge()); - if (flag == CC_MAX_STALE && maxStale()!=MAX_STALE_ALWAYS) + if (flag == CC_MAX_STALE && maxStale()!=MAX_STALE_ANY) packerPrintf(p, "=%d", (int) maxStale()); if (flag == CC_MIN_FRESH) diff --git a/src/HttpHdrCc.cci b/src/HttpHdrCc.cci index 0c9011fb6c..1f239be1fa 100644 --- a/src/HttpHdrCc.cci +++ b/src/HttpHdrCc.cci @@ -44,3 +44,20 @@ HttpHdrCc::setMask(http_hdr_cc_type id, bool newval) else EBIT_CLR(mask,id); } + +/// set a data member to a new value, and set the corresponding mask-bit. +/// if setting is false, then the mask-bit is cleared. +void +HttpHdrCc::setValue(int32_t &value, int32_t new_value, http_hdr_cc_type hdr, bool setting) +{ + if (setting) { + if (new_value < 0) + debugs(65,3,HERE << "rejecting negative Cache-Control directive " << hdr + << " value " << new_value ); + } else { + new_value=-1; //rely on the convention that "unknown" is -1 + } + + value=new_value; + setMask(hdr,setting); +} diff --git a/src/HttpHdrCc.h b/src/HttpHdrCc.h index 6d4d2b41c8..5774c693a1 100644 --- a/src/HttpHdrCc.h +++ b/src/HttpHdrCc.h @@ -47,7 +47,7 @@ public: 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 MAX_STALE_ANY=0x7fffffff; //max-stale is set to no value, meaning "any" 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 @@ -63,87 +63,85 @@ public: bool parse(const String & s); //manipulation for Cache-Control: public header - inline bool havePublic() const {return isSet(CC_PUBLIC);} - inline bool Public() const {return isSet(CC_PUBLIC);} - inline void Public(bool newval) {setMask(CC_PUBLIC,newval);} - inline void clearPublic() {setMask(CC_PUBLIC,false);} + bool hasPublic() const {return isSet(CC_PUBLIC);} + bool Public() const {return isSet(CC_PUBLIC);} + void Public(bool v) {setMask(CC_PUBLIC,v);} + void clearPublic() {setMask(CC_PUBLIC,false);} //manipulation for Cache-Control: private header - inline bool havePrivate() const {return isSet(CC_PRIVATE);} - inline bool Private() const {return isSet(CC_PRIVATE);} - inline void Private(bool newval) {setMask(CC_PRIVATE,newval);} - inline void clearPrivate() {setMask(CC_PRIVATE,false);} + bool hasPrivate() const {return isSet(CC_PRIVATE);} + bool Private() const {return isSet(CC_PRIVATE);} + void Private(bool v) {setMask(CC_PRIVATE,v);} + 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);} + bool hasNoCache() const {return isSet(CC_NO_CACHE);} + bool noCache() const {return isSet(CC_NO_CACHE);} + void noCache(bool v) {setMask(CC_NO_CACHE,v);} + 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);} + bool hasNoStore() const {return isSet(CC_NO_STORE);} + bool noStore() const {return isSet(CC_NO_STORE);} + void noStore(bool v) {setMask(CC_NO_STORE,v);} + 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);} + bool hasNoTransform() const {return isSet(CC_NO_TRANSFORM);} + bool noTransform() const {return isSet(CC_NO_TRANSFORM);} + void noTransform(bool v) {setMask(CC_NO_TRANSFORM,v);} + 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);} + bool hasMustRevalidate() const {return isSet(CC_MUST_REVALIDATE);} + bool mustRevalidate() const {return isSet(CC_MUST_REVALIDATE);} + void mustRevalidate(bool v) {setMask(CC_MUST_REVALIDATE,v);} + 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);} + bool hasProxyRevalidate() const {return isSet(CC_PROXY_REVALIDATE);} + bool proxyRevalidate() const {return isSet(CC_PROXY_REVALIDATE);} + void proxyRevalidate(bool v) {setMask(CC_PROXY_REVALIDATE,v);} + 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 newval) {if (newval < 0) return; max_age = newval; setMask(CC_MAX_AGE); } - inline void clearMaxAge() {max_age=MAX_AGE_UNKNOWN; setMask(CC_MAX_AGE,false);} + bool hasMaxAge() const {return isSet(CC_MAX_AGE);} + int32_t maxAge() const { return max_age;} + void maxAge(int32_t v) {setValue(max_age,v,CC_MAX_AGE); } + void clearMaxAge() {setValue(max_age,MAX_AGE_UNKNOWN,CC_MAX_AGE,false);} //manipulation for Cache-Control: s-maxage header - inline bool haveSMaxAge() const {return isSet(CC_S_MAXAGE);} - inline int32_t sMaxAge() const { return s_maxage;} - inline void sMaxAge(int32_t newval) {if (newval < 0) return; s_maxage = newval; setMask(CC_S_MAXAGE); } - inline void clearSMaxAge() {s_maxage=MAX_AGE_UNKNOWN; setMask(CC_S_MAXAGE,false);} + bool hasSMaxAge() const {return isSet(CC_S_MAXAGE);} + int32_t sMaxAge() const { return s_maxage;} + void sMaxAge(int32_t v) {setValue(s_maxage,v,CC_S_MAXAGE); } + void clearSMaxAge() {setValue(s_maxage,MAX_AGE_UNKNOWN,CC_S_MAXAGE,false);} //manipulation for Cache-Control: max-stale header - inline bool haveMaxStale() const {return isSet(CC_MAX_STALE);} - inline int32_t maxStale() const { return max_stale;} - // max-stale has a special value (MAX_STALE_ALWAYS) which correspond to having + bool hasMaxStale() const {return isSet(CC_MAX_STALE);} + int32_t maxStale() const { return max_stale;} + // max-stale has a special value (MAX_STALE_ANY) which correspond to having // the directive without a numeric specification, and directs to consider the object // as always-expired. - inline void maxStale(int32_t newval) { - if (newval < 0 && newval != CC_MAX_STALE) return; - max_stale = newval; setMask(CC_MAX_STALE); } - inline void clearMaxStale() {max_stale=MAX_STALE_UNKNOWN; setMask(CC_MAX_STALE,false);} + void maxStale(int32_t v) {setValue(max_stale,v,CC_MAX_STALE);} + void clearMaxStale() {setValue(max_stale,MAX_STALE_UNKNOWN,CC_MAX_STALE,false);} //manipulation for Cache-Control:min-fresh header - inline bool haveMinFresh() const {return isSet(CC_MIN_FRESH);} - inline int32_t minFresh() const { return min_fresh;} - inline void minFresh(int32_t newval) {if (newval < 0) return; min_fresh = newval; setMask(CC_MIN_FRESH); } - inline void clearMinFresh() {min_fresh=MIN_FRESH_UNKNOWN; setMask(CC_MIN_FRESH,false);} + bool hasMinFresh() const {return isSet(CC_MIN_FRESH);} + int32_t minFresh() const { return min_fresh;} + void minFresh(int32_t v) {if (v < 0) return; setValue(min_fresh,v,CC_MIN_FRESH); } + void clearMinFresh() {setValue(min_fresh,MIN_FRESH_UNKNOWN,CC_MIN_FRESH,false);} //manipulation for Cache-Control: only-if-cached header - inline bool haveOnlyIfCached() const {return isSet(CC_ONLY_IF_CACHED);} - inline bool onlyIfCached() const {return isSet(CC_ONLY_IF_CACHED);} - inline void onlyIfCached(bool newval) {setMask(CC_ONLY_IF_CACHED,newval);} - inline void clearOnlyIfCached() {setMask(CC_ONLY_IF_CACHED,false);} + bool hasOnlyIfCached() const {return isSet(CC_ONLY_IF_CACHED);} + bool onlyIfCached() const {return isSet(CC_ONLY_IF_CACHED);} + void onlyIfCached(bool v) {setMask(CC_ONLY_IF_CACHED,v);} + void clearOnlyIfCached() {setMask(CC_ONLY_IF_CACHED,false);} //manipulation for Cache-Control: stale-if-error header - inline bool haveStaleIfError() const {return isSet(CC_STALE_IF_ERROR);} - inline int32_t staleIfError() const { return stale_if_error;} - inline void staleIfError(int32_t newval) {if (newval < 0) return; stale_if_error = newval; setMask(CC_STALE_IF_ERROR); } - inline void clearStaleIfError() {stale_if_error=STALE_IF_ERROR_UNKNOWN; setMask(CC_STALE_IF_ERROR,false);} + bool hasStaleIfError() const {return isSet(CC_STALE_IF_ERROR);} + int32_t staleIfError() const { return stale_if_error;} + void staleIfError(int32_t v) {setValue(stale_if_error,v,CC_STALE_IF_ERROR); } + void clearStaleIfError() {setValue(stale_if_error,STALE_IF_ERROR_UNKNOWN,CC_STALE_IF_ERROR,false);} /// check whether the attribute value supplied by id is set _SQUID_INLINE_ bool isSet(http_hdr_cc_type id) const; @@ -166,6 +164,7 @@ private: 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); + _SQUID_INLINE_ void setValue(int32_t &value, int32_t new_value, http_hdr_cc_type hdr, bool setting=true); public: /**comma-separated representation of the header values which were diff --git a/src/HttpReply.cc b/src/HttpReply.cc index 1a1a910f74..e41bddfae9 100644 --- a/src/HttpReply.cc +++ b/src/HttpReply.cc @@ -331,10 +331,10 @@ HttpReply::hdrExpirationTime() if (cache_control) { if (date >= 0) { - if (cache_control->haveSMaxAge()) + if (cache_control->hasSMaxAge()) return date + cache_control->sMaxAge(); - if (cache_control->haveMaxAge()) + if (cache_control->hasMaxAge()) return date + cache_control->maxAge(); } else { /* @@ -342,10 +342,10 @@ HttpReply::hdrExpirationTime() * header, but no Date for reference? */ - if (cache_control->haveSMaxAge()) + if (cache_control->hasSMaxAge()) return squid_curtime; - if (cache_control->haveMaxAge()) + if (cache_control->hasMaxAge()) return squid_curtime; } } diff --git a/src/base/StringArea.h b/src/base/StringArea.h index 719bd1f850..9463e14955 100644 --- a/src/base/StringArea.h +++ b/src/base/StringArea.h @@ -55,9 +55,6 @@ class StringArea { 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 diff --git a/src/http.cc b/src/http.cc index 7d7504c0a7..a0c0592228 100644 --- a/src/http.cc +++ b/src/http.cc @@ -928,7 +928,7 @@ no_cache: if (!ignoreCacheControl && rep->cache_control) { if (rep->cache_control->proxyRevalidate() || rep->cache_control->mustRevalidate() || - rep->cache_control->haveSMaxAge() + rep->cache_control->hasSMaxAge() ) EBIT_SET(entry->flags, ENTRY_REVALIDATE); } @@ -1771,7 +1771,7 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request, #endif /* Add max-age only without no-cache */ - if (cc->haveMaxAge() && !cc->noCache()) { + if (cc->hasMaxAge() && !cc->noCache()) { const char *url = entry ? entry->url() : urlCanonical(request); cc->maxAge(getMaxAge(url)); diff --git a/src/refresh.cc b/src/refresh.cc index 7141364a5e..154b66b928 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->haveMinFresh()) { + if (cc && cc->hasMinFresh()) { const int32_t minFresh=cc->minFresh(); 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->haveStaleIfError() && + entry->mem_obj->getReply()->cache_control->hasStaleIfError() && entry->mem_obj->getReply()->cache_control->staleIfError() < staleness) { debugs(22, 3, "refreshCheck: stale-if-error period expired."); @@ -336,7 +336,7 @@ refreshCheck(const StoreEntry * entry, HttpRequest * request, time_t delta) #endif if (NULL != cc) { - if (cc->haveMaxAge()) { + if (cc->hasMaxAge()) { #if USE_HTTP_VIOLATIONS if (R->flags.ignore_reload && cc->maxAge() == 0) { debugs(22, 3, "refreshCheck: MAYBE: client-max-age = 0 and ignore-reload"); @@ -355,8 +355,8 @@ refreshCheck(const StoreEntry * entry, HttpRequest * request, time_t delta) } } - if (cc->haveMaxStale() && staleness > -1) { - if (cc->maxStale()==HttpHdrCc::MAX_STALE_ALWAYS) { + if (cc->hasMaxStale() && staleness > -1) { + if (cc->maxStale()==HttpHdrCc::MAX_STALE_ANY) { /* max-stale directive without a value */ debugs(22, 3, "refreshCheck: NO: max-stale wildcard"); return FRESH_REQUEST_MAX_STALE_ALL;