squid-1872

Version:

2.6.STABLE7 (fixed in 2.6.STABLE8)

Bug link:

http://bugs.squid-cache.org/show_bug.cgi?id=1872

How it is diagnosed (reproduced or source analysis)?

We did not reproduce the failure, but only analyzed the source code and discussion threads.

Symptom:

When a time stamp in the http response is one hour in the future, Squid would cache it even it is not cache-able. This is because Squid 2.6STABLE6 introduced a change in lib/rfc1123.c that causes an ‘Expires’ with a value one hour in the future to be assigned to responses that don't
contain one.

Root cause:

The root cause is such a time stamp would result ‘mktime’ library call to return error value (-1). However, squid did not handle the error correctly: Squid mistakenly set the ‘entry->expires’ to the current timestamp, which normally was only set when the HTTP header explicitly contains an Expires header. This will cause squid to think the object is cacheable anyway, thus cached the entry.

This is a case where error condition was not well handled and the error manifested into mysterious failure.

Note in ‘refreshStaleness’:

static int refreshStaleness(const StoreEntry * entry, time_t check_time, ...)

{

   /*

    * Check for an explicit expiration time.

    */

   if (entry->expires > -1) {

         /* Prepare to cache the object! */

   }

   ...

}

Since within the http header, ‘Expire’ was not set, so ‘entry->expire’ should not have a value ‘>-1’.

But in the buggy execution, ‘expires’ was set to a value > -1 (in fact, current time), because of the bug in ‘tmSaneValues’. Here is what happened:

static int tmSaneValues(struct tm *tm) {

   if (tm->tm_sec < 0 || tm->tm_sec > 59)

        return 0;

   if (tm->tm_min < 0 || tm->tm_min > 59)

        return 0;

   if (tm->tm_hour < 0 || tm->tm_hour > 23)

        return 0;

   if (tm->tm_mday < 1 || tm->tm_mday > 31)

        return 0;

   if (tm->tm_mon < 0 || tm->tm_mon > 11)

        return 0;

--  return mktime(tm) != -1;

++  return 1;

   // mktime in this case failed and returned -1, causing tmSancValue to return 0.

}

parse_date_elements(const char *day, const char *month, const char *year,... {

   ...

   return tmSaneValues (&tm) ? &tm : NULL;

   // tmSaneValues returned 0, causing parse_date_elements to return NULL!

}

tm* parse_date(...) {

   tm = parse_date_elements(day, month, year, time, zone);

   return tm;

   // parse_date_elements returned NULL, causing parse_date return NULL!

}

time_t parse_rfc1123(const char *str, int len) {

   tm = parse_date(str, len);

   if (!tm)

        return -1;

   // parse_date returned NULL, causing parse_rfc1123 to return -1.

}

httpHeaderGetTime(const HttpHeader * hdr, http_hdr_type id) {

   ...

   if ((e = httpHeaderFindEntry(hdr, id))) {

        value = parse_rfc1123(strBuf(e->value), strLen(e->value));

        httpHeaderNoteParsedEntry(e->id, e->value, value < 0);

   }

   return value;

   // parse_rfc1123 returned -1, causing httpHeaderGetTime to return -1.

}

httpReplyHdrCacheInit(HttpReply * rep) {

   ...

   rep->date = httpHeaderGetTime(hdr, HDR_DATE);

   // httpHeaderGetTime to returned -1, so re->date is set to -1.

   // This will cause ‘httpReplyHdrExpirationTime’ below to

   // return ‘squid_curtime’, thus set ‘re->expires’ to ‘squid_curtime’, > -1!

   ...

   rep->expires = httpReplyHdrExpirationTime(rep);

}

time_t httpReplyHdrExpirationTime(const HttpReply * rep) {

   if (rep->cache_control) {

        /* rep->date == -1 */

           if (rep->date >= 0) {

           ...

        } else {

            if (rep->cache_control->s_maxage >= 0)

            /* returns squid_curtime, which is the current time! */

                return squid_curtime;

            if (rep->cache_control->max_age >= 0)

                return squid_curtime;

        }

   }

}

Is there Error Message?

No.

Can developer/Errlog anticipate the Error:

Yes. The error pattern is: libc call ‘mktime’ failed -- returned -1.