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

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

From: James McCoy <jamessan_at_jamessan.com>
Date: Sat, 15 Feb 2020 13:10:32 -0500

On Sat, Feb 15, 2020 at 05:26:25PM +0000, Daniel Shahaf wrote:
> jamessan_at_apache.org wrote on Sat, 15 Feb 2020 16:24 -0000:
> > Escape filenames when invoking $SVN_EDITOR
> >
> > Per https://subversion.apache.org/faq.html#svn-editor, $SVN_EDITOR is invoked
> > through the shell instead of directly executed. The user is expected to
> > properly escape/quote $SVN_EDITOR, but svn was putting the filename directly
> > into the command without any escaping. This therefore breaks attempts to,
> > e.g., run the editor from the merge conflict dialog when a path has special
> > characters.
> >
> > Update locations where we invoke the editor to quote the filename as well as
> > escape shell special characters using apr_pescape_shell(). The quotes are
> > needed in addition to the escaping, since apr_pescape_shell() does not escape
> > whitespace.
>
> > +++ subversion/trunk/subversion/libsvn_subr/cmdline.c Sat Feb 15 16:24:53 2020
> > @@ -1330,7 +1331,10 @@ svn_cmdline__edit_file_externally(const
> > return svn_error_wrap_apr
> > (apr_err, _("Can't change working directory to '%s'"), base_dir);
> >
> > - cmd = apr_psprintf(pool, "%s %s", editor, file_name);
> > + /* editor is explicitly documented as being interpreted by the user's shell,
> > + and as such should already be quoted/escaped as needed. */
> > + cmd = apr_psprintf(pool, "%s \"%s\"", editor,
> > + apr_pescape_shell(pool, file_name));
>
> If FILE_NAME is «;» (i.e., a single semicolon), apr_pescape_shell()
> will return «\;» (two bytes) and then the command string will be «vi "\;"»,
> so vi would edit the file «\;» (two bytes).

Ah, indeed. I was thinking of C strings where backslash must always be
escaped to be literal. Shells only treat them as an escape for certain
characters.

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.

Cheers,

-- 
James
GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB
Received on 2020-02-15 19:10:43 CET

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.