httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Paul J. Reder" <rede...@remulak.net>
Subject [Patch]: ap_cache_check_freshness 64 bit oddities
Date Fri, 11 Oct 2002 23:04:53 GMT
I have run into a problem where the cache code randomly decides that a
cached entry is stale or that the last modified date is some time in
the future. I tracked it back to the ap_cache_check_freshness code
which does a lot of checking of dates.

Some of this date checking code compares and assigns uncast numeric values
(such as -1) and the output of atoi() to the apr_time_t values (which are
long longs). The debugger showed me that only the bottom half of the
apr_time_t was being updated/compared.

I would like to fix the code in the following ways (if there are no
objections):
    1) Replace the assignments/comparisons of 0 with APR_DATE_BAD
    2) Have someone create a "#define APR_DATE_UNASSIGNED ((apr_time_t)-1)"
           value in apr_date.h.
    3) Replace the assignments/comparisons of -1 with APR_DATE_UNASSIGNED
    4) Replace the atoi() calls with calls to apr_atoi64().

Please see the attached patch.

With no objections (and a litle help from someone who has APR commit
authority) I would like to commit this this weekend.

Thanks,

-- 
Paul J. Reder
-----------------------------------------------------------
"The strength of the Constitution lies entirely in the determination of each
citizen to defend it.  Only if every single citizen feels duty bound to do
his share in this defense are the constitutional rights secure."
-- Albert Einstein







Index: httpd-2.0/modules/experimental/cache_util.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/experimental/cache_util.c,v
retrieving revision 1.18
diff -u -r1.18 cache_util.c
--- httpd-2.0/modules/experimental/cache_util.c 26 Aug 2002 16:41:56 -0000      1.18
+++ httpd-2.0/modules/experimental/cache_util.c 11 Oct 2002 22:56:14 -0000
@@ -162,7 +162,7 @@
      const char *cc_cresp, *cc_req, *pragma_cresp;
      const char *agestr = NULL;
      char *val;
-    apr_time_t age_c = 0;
+    apr_time_t age_c = APR_DATE_BAD;
      cache_info *info = &(cache->handle->cache_obj->info);

      /*
@@ -202,7 +202,7 @@
      /* TODO: pragma_cresp not being used? */
      pragma_cresp = apr_table_get(r->headers_out, "Pragma");
      if ((agestr = apr_table_get(r->headers_out, "Age"))) {
-        age_c = atoi(agestr);
+        age_c = apr_atoi64(agestr);
      }

      /* calculate age of object */
@@ -210,53 +210,53 @@

      /* extract s-maxage */
      if (cc_cresp && ap_cache_liststr(cc_cresp, "s-maxage", &val))
-        smaxage = atoi(val);
+        smaxage = apr_atoi64(val);
      else
-        smaxage = -1;
+        smaxage = APR_DATE_UNASSIGNED;

      /* extract max-age from request */
      if (cc_req && ap_cache_liststr(cc_req, "max-age", &val))
-        maxage_req = atoi(val);
+        maxage_req = apr_atoi64(val);
      else
-        maxage_req = -1;
+        maxage_req = APR_DATE_UNASSIGNED;

      /* extract max-age from response */
      if (cc_cresp && ap_cache_liststr(cc_cresp, "max-age", &val))
-        maxage_cresp = atoi(val);
+        maxage_cresp = apr_atoi64(val);
      else
-        maxage_cresp = -1;
+        maxage_cresp = APR_DATE_UNASSIGNED;

      /*
       * if both maxage request and response, the smaller one takes priority
       */
-    if (-1 == maxage_req)
+    if (APR_DATE_UNASSIGNED == maxage_req)
          maxage = maxage_cresp;
-    else if (-1 == maxage_cresp)
+    else if (APR_DATE_UNASSIGNED == maxage_cresp)
          maxage = maxage_req;
      else
          maxage = MIN(maxage_req, maxage_cresp);

      /* extract max-stale */
      if (cc_req && ap_cache_liststr(cc_req, "max-stale", &val))
-        maxstale = atoi(val);
+        maxstale = apr_atoi64(val);
      else
-        maxstale = 0;
+        maxstale = APR_DATE_BAD;

      /* extract min-fresh */
      if (cc_req && ap_cache_liststr(cc_req, "min-fresh", &val))
-        minfresh = atoi(val);
+        minfresh = apr_atoi64(val);
      else
-        minfresh = 0;
+        minfresh = APR_DATE_BAD;

      /* override maxstale if must-revalidate or proxy-revalidate */
-    if (maxstale && ((cc_cresp &&
+    if ((maxstale != APR_DATE_BAD) && ((cc_cresp &&
                        ap_cache_liststr(cc_cresp, "must-revalidate", NULL))
                       || (cc_cresp && ap_cache_liststr(cc_cresp,
                                                        "proxy-revalidate", NULL))))
-        maxstale = 0;
+        maxstale = APR_DATE_BAD;
      /* handle expiration */
-    if ((-1 < smaxage && age < (smaxage - minfresh)) ||
-        (-1 < maxage && age < (maxage + maxstale - minfresh)) ||
+    if ((APR_DATE_UNASSIGNED < smaxage && age < (smaxage - minfresh)) ||
+        (APR_DATE_UNASSIGNED < maxage && age < (maxage + maxstale - minfresh))
||
          (info->expire != APR_DATE_BAD && age < (info->expire - info->date
+ maxstale - minfresh))) {
          /* it's fresh darlings... */
          /* set age header on response */
@@ -264,8 +264,8 @@
                        apr_psprintf(r->pool, "%lu", (unsigned long)age));

          /* add warning if maxstale overrode freshness calculation */
-        if (!((-1 < smaxage && age < smaxage) ||
-              (-1 < maxage && age < maxage) ||
+        if (!((APR_DATE_UNASSIGNED < smaxage && age < smaxage) ||
+              (APR_DATE_UNASSIGNED < maxage && age < maxage) ||
                (info->expire != APR_DATE_BAD && (info->expire - info->date)
> age))) {
              /* make sure we don't stomp on a previous warning */
              apr_table_merge(r->headers_out, "Warning", "110 Response is stale");
Index: httpd-2.0/srclib/apr-util/include/apr_date.h
===================================================================
RCS file: /home/cvspublic/apr-util/include/apr_date.h,v
retrieving revision 1.8
diff -u -r1.8 apr_date.h
--- httpd-2.0/srclib/apr-util/include/apr_date.h        24 May 2002 02:35:02 -0000      1.8
+++ httpd-2.0/srclib/apr-util/include/apr_date.h        11 Oct 2002 22:56:16 -0000
@@ -83,6 +83,7 @@


  #define APR_DATE_BAD ((apr_time_t)0)
+#define APR_DATE_UNASSIGNED ((apr_time_t)-1)

  /**
   * Compare a string to a mask


Mime
View raw message