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....@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.
>
