httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Fix mod_expires to set correct expires date (PR41391)
Date Wed, 27 Aug 2008 19:48:32 GMT
While reviewing the patch for PR41391 (https://issues.apache.org/bugzilla/show_bug.cgi?id=41391),
I noticed that besides a possible overflow, it could happen that the expires date is in the
past
(just take an old file and say modification plus 1 minute). What we really want to say in
this
situation is that the resource is already expired. To state this RF2616 14.21 says that the
Expires header should be set to the same value as the Date header.
The following patch fixes this and ensures that max-age will be zero in this situation:

Index: modules/metadata/mod_expires.c
===================================================================
--- modules/metadata/mod_expires.c      (Revision 689502)
+++ modules/metadata/mod_expires.c      (Arbeitskopie)
@@ -386,6 +386,8 @@
      return new;
  }

+#define SECS_PER_YEAR 86400 * 365
+
  /*
   * Handle the setting of the expiration response header fields according
   * to our criteria.
@@ -395,7 +397,6 @@
                                   apr_table_t *t)
  {
      apr_time_t base;
-    apr_time_t additional;
      apr_time_t expires;
      int additional_sec;
      char *timestr;
@@ -409,16 +410,12 @@
              return DECLINED;
          }
          base = r->finfo.mtime;
-        additional_sec = atoi(&code[1]);
-        additional = apr_time_from_sec(additional_sec);
          break;
      case 'A':
          /* there's been some discussion and it's possible that
           * 'access time' will be stored in request structure
           */
          base = r->request_time;
-        additional_sec = atoi(&code[1]);
-        additional = apr_time_from_sec(additional_sec);
          break;
      default:
          /* expecting the add_* routines to be case-hardened this
@@ -429,7 +426,31 @@
          return HTTP_INTERNAL_SERVER_ERROR;
      }

-    expires = base + additional;
+    additional_sec = atoi(&code[1]);
+    expires = base + apr_time_from_sec(additional_sec);
+    /*
+     * Check if we have an overflow, so the resource should not expire.
+     * According to RFC2616 14.21 we should set a date one year in the future
+     * then, but not more.
+     */
+    if (expires < 0) {
+        expires = r->request_time + apr_time_from_sec(SECS_PER_YEAR);
+    }
+    /*
+     * We had an overflow again. So we seem to be less then a year from
+     * APR_INT64_MAX
+     */
+    if (expires < 0) {
+        expires = APR_INT64_MAX;
+    }
+    /*
+     * Our expires date is in the past. According to RFC2616 14.21 we should
+     * set expires to the same value as the Date header to show that the
+     * resource is already expired.
+     */
+    if (expires < r->request_time) {
+        expires = r->request_time;
+    }
      apr_table_mergen(t, "Cache-Control",
                       apr_psprintf(r->pool, "max-age=%" APR_TIME_T_FMT,
                                    apr_time_sec(expires - r->request_time)));


The downside is that this patch breaks several tests. But I believe that this is caused by
a wrong
test. The following patch would fix this:

Index: t/modules/expires.t
===================================================================
--- t/modules/expires.t (Revision 689502)
+++ t/modules/expires.t (Arbeitskopie)
@@ -243,6 +243,17 @@
          $actual = $headers{expires} - $headers{access};
      }

+    ## if we use the modification date as base the calculated expiration date
+    ## might be past and the message that should be sent is that this resource
+    ## is already expired. According to RFC2616 14.21 the Date header and the
+    ## Expires header should be equal in this case.
+    ##
+    if (($exp_type eq 'M')
+        && ($headers{modified} + $expected < $headers{access})) {
+        $expected = 0;
+        $actual = $headers{expires} - $headers{access};
+    }
+
      print "# debug: expected: $expected\n";
      print "# debug: actual  : $actual\n";
      return ($actual == $expected);


Comments?

Regards

RĂ¼diger

Mime
View raw message