Re: svn commit: r1874057 - in /subversion/trunk/subversion: libsvn_subr/cmdline.c tests/cmdline/update_tests.py

On Sun, Feb 16, 2020 at 09:59:53AM +0000, Daniel Shahaf wrote:
> James McCoy wrote on Sat, 15 Feb 2020 13:10 -0500:
> > Well, that makes this more involved... I guess the simplest option is to
> > do our own scan of the string and pre-escape any whitespace before
> > having apr_pescape_shell() handle the rest.
>
> If we did that, apr_pescape_shell() would re-escape the backslashes or
> quotes we'd escape spaces with.

Yes, I realized this after hitting "Send". :)

> We could split on whitespace, apr_pescape_shell() each part, then join
> them back together.

What I have locally is escaping whitespace after calling
apr_pescape_shell(). This should work as long as they don't change
the semantics of this API to also handle whitespace.

However, the svn-x64-macosx-apr1.3-nothread buildbot informed me that
apr_pescape_shell() isn't available in that version. It wasn't added
until 1.5.0.

Now we need to either:

a) Raise the minimum APR version
b) Conditionally use the new code when built against an acceptable APR
   version and
   i) maintain the status quo for anyone using latest svn with an 8 year
      old version of APR or
   ii) add a minimal version of APR's escaping code that can be used
       when built again a pre-1.5.0 APR.

> We should probably propose to APR to add an API doing the kind of
> escaping we need here. We'll still have to carry our own implementation
> so long as we support building with versions of APR that don't have
> that API, of course.

Yeah, I'm not sure why whitespace (specifically, space and tab) were't
included in the original implementation. I can report this upstream and
see what they say.

Cheers,

-- 
James
GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB