You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2011/02/12 14:08:57 UTC

svn commit: r1070075 - in /httpd/httpd/trunk: CHANGES modules/cache/cache_util.c

Author: minfrin
Date: Sat Feb 12 13:08:57 2011
New Revision: 1070075

URL: http://svn.apache.org/viewvc?rev=1070075&view=rev
Log:
mod_cache: We must ignore quoted-string values that appear in a 
Cache-Control header. PR 50199.

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/modules/cache/cache_util.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1070075&r1=1070074&r2=1070075&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Sat Feb 12 13:08:57 2011
@@ -2,6 +2,9 @@
 
 Changes with Apache 2.3.11
 
+  *) mod_cache: We must ignore quoted-string values that appear in a
+     Cache-Control header. PR 50199. [Graham Leggett]
+
   *) mod_dav: Revert change to send 501 error if unknown Content-* header is
     received for a PUT request. PR 42978. [Stefan Fritsch]
 

Modified: httpd/httpd/trunk/modules/cache/cache_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/cache_util.c?rev=1070075&r1=1070074&r2=1070075&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/cache_util.c (original)
+++ httpd/httpd/trunk/modules/cache/cache_util.c Sat Feb 12 13:08:57 2011
@@ -27,6 +27,8 @@ extern APR_OPTIONAL_FN_TYPE(ap_cache_gen
 
 extern module AP_MODULE_DECLARE_DATA cache_module;
 
+#define CACHE_SEPARATOR ",   "
+
 /* Determine if "url" matches the hostname, scheme and port and path
  * in "filter". All but the path comparisons are case-insensitive.
  */
@@ -1022,6 +1024,74 @@ CACHE_DECLARE(apr_table_t *)ap_cache_cac
 }
 
 /**
+ * String tokenizer that ignores separator characters within quoted strings
+ * and escaped characters, as per RFC2616 section 2.2.
+ */
+static char *cache_strqtok(char *str, const char *sep, char **last)
+{
+    char *token;
+    int quoted = 0;
+
+    if (!str) {         /* subsequent call */
+        str = *last;    /* start where we left off */
+    }
+
+    /* skip characters in sep (will terminate at '\0') */
+    while (*str && strchr(sep, *str)) {
+        ++str;
+    }
+
+    if (!*str) {        /* no more tokens */
+        return NULL;
+    }
+
+    token = str;
+
+    /* skip valid token characters to terminate token and
+     * prepare for the next call (will terminate at '\0)
+     * on the way, ignore all quoted strings, and within
+     * quoted strings, escaped characters.
+     */
+    *last = token + 1;
+    while (**last) {
+        if (!quoted) {
+            if (**last == '\"') {
+                quoted = 1;
+                ++*last;
+            }
+            else if (!strchr(sep, **last)) {
+                ++*last;
+            }
+            else {
+                break;
+            }
+        }
+        else {
+            if (**last == '\"') {
+                quoted = 0;
+                ++*last;
+            }
+            else if (**last == '\\') {
+                ++*last;
+                if (**last) {
+                    ++*last;
+                }
+            }
+            else {
+                ++*last;
+            }
+        }
+    }
+
+    if (**last) {
+        **last = '\0';
+        ++*last;
+    }
+
+    return token;
+}
+
+/**
  * Parse the Cache-Control and Pragma headers in one go, marking
  * which tokens appear within the header. Populate the structure
  * passed in.
@@ -1043,7 +1113,7 @@ int ap_cache_control(request_rec *r, cac
 
     if (pragma_header) {
         char *header = apr_pstrdup(r->pool, pragma_header);
-        const char *token = apr_strtok(header, ", ", &last);
+        const char *token = cache_strqtok(header, CACHE_SEPARATOR, &last);
         while (token) {
             /* handle most common quickest case... */
             if (!strcmp(token, "no-cache")) {
@@ -1053,14 +1123,14 @@ int ap_cache_control(request_rec *r, cac
             else if (!strcasecmp(token, "no-cache")) {
                 cc->no_cache = 1;
             }
-            token = apr_strtok(NULL, ", ", &last);
+            token = cache_strqtok(NULL, CACHE_SEPARATOR, &last);
         }
         cc->pragma = 1;
     }
 
     if (cc_header) {
         char *header = apr_pstrdup(r->pool, cc_header);
-        const char *token = apr_strtok(header, ", ", &last);
+        const char *token = cache_strqtok(header, CACHE_SEPARATOR, &last);
         while (token) {
             switch (token[0]) {
             case 'n':
@@ -1178,7 +1248,7 @@ int ap_cache_control(request_rec *r, cac
                 break;
             }
             }
-            token = apr_strtok(NULL, ", ", &last);
+            token = cache_strqtok(NULL, CACHE_SEPARATOR, &last);
         }
         cc->cache_control = 1;
     }



Re: svn commit: r1070075 - in /httpd/httpd/trunk: CHANGES modules/cache/cache_util.c

Posted by Graham Leggett <mi...@sharp.fm>.
On 14 Feb 2011, at 9:22 AM, Ruediger Pluem wrote:

>>> What happens if str is supplied as "a, b"?
>>> I mean why token + 1 and not token?
>>
>> I guess it's because we know *token isn't a separator, so there is no
>> point checking if it is one a second time.
>
> *token might not be a separator, but it might be ".

Sorry - I thought you were highlighting the comma and space, rather  
than the quotes around it.

You're right, the +1 does skip an opening quote, which is wrong. It's  
fixed in r1070699.

Regards,
Graham
--


Re: svn commit: r1070075 - in /httpd/httpd/trunk: CHANGES modules/cache/cache_util.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 02/13/2011 11:18 PM, Graham Leggett wrote:
> On 13 Feb 2011, at 5:22 PM, Ruediger Pluem wrote:
> 
>>> +    /* skip characters in sep (will terminate at '\0') */
>>> +    while (*str && strchr(sep, *str)) {
>>> +        ++str;
>>> +    }
>>> +
>>> +    if (!*str) {        /* no more tokens */
>>> +        return NULL;
>>> +    }
>>> +
>>> +    token = str;
>>> +
>>> +    /* skip valid token characters to terminate token and
>>> +     * prepare for the next call (will terminate at '\0)
>>> +     * on the way, ignore all quoted strings, and within
>>> +     * quoted strings, escaped characters.
>>> +     */
>>> +    *last = token + 1;
>>
>> What happens if str is supplied as "a, b"?
>> I mean why token + 1 and not token?
> 
> I guess it's because we know *token isn't a separator, so there is no
> point checking if it is one a second time.

*token might not be a separator, but it might be ".

> 
> The same pattern exists in apr_strtok.c:
> 
> https://svn.apache.org/repos/asf/apr/apr/trunk/strings/apr_strtok.c

Maybe, but that doesn't mean that it is correct :-).

IMHO "a, b" would result in the two tokens

a
b"

which would be wrong. The result should be one token:

"a, b"


Regards

RĂ¼diger


Re: svn commit: r1070075 - in /httpd/httpd/trunk: CHANGES modules/cache/cache_util.c

Posted by Graham Leggett <mi...@sharp.fm>.
On 13 Feb 2011, at 5:22 PM, Ruediger Pluem wrote:

>> +    /* skip characters in sep (will terminate at '\0') */
>> +    while (*str && strchr(sep, *str)) {
>> +        ++str;
>> +    }
>> +
>> +    if (!*str) {        /* no more tokens */
>> +        return NULL;
>> +    }
>> +
>> +    token = str;
>> +
>> +    /* skip valid token characters to terminate token and
>> +     * prepare for the next call (will terminate at '\0)
>> +     * on the way, ignore all quoted strings, and within
>> +     * quoted strings, escaped characters.
>> +     */
>> +    *last = token + 1;
>
> What happens if str is supplied as "a, b"?
> I mean why token + 1 and not token?

I guess it's because we know *token isn't a separator, so there is no  
point checking if it is one a second time.

The same pattern exists in apr_strtok.c:

https://svn.apache.org/repos/asf/apr/apr/trunk/strings/apr_strtok.c

Regards,
Graham
--


Re: svn commit: r1070075 - in /httpd/httpd/trunk: CHANGES modules/cache/cache_util.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 02/12/2011 02:08 PM, minfrin@apache.org wrote:
> Author: minfrin
> Date: Sat Feb 12 13:08:57 2011
> New Revision: 1070075
> 
> URL: http://svn.apache.org/viewvc?rev=1070075&view=rev
> Log:
> mod_cache: We must ignore quoted-string values that appear in a 
> Cache-Control header. PR 50199.
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/cache/cache_util.c
> 
> Modified: httpd/httpd/trunk/modules/cache/cache_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/cache_util.c?rev=1070075&r1=1070074&r2=1070075&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/cache/cache_util.c (original)
> +++ httpd/httpd/trunk/modules/cache/cache_util.c Sat Feb 12 13:08:57 2011
> @@ -1022,6 +1024,74 @@ CACHE_DECLARE(apr_table_t *)ap_cache_cac
>  }
>  
>  /**
> + * String tokenizer that ignores separator characters within quoted strings
> + * and escaped characters, as per RFC2616 section 2.2.
> + */
> +static char *cache_strqtok(char *str, const char *sep, char **last)
> +{
> +    char *token;
> +    int quoted = 0;
> +
> +    if (!str) {         /* subsequent call */
> +        str = *last;    /* start where we left off */
> +    }
> +
> +    /* skip characters in sep (will terminate at '\0') */
> +    while (*str && strchr(sep, *str)) {
> +        ++str;
> +    }
> +
> +    if (!*str) {        /* no more tokens */
> +        return NULL;
> +    }
> +
> +    token = str;
> +
> +    /* skip valid token characters to terminate token and
> +     * prepare for the next call (will terminate at '\0)
> +     * on the way, ignore all quoted strings, and within
> +     * quoted strings, escaped characters.
> +     */
> +    *last = token + 1;

What happens if str is supplied as "a, b"?
I mean why token + 1 and not token?

> +    while (**last) {
> +        if (!quoted) {
> +            if (**last == '\"') {
> +                quoted = 1;
> +                ++*last;
> +            }
> +            else if (!strchr(sep, **last)) {
> +                ++*last;
> +            }
> +            else {
> +                break;
> +            }
> +        }
> +        else {
> +            if (**last == '\"') {
> +                quoted = 0;
> +                ++*last;
> +            }
> +            else if (**last == '\\') {
> +                ++*last;
> +                if (**last) {
> +                    ++*last;
> +                }
> +            }
> +            else {
> +                ++*last;
> +            }
> +        }
> +    }
> +
> +    if (**last) {
> +        **last = '\0';
> +        ++*last;
> +    }
> +
> +    return token;
> +}
> +
> +/**
>   * Parse the Cache-Control and Pragma headers in one go, marking
>   * which tokens appear within the header. Populate the structure
>   * passed in.