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 4ec42a3582e7c991534c0349ae2527349b0e0ef5..d6c18e565c07daeda9540f9e6d5c971ec3783d7b 100644 (file)
--- a/src/HttpHdrCc.cc
+++ b/src/HttpHdrCc.cc
@@ -69,7 +69,7 @@ static HttpHeaderCcFields CcAttrs[CC_ENUM_END] = {
 typedef std::map<const StringArea,http_hdr_cc_type> 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 0c9011fb6cd51ad6bfe16c91d78a6944f6af91e2..1f239be1fa70e595428fedbb3473a4c2d43f65fb 100644 (file)
--- 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 6d4d2b41c8c88866e3d23cb79d0d15516f128f53..5774c693a15d56da3a2ae8a0566d126fcffc9e4f 100644 (file)
--- 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 1a1a910f748afe1238182cca1daf7cab346f3422..e41bddfae9ed91514c0a64e0801873a43ff20e26 100644 (file)
--- 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 719bd1f8501f1bd6eb7be11b0a9899790ef01603..9463e1495565cae4377daa6d2b4cea31a973883b 100644 (file)
--- 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 7d7504c0a79b19f8f051cbd6b89408804f4bfbee..a0c0592228b347c8499094e8233c8c32e2f2d891 100644 (file)
--- 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 7141364a5ecc0714035a383c69a24b1aed4c430b..154b66b9284d32ee72d68f2e87fe89805952a94a 100644 (file)
--- 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;