You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2019/03/28 16:39:40 UTC
svn commit: r1856493 - in /httpd/httpd/trunk: CHANGES
modules/cache/cache_storage.c modules/cache/cache_util.c
modules/cache/cache_util.h
Author: ylavic
Date: Thu Mar 28 16:39:39 2019
New Revision: 1856493
URL: http://svn.apache.org/viewvc?rev=1856493&view=rev
Log:
mod_cache: Fix parsing of quoted Cache-Control token arguments. PR 63288.
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
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/cache/cache_storage.c
httpd/httpd/trunk/modules/cache/cache_util.c
httpd/httpd/trunk/modules/cache/cache_util.h
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1856493&r1=1856492&r2=1856493&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Thu Mar 28 16:39:39 2019
@@ -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]
Modified: httpd/httpd/trunk/modules/cache/cache_storage.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/cache_storage.c?rev=1856493&r1=1856492&r2=1856493&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/cache_storage.c (original)
+++ httpd/httpd/trunk/modules/cache/cache_storage.c Thu Mar 28 16:39:39 2019
@@ -274,13 +274,15 @@ int cache_select(cache_request_rec *cach
*
* 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 *cach
mismatch = 1;
break;
}
- vary = cache_strqtok(NULL, CACHE_SEPARATOR, &last);
}
/* no vary match, try next provider */
Modified: httpd/httpd/trunk/modules/cache/cache_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/cache_util.c?rev=1856493&r1=1856492&r2=1856493&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/cache_util.c (original)
+++ httpd/httpd/trunk/modules/cache/cache_util.c Thu Mar 28 16:39:39 2019
@@ -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_n
}
/**
- * 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, cac
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, cac
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, cac
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, cac
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, cac
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);
}
}
Modified: httpd/httpd/trunk/modules/cache/cache_util.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/cache_util.h?rev=1856493&r1=1856492&r2=1856493&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/cache_util.h (original)
+++ httpd/httpd/trunk/modules/cache/cache_util.h Thu Mar 28 16:39:39 2019
@@ -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
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
Re: svn commit: r1856493 - in /httpd/httpd/trunk: CHANGES
modules/cache/cache_storage.c modules/cache/cache_util.c
modules/cache/cache_util.h
Posted by Ruediger Pluem <rp...@apache.org>.
On 03/28/2019 10:24 PM, Ruediger Pluem wrote:
>
>
> On 03/28/2019 05:39 PM, ylavic@apache.org wrote:
>> Author: ylavic
>> Date: Thu Mar 28 16:39:39 2019
>> New Revision: 1856493
>>
>> URL: http://svn.apache.org/viewvc?rev=1856493&view=rev
>> Log:
>> mod_cache: Fix parsing of quoted Cache-Control token arguments. PR 63288.
>>
>> 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
>>
>> Modified:
>> httpd/httpd/trunk/CHANGES
>> httpd/httpd/trunk/modules/cache/cache_storage.c
>> httpd/httpd/trunk/modules/cache/cache_util.c
>> httpd/httpd/trunk/modules/cache/cache_util.h
>>
>
>> Modified: httpd/httpd/trunk/modules/cache/cache_util.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/cache_util.c?rev=1856493&r1=1856492&r2=1856493&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/cache/cache_util.c (original)
>> +++ httpd/httpd/trunk/modules/cache/cache_util.c Thu Mar 28 16:39:39 2019
>
>> @@ -923,75 +925,84 @@ CACHE_DECLARE(char *)ap_cache_generate_n
>> }
>>
>> /**
>> - * 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 == '"') {
>
> Question: Is the token allowed to the quoted?
Answering myself: The token is not allowed to be quoted.
Regards
Rüdiger
Re: svn commit: r1856493 - in /httpd/httpd/trunk: CHANGES
modules/cache/cache_storage.c modules/cache/cache_util.c modules/cache/cache_util.h
Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Mar 28, 2019 at 11:40 PM Yann Ylavic <yl...@gmail.com> wrote:
>
> On Thu, Mar 28, 2019 at 10:30 PM Ruediger Pluem <rp...@apache.org> wrote:
> >
> >
> >
> > On 03/28/2019 10:27 PM, Yann Ylavic wrote:
> > > On Thu, Mar 28, 2019 at 10:24 PM Ruediger Pluem <rp...@apache.org> wrote:
> > >>
> > >> On 03/28/2019 05:39 PM, ylavic@apache.org wrote:
> > >>>
> > >>> + for (wpos = str; *str; ++str) {
> > >>> if (!quoted) {
> > >>> - if (**last == '\"' && !ap_strchr_c(sep, '\"')) {
> > >>> + if (*str == '"') {
> > >>
> > >> Question: Is the token allowed to the quoted?
> > >
> > > Hmm no, I asked and was told to RTFM, and then forgot :)
> > > Will fix, thanks!
> > >
> >
> > All good. It is not allowed to be quoted and then the code behaves correctly and returns with APR_EINVAL.
>
> Not really, 'Cache-Control: "private"' for instance is not allowed but
> accepted here.
> Hopefully fixed in r1856507.
Oh, you were right actually, the quoted token was already caught by
the first "skip separators" checks.
But I find the new code from r1856507 clearer anyway, so I'll leave it
(unless I'm the only one)..
Regards,
Yann.
Re: svn commit: r1856493 - in /httpd/httpd/trunk: CHANGES
modules/cache/cache_storage.c modules/cache/cache_util.c modules/cache/cache_util.h
Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Mar 28, 2019 at 10:30 PM Ruediger Pluem <rp...@apache.org> wrote:
>
>
>
> On 03/28/2019 10:27 PM, Yann Ylavic wrote:
> > On Thu, Mar 28, 2019 at 10:24 PM Ruediger Pluem <rp...@apache.org> wrote:
> >>
> >> On 03/28/2019 05:39 PM, ylavic@apache.org wrote:
> >>>
> >>> + for (wpos = str; *str; ++str) {
> >>> if (!quoted) {
> >>> - if (**last == '\"' && !ap_strchr_c(sep, '\"')) {
> >>> + if (*str == '"') {
> >>
> >> Question: Is the token allowed to the quoted?
> >
> > Hmm no, I asked and was told to RTFM, and then forgot :)
> > Will fix, thanks!
> >
>
> All good. It is not allowed to be quoted and then the code behaves correctly and returns with APR_EINVAL.
Not really, 'Cache-Control: "private"' for instance is not allowed but
accepted here.
Hopefully fixed in r1856507.
Thanks,
Yann.
Re: svn commit: r1856493 - in /httpd/httpd/trunk: CHANGES
modules/cache/cache_storage.c modules/cache/cache_util.c
modules/cache/cache_util.h
Posted by Ruediger Pluem <rp...@apache.org>.
On 03/28/2019 10:27 PM, Yann Ylavic wrote:
> On Thu, Mar 28, 2019 at 10:24 PM Ruediger Pluem <rp...@apache.org> wrote:
>>
>> On 03/28/2019 05:39 PM, ylavic@apache.org wrote:
>>>
>>> + for (wpos = str; *str; ++str) {
>>> if (!quoted) {
>>> - if (**last == '\"' && !ap_strchr_c(sep, '\"')) {
>>> + if (*str == '"') {
>>
>> Question: Is the token allowed to the quoted?
>
> Hmm no, I asked and was told to RTFM, and then forgot :)
> Will fix, thanks!
>
All good. It is not allowed to be quoted and then the code behaves correctly and returns with APR_EINVAL.
Regards
Rüdiger
Re: svn commit: r1856493 - in /httpd/httpd/trunk: CHANGES
modules/cache/cache_storage.c modules/cache/cache_util.c modules/cache/cache_util.h
Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Mar 28, 2019 at 10:24 PM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 03/28/2019 05:39 PM, ylavic@apache.org wrote:
> >
> > + for (wpos = str; *str; ++str) {
> > if (!quoted) {
> > - if (**last == '\"' && !ap_strchr_c(sep, '\"')) {
> > + if (*str == '"') {
>
> Question: Is the token allowed to the quoted?
Hmm no, I asked and was told to RTFM, and then forgot :)
Will fix, thanks!
Regards,
Yann.
Re: svn commit: r1856493 - in /httpd/httpd/trunk: CHANGES
modules/cache/cache_storage.c modules/cache/cache_util.c
modules/cache/cache_util.h
Posted by Ruediger Pluem <rp...@apache.org>.
On 03/28/2019 05:39 PM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Thu Mar 28 16:39:39 2019
> New Revision: 1856493
>
> URL: http://svn.apache.org/viewvc?rev=1856493&view=rev
> Log:
> mod_cache: Fix parsing of quoted Cache-Control token arguments. PR 63288.
>
> 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
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/cache/cache_storage.c
> httpd/httpd/trunk/modules/cache/cache_util.c
> httpd/httpd/trunk/modules/cache/cache_util.h
>
> Modified: httpd/httpd/trunk/modules/cache/cache_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/cache_util.c?rev=1856493&r1=1856492&r2=1856493&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/cache/cache_util.c (original)
> +++ httpd/httpd/trunk/modules/cache/cache_util.c Thu Mar 28 16:39:39 2019
> @@ -923,75 +925,84 @@ CACHE_DECLARE(char *)ap_cache_generate_n
> }
>
> /**
> - * 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 == '"') {
Question: Is the token allowed to the quoted?
Regards
Rüdiger