]> granicus.if.org Git - apache/commitdiff
mod_cache: Fix parsing of quoted Cache-Control token arguments. PR 63288.
authorYann Ylavic <ylavic@apache.org>
Thu, 28 Mar 2019 16:39:39 +0000 (16:39 +0000)
committerYann Ylavic <ylavic@apache.org>
Thu, 28 Mar 2019 16:39:39 +0000 (16:39 +0000)
Make cache_strqtok() return both the token and its unquoted argument (if any),
or an error if the parsing fails.

Cache-Control integer values (max-age, max-stale, ...) can then be parsed w/o
taking care of the (optional) quoting.

Suggested by: fielding

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1856493 13f79535-47bb-0310-9956-ffa450edef68

CHANGES
modules/cache/cache_storage.c
modules/cache/cache_util.c
modules/cache/cache_util.h

diff --git a/CHANGES b/CHANGES
index 314fa2bfd64d885d96fcf6e5082edb6c06df98b9..7816a26051f1851bae24248d95fd6b8b1b571ca7 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.1
 
+  *) mod_cache: Fix parsing of quoted Cache-Control token arguments.
+     PR 63288. [Yann Ylavic]
+
   *) mod_md: Store permissions are enforced on file creation, enforcing restrictions in
      spite of umask. Fixes <https://github.com/icing/mod_md/issues/117>. [Stefan Eissing]
      
index dfda34b1c1d01a16d4a14c311db3b121b3e8eb94..d5998e756e2902e56b4fb00972e4d61b1942fa9d 100644 (file)
@@ -274,13 +274,15 @@ int cache_select(cache_request_rec *cache, request_rec *r)
              *
              * RFC2616 13.6 and 14.44 describe the Vary mechanism.
              */
-            vary = cache_strqtok(
-                    apr_pstrdup(r->pool,
-                            cache_table_getm(r->pool, h->resp_hdrs, "Vary")),
-                    CACHE_SEPARATOR, &last);
-            while (vary) {
+            for (rv = cache_strqtok(apr_pstrdup(r->pool,
+                                                cache_table_getm(r->pool,
+                                                                 h->resp_hdrs,
+                                                                 "Vary")),
+                                    &vary, NULL, &last);
+                 rv == APR_SUCCESS;
+                 rv = cache_strqtok(NULL, &vary, NULL, &last))
+            {
                 const char *h1, *h2;
-
                 /*
                  * is this header in the request and the header in the cached
                  * request identical? If not, we give up and do a straight get
@@ -300,7 +302,6 @@ int cache_select(cache_request_rec *cache, request_rec *r)
                     mismatch = 1;
                     break;
                 }
-                vary = cache_strqtok(NULL, CACHE_SEPARATOR, &last);
             }
 
             /* no vary match, try next provider */
index fc8f2a604cf25c88a34bebdac73b8c6e5bc05514..2c5a30734aad48b20db336b931ad946ad52a4514 100644 (file)
@@ -19,6 +19,8 @@
 #include "cache_util.h"
 #include <ap_provider.h>
 
+#include "test_char.h"
+
 APLOG_USE_MODULE(cache);
 
 /* -------------------------------------------------------------- */
@@ -923,75 +925,84 @@ CACHE_DECLARE(char *)ap_cache_generate_name(apr_pool_t *p, int dirlevels,
 }
 
 /**
- * String tokenizer that ignores separator characters within quoted strings
- * and escaped characters, as per RFC2616 section 2.2.
+ * String tokenizer per RFC 7234 section 5.2 (1#token[=["]arg["]]).
+ * If any (and arg not NULL), the argument is also returned (unquoted).
  */
-char *cache_strqtok(char *str, const char *sep, char **last)
+apr_status_t cache_strqtok(char *str, char **token, char **arg, char **last)
 {
-    char *token;
+#define CACHE_TOKEN_SEPS "\t ,"
     int quoted = 0;
+    char *wpos;
 
     if (!str) {         /* subsequent call */
         str = *last;    /* start where we left off */
     }
-
     if (!str) {         /* no more tokens */
-        return NULL;
+        return APR_EOF;
     }
 
-    /* skip characters in sep (will terminate at '\0') */
-    while (*str && ap_strchr_c(sep, *str)) {
+    /* skip separators (will terminate at '\0') */
+    while (*str && TEST_CHAR(*str, T_HTTP_TOKEN_STOP)) {
+        if (!ap_strchr_c(CACHE_TOKEN_SEPS, *str)) {
+            return APR_EINVAL;
+        }
         ++str;
     }
-
     if (!*str) {        /* no more tokens */
-        return NULL;
+        return APR_EOF;
     }
 
-    token = str;
+    *token = str;
+    if (arg) {
+        *arg = NULL;
+    }
 
     /* 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
+     * on the way, handle quoted strings, and within
      * quoted strings, escaped characters.
      */
-    *last = token;
-    while (**last) {
+    for (wpos = str; *str; ++str) {
         if (!quoted) {
-            if (**last == '\"' && !ap_strchr_c(sep, '\"')) {
+            if (*str == '"') {
                 quoted = 1;
-                ++*last;
+                continue;
             }
-            else if (!ap_strchr_c(sep, **last)) {
-                ++*last;
+            if (arg && !*arg && *str == '=') {
+                *wpos++ = '\0';
+                *arg = wpos;
+                continue;
             }
-            else {
+            if (TEST_CHAR(*str, T_HTTP_TOKEN_STOP)) {
                 break;
             }
         }
         else {
-            if (**last == '\"') {
-                quoted = 0;
-                ++*last;
-            }
-            else if (**last == '\\') {
-                ++*last;
-                if (**last) {
-                    ++*last;
-                }
+            if (*str == '"') {
+                ++str;
+                break;
             }
-            else {
-                ++*last;
+            if (*str == '\\' && *(str + 1)) {
+                ++str;
             }
         }
+        *wpos++ = *str;
     }
+    *wpos = '\0';
 
-    if (**last) {
-        **last = '\0';
-        ++*last;
+    /* anything after should be trailing OWS or comma */
+    for (; *str; ++str) {
+        if (*str == ',') {
+            ++str;
+            break;
+        }
+        if (*str != '\t' && *str != ' ') {
+            return APR_EINVAL;
+        }
     }
+    *last = str;
 
-    return token;
+    return APR_SUCCESS;
 }
 
 /**
@@ -1003,6 +1014,7 @@ int ap_cache_control(request_rec *r, cache_control_t *cc,
         const char *cc_header, const char *pragma_header, apr_table_t *headers)
 {
     char *last;
+    apr_status_t rv;
 
     if (cc->parsed) {
         return cc->cache_control || cc->pragma;
@@ -1015,33 +1027,35 @@ int ap_cache_control(request_rec *r, cache_control_t *cc,
     cc->s_maxage_value = -1;
 
     if (pragma_header) {
-        char *header = apr_pstrdup(r->pool, pragma_header);
-        const char *token = cache_strqtok(header, CACHE_SEPARATOR, &last);
-        while (token) {
+        char *header = apr_pstrdup(r->pool, pragma_header), *token;
+        for (rv = cache_strqtok(header, &token, NULL, &last);
+             rv == APR_SUCCESS;
+             rv = cache_strqtok(NULL, &token, NULL, &last)) {
             if (!ap_cstr_casecmp(token, "no-cache")) {
                 cc->no_cache = 1;
             }
-            token = cache_strqtok(NULL, CACHE_SEPARATOR, &last);
         }
         cc->pragma = 1;
     }
 
     if (cc_header) {
-        char *endp;
-        apr_off_t offt;
-        char *header = apr_pstrdup(r->pool, cc_header);
-        const char *token = cache_strqtok(header, CACHE_SEPARATOR, &last);
-        while (token) {
+        char *header = apr_pstrdup(r->pool, cc_header), *token, *arg;
+        for (rv = cache_strqtok(header, &token, &arg, &last);
+             rv == APR_SUCCESS;
+             rv = cache_strqtok(NULL, &token, &arg, &last)) {
+            char *endp;
+            apr_off_t offt;
+
             switch (token[0]) {
             case 'n':
-            case 'N': {
-                if (!ap_cstr_casecmpn(token, "no-cache", 8)) {
-                    if (token[8] == '=') {
-                        cc->no_cache_header = 1;
-                    }
-                    else if (!token[8]) {
+            case 'N':
+                if (!ap_cstr_casecmp(token, "no-cache")) {
+                    if (!arg) {
                         cc->no_cache = 1;
                     }
+                    else {
+                        cc->no_cache_header = 1;
+                    }
                 }
                 else if (!ap_cstr_casecmp(token, "no-store")) {
                     cc->no_store = 1;
@@ -1050,13 +1064,12 @@ int ap_cache_control(request_rec *r, cache_control_t *cc,
                     cc->no_transform = 1;
                 }
                 break;
-            }
+
             case 'm':
-            case 'M': {
-                if (!ap_cstr_casecmpn(token, "max-age", 7)) {
-                    if (token[7] == '='
-                            && !apr_strtoff(&offt, token + 8, &endp, 10)
-                            && endp > token + 8 && !*endp) {
+            case 'M':
+                if (arg && !ap_cstr_casecmp(token, "max-age")) {
+                    if (!apr_strtoff(&offt, arg, &endp, 10)
+                            && endp > arg && !*endp) {
                         cc->max_age = 1;
                         cc->max_age_value = offt;
                     }
@@ -1064,67 +1077,62 @@ int ap_cache_control(request_rec *r, cache_control_t *cc,
                 else if (!ap_cstr_casecmp(token, "must-revalidate")) {
                     cc->must_revalidate = 1;
                 }
-                else if (!ap_cstr_casecmpn(token, "max-stale", 9)) {
-                    if (token[9] == '='
-                            && !apr_strtoff(&offt, token + 10, &endp, 10)
-                            && endp > token + 10 && !*endp) {
+                else if (!ap_cstr_casecmp(token, "max-stale")) {
+                    if (!arg) {
                         cc->max_stale = 1;
-                        cc->max_stale_value = offt;
+                        cc->max_stale_value = -1;
                     }
-                    else if (!token[9]) {
+                    else if (!apr_strtoff(&offt, arg, &endp, 10)
+                             && endp > arg && !*endp) {
                         cc->max_stale = 1;
-                        cc->max_stale_value = -1;
+                        cc->max_stale_value = offt;
                     }
                 }
-                else if (!ap_cstr_casecmpn(token, "min-fresh", 9)) {
-                    if (token[9] == '='
-                            && !apr_strtoff(&offt, token + 10, &endp, 10)
-                            && endp > token + 10 && !*endp) {
+                else if (arg && !ap_cstr_casecmp(token, "min-fresh")) {
+                    if (!apr_strtoff(&offt, arg, &endp, 10)
+                            && endp > arg && !*endp) {
                         cc->min_fresh = 1;
                         cc->min_fresh_value = offt;
                     }
                 }
                 break;
-            }
+
             case 'o':
-            case 'O': {
+            case 'O':
                 if (!ap_cstr_casecmp(token, "only-if-cached")) {
                     cc->only_if_cached = 1;
                 }
                 break;
-            }
+
             case 'p':
-            case 'P': {
+            case 'P':
                 if (!ap_cstr_casecmp(token, "public")) {
                     cc->public = 1;
                 }
-                else if (!ap_cstr_casecmpn(token, "private", 7)) {
-                    if (token[7] == '=') {
-                        cc->private_header = 1;
-                    }
-                    else if (!token[7]) {
+                else if (!ap_cstr_casecmp(token, "private")) {
+                    if (!arg) {
                         cc->private = 1;
                     }
+                    else {
+                        cc->private_header = 1;
+                    }
                 }
                 else if (!ap_cstr_casecmp(token, "proxy-revalidate")) {
                     cc->proxy_revalidate = 1;
                 }
                 break;
-            }
+
             case 's':
-            case 'S': {
-                if (!ap_cstr_casecmpn(token, "s-maxage", 8)) {
-                    if (token[8] == '='
-                            && !apr_strtoff(&offt, token + 9, &endp, 10)
-                            && endp > token + 9 && !*endp) {
+            case 'S':
+                if (arg && !ap_cstr_casecmp(token, "s-maxage")) {
+                    if (!apr_strtoff(&offt, arg, &endp, 10)
+                            && endp > arg && !*endp) {
                         cc->s_maxage = 1;
                         cc->s_maxage_value = offt;
                     }
                 }
                 break;
             }
-            }
-            token = cache_strqtok(NULL, CACHE_SEPARATOR, &last);
         }
         cc->cache_control = 1;
     }
@@ -1139,48 +1147,44 @@ int ap_cache_control(request_rec *r, cache_control_t *cc,
 static int cache_control_remove(request_rec *r, const char *cc_header,
         apr_table_t *headers)
 {
-    char *last, *slast;
+    char *last, *slast, *sheader;
     int found = 0;
 
     if (cc_header) {
-        char *header = apr_pstrdup(r->pool, cc_header);
-        char *token = cache_strqtok(header, CACHE_SEPARATOR, &last);
-        while (token) {
+        apr_status_t rv;
+        char *header = apr_pstrdup(r->pool, cc_header), *token, *arg;
+        for (rv = cache_strqtok(header, &token, &arg, &last);
+             rv == APR_SUCCESS;
+             rv = cache_strqtok(NULL, &token, &arg, &last)) {
+            if (!arg) {
+                continue;
+            }
+
             switch (token[0]) {
             case 'n':
-            case 'N': {
-                if (!ap_cstr_casecmpn(token, "no-cache", 8)) {
-                    if (token[8] == '=') {
-                        const char *header = cache_strqtok(token + 9,
-                                CACHE_SEPARATOR "\"", &slast);
-                        while (header) {
-                            apr_table_unset(headers, header);
-                            header = cache_strqtok(NULL, CACHE_SEPARATOR "\"",
-                                    &slast);
-                        }
-                        found = 1;
+            case 'N':
+                if (!ap_cstr_casecmp(token, "no-cache")) {
+                    for (rv = cache_strqtok(arg, &sheader, NULL, &slast);
+                         rv == APR_SUCCESS;
+                         rv = cache_strqtok(NULL, &sheader, NULL, &slast)) {
+                        apr_table_unset(headers, sheader);
                     }
+                    found = 1;
                 }
                 break;
-            }
+
             case 'p':
-            case 'P': {
-                if (!ap_cstr_casecmpn(token, "private", 7)) {
-                    if (token[7] == '=') {
-                        const char *header = cache_strqtok(token + 8,
-                                CACHE_SEPARATOR "\"", &slast);
-                        while (header) {
-                            apr_table_unset(headers, header);
-                            header = cache_strqtok(NULL, CACHE_SEPARATOR "\"",
-                                    &slast);
-                        }
-                        found = 1;
+            case 'P':
+                if (!ap_cstr_casecmp(token, "private")) {
+                    for (rv = cache_strqtok(arg, &sheader, NULL, &slast);
+                         rv == APR_SUCCESS;
+                         rv = cache_strqtok(NULL, &sheader, NULL, &slast)) {
+                        apr_table_unset(headers, sheader);
                     }
+                    found = 1;
                 }
                 break;
             }
-            }
-            token = cache_strqtok(NULL, CACHE_SEPARATOR, &last);
         }
     }
 
index ae5a4c50ea87436352f612ee19f47a50eee21eab..41dca407a950e2515d5d82dc5fe49e2942ecb6ab 100644 (file)
@@ -99,7 +99,6 @@ extern "C" {
 #define CACHE_LOCKNAME_KEY "mod_cache-lockname"
 #define CACHE_LOCKFILE_KEY "mod_cache-lockfile"
 #define CACHE_CTX_KEY "mod_cache-ctx"
-#define CACHE_SEPARATOR ", \t"
 
 /**
  * cache_util.c
@@ -316,10 +315,10 @@ const char *cache_table_getm(apr_pool_t *p, const apr_table_t *t,
         const char *key);
 
 /**
- * String tokenizer that ignores separator characters within quoted strings
- * and escaped characters, as per RFC2616 section 2.2.
+ * String tokenizer per RFC 7234 section 5.2 (1#token[=["]arg["]]).
+ * If any (and arg not NULL), the argument is also returned (unquoted).
  */
-char *cache_strqtok(char *str, const char *sep, char **last);
+apr_status_t cache_strqtok(char *str, char **token, char **arg, char **last);
 
 /**
  * Merge err_headers_out into headers_out and add request's Content-Type and