[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] svn --version --verbose += /etc/os-release

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Tue, 27 Jan 2015 11:24:04 +0000

Stefan Sperling wrote on Tue, Jan 27, 2015 at 11:39:26 +0100:
> On Tue, Jan 27, 2015 at 10:14:50AM +0000, Daniel Shahaf wrote:
> > +/* Attempt to strip double quotes from a string.
> > + *
> > + * The string considered is (STR->DATA + OFFSET). If it starts
> > + * and ends with double quotes, and has no escape sequences, return
> > + * the string delimited by the double quotes. Otherwise, return
> > + * the original string verbatim.
> > + */
> > +static const char *
> > +remove_shell_quoting(svn_stringbuf_t *buf,
> > + int offset,
> > + apr_pool_t *result_pool)
> > +{
> > + const char *str = buf->data + offset;
> > + const char *second_double_quote;
> > +
>
> Perhaps also trim leading and trailing whitespace before checking for quotes?
>

Okay.

> > + /* Are there exactly two double quotes, at the first and last positions? */
>
> What about single quotes? The spec allows single quotes:
> "Variable assignment values must be enclosed in double or single quotes if they
> include spaces, semicolons or other special characters outside of A-Z, a-z, 0-9"
>

Missed that, will fix.

> > + if (str[0] == '"' && (second_double_quote = strchr(str+1, '"'))
> > + && second_double_quote[1] == '\0')
> > + /* Are there any backslashes? */
> > + if (!strchr(str, '\\'))
>
> As I understand it backslashes used for escaping should be stripped so
> that e.g. \$ becomes $ and \\ becomes \.
>

I'm not completely sure.

The spec says:
         Shell special characters ("$", quotes, backslash, backtick)
         must be escaped with backslashes, following shell style.

To me, the following points are not answered by that specification:

- Is \ special inside single quotes, or only inside double quotes?
  [ Example: '\t' is two characters, "\t" is a tab in bash ]

- Is \ special when it is followed by something other than one of the
  five characters $"'`\\ ?
  [ Example: "\t" is two characters in dash, but a tab in bash]

Some shells also have $'...' style strings, but I don't think we need to
support those.

So, I agree with you that \\ and \$ should become backslash and dollar,
when enclosed by double quotes. But I am not sure how

        "\t"
        "\011"
        '\\'

should render.

> I'd suggest if we're going to go through the trouble of parsing quotes and
> escapes it should be done properly. Alternatively we could just always use
> the string verbatim.
>

That's what my first version of the patch did, but it resulted in this:

    System information:
    
    * running on x86_64-unknown-linux-gnu
      - "Debian GNU/Linux 7 (wheezy)" [Linux 3.2.0-4-amd64]

and I figured it was too inelegant, so I went back and added the
dquote-stripping...

Thanks for the review.

Daniel

P.S. I really like the "available authz backends" feature. I saw your
name is on it, so thanks :)
Received on 2015-01-27 12:26:44 CET

This is an archived mail posted to the Subversion Dev mailing list.