httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From William A Rowe Jr <wr...@rowe-clan.net>
Subject Re: ap_getword_conf() and badly quoted strings
Date Sat, 18 Apr 2015 03:34:02 GMT
I think in trunk we should properly bail if the same quote char does not
occur as termination.

I don't think we should second-guess the admin's intent.




On Fri, Apr 17, 2015 at 6:43 AM, Yann Ylavic <ylavic.dev@gmail.com> wrote:

> Hi,
>
> currently ap_getword_conf() considers a word is quoted when (and only
> when) it starts with a quote, regardless of this word ending (or not)
> with the same quote.
>
> That is, eg., ap_getword_conf("\"") == "" or
> ap_getword_conf("\"whatever \\\"badly\\\" quoted") == "whatever
> \"badly\" quoted".
>
> I wonder if it should not return the (first) word as-is in this case,
> hence including the leading quote and up to the first space (ie.
> restart "normal" parsing from the beginning of the given line):
>
> Index: server/util.c
> ===================================================================
> --- server/util.c    (revision 1674046)
> +++ server/util.c    (working copy)
> @@ -780,7 +780,7 @@ AP_DECLARE(char *) ap_getword_conf_nc(apr_pool_t *
>
>  AP_DECLARE(char *) ap_getword_conf(apr_pool_t *p, const char **line)
>  {
> -    const char *str = *line, *strend;
> +    const char *str = *line, *strend = NULL;
>      char *res;
>      char quote;
>
> @@ -803,12 +803,16 @@ AP_DECLARE(char *) ap_getword_conf(apr_pool_t *p,
>                  ++strend;
>              }
>          }
> -        res = substring_conf(p, str + 1, strend - str - 1, quote);
>
> -        if (*strend == quote)
> +        if (*strend) {
> +            res = substring_conf(p, str + 1, strend - str - 1, quote);
>              ++strend;
> +        }
> +        else {
> +            strend = NULL;
> +        }
>      }
> -    else {
> +    if (!strend) {
>          strend = str;
>          while (*strend && !apr_isspace(*strend))
>              ++strend;
> --
>
> With this, ap_getword_conf("\"") == "\"" and
> ap_getword_conf("\"whatever \\\"badly\\\" quoted") == "\"whatever" =>
> "\\\"badly\\\" => "quoted", which may raise syntax errors/typos the
> administrator could be interested in.
>
> Is that more of a fix or a compatibility issue?
>
> Regards,
> Yann.
>

Mime
View raw message