Daniel Shahaf wrote on Mon, 17 Feb 2020 08:44 +0000:
> James McCoy wrote on Sun, 16 Feb 2020 21:27 -0500:
> > On Sun, Feb 16, 2020 at 10:06:49AM -0500, James McCoy wrote:
> > > 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.
> >
> > Done in r1874093.
>
> This approach is tightly coupled to apr_pescape_shell(), though: the escaping
> task is a single logical task but it's accomplished partly by one product
> (APR)
> and partly by another (Subversion). Is there a way to avoid this coupling?
Also, what if APR is changed so apr_pescape_shell("foo bar") starts to
return "foo bar" with two spaces? Presumably that would be correct
for the original use-case of apr_pescape_shell() (the one that resulted
in that function being created and implemented to _not_ escape
whitsespace), but it wouldn't be correct in our case.
> (Yes, this would have applied to the splitting idea I floated yesterday too.
> I didn't realize this then.)
