On Wed, 2 May 2007, Niklas Edmundsson wrote:

We encountered the following bug: httpd segfaulted due to a client emitting "Cache-Control: max-age=216000, max-stale" which is a perfectly valid header.

The segfault is caused by the fact that ap_cache_liststr() sets the value pointer to NULL when there is no value, and this isn't checked at all in the cases when a value pointer is passed.

I think that this patch catches all those occurances.

Or so I thought.

It turned out that ap_cache_liststr() didn't set the value pointer to NULL in all cases where it should. Now it does.

I'm not proud of the solution for max-stale without value, but it should do the job...

It did, but it caused the freshness calculation to overflow so the end result was bollocks. I hard-coded 100 years for the max-stale without value case, not pretty but it works.

Updated patch attached.


/Nikke - not fond of fixing bugs with core-files as the only source of
         information :/
--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 Niklas Edmundsson, Admin @ {acc,hpc2n}.umu.se      |     [EMAIL PROTECTED]
---------------------------------------------------------------------------
 REJECTION: When your imaginary friends won't talk to you.
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
--- ../../../dist/modules/cache/cache_util.c    2006-10-13 01:11:33.000000000 
+0200
+++ cache_util.c        2007-05-02 14:01:06.000000000 +0200
@@ -243,7 +243,8 @@
     age = ap_cache_current_age(info, age_c, r->request_time);
 
     /* extract s-maxage */
-    if (cc_cresp && ap_cache_liststr(r->pool, cc_cresp, "s-maxage", &val)) {
+    if (cc_cresp && ap_cache_liststr(r->pool, cc_cresp, "s-maxage", &val)
+        && val != NULL) {
         smaxage = apr_atoi64(val);
     }
     else {
@@ -252,7 +253,8 @@
 
     /* extract max-age from request */
     if (!conf->ignorecachecontrol
-        && cc_req && ap_cache_liststr(r->pool, cc_req, "max-age", &val)) {
+        && cc_req && ap_cache_liststr(r->pool, cc_req, "max-age", &val)
+        && val != NULL) {
         maxage_req = apr_atoi64(val);
     }
     else {
@@ -260,7 +262,8 @@
     }
 
     /* extract max-age from response */
-    if (cc_cresp && ap_cache_liststr(r->pool, cc_cresp, "max-age", &val)) {
+    if (cc_cresp && ap_cache_liststr(r->pool, cc_cresp, "max-age", &val)
+        && val != NULL) {
         maxage_cresp = apr_atoi64(val);
     }
     else {
@@ -282,7 +285,16 @@
 
     /* extract max-stale */
     if (cc_req && ap_cache_liststr(r->pool, cc_req, "max-stale", &val)) {
-        maxstale = apr_atoi64(val);
+        if(val != NULL) {
+            maxstale = apr_atoi64(val);
+        }
+        else {
+            /* If no value is assigned to max-stale, then the client is willing
+             * to accept a stale response of any age */
+            /* Let's pretend 100 years is enough, need margin some marging here
+             * or the freshness calculation later will overflow */
+            maxstale = APR_INT64_C(86400*365*100);
+        }
     }
     else {
         maxstale = 0;
@@ -290,7 +302,8 @@
 
     /* extract min-fresh */
     if (!conf->ignorecachecontrol
-        && cc_req && ap_cache_liststr(r->pool, cc_req, "min-fresh", &val)) {
+        && cc_req && ap_cache_liststr(r->pool, cc_req, "min-fresh", &val)
+        && val != NULL) {
         minfresh = apr_atoi64(val);
     }
     else {
@@ -419,6 +432,9 @@
                                                   next - val_start);
                         }
                     }
+                    else {
+                        *val = NULL;
+                    }
                 }
                 return 1;
             }

Reply via email to