You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Michael W Thelen <mi...@pietdepsi.com> on 2005/08/09 18:47:52 UTC
[PATCH] svncopy.pl fails with spaces in file paths.
Michael W Thelen wrote:
> John Peacock wrote:
>>>> Would it be better to guard the parameters inside the SVNCall
>>>> subroutine itself, with something like this?
>>
>> Definitely the right idea; I *was* going to say wrong technique, but
>> it's really harmless to quote all of the options (under every shell
>> that I am aware of). If you wanted to be parsimonious about quoting
>> you could use something like this:
>
> Yep, I decided to quote all options for simplicity, and since we're
> depending on the shell to do the right thing with quotes anyway.
> Actually, I thought about quoting everything in @commandline, but I
> noticed that only @options depends on input from the user -- everything
> else is under the script's direct control.
>
> Either implementation should work fine. I think I prefer to quote
> everything since it is simpler code to read, though.
It looks like nothing ever happened with this patch... is it all right
if I commit this change? (I'm not sure how to assign credit for the
patch, review, etc. since Martin wrote the original patch and this one
is different but accomplishes the same goal.)
[[[
Surround command-line arguments with quotes when calling svn via the
shell, so arguments with spaces are handled correctly.
Patch by: Martin Tomes <li...@tomes.org>
mthelen
Review by: John Peacock <jp...@rowman.com>
* contrib/client-side/svncopy.pl.in
(SVNCall): Surround user-generated command-line options with quotes so
they are correctly interpreted by the shell.
]]]
==================================================================
--- contrib/client-side/svncopy.pl.in (revision 21885)
+++ contrib/client-side/svncopy.pl.in (local)
@@ -686,7 +686,7 @@
{
my ( $command, @options ) = @_;
- my @commandline = ( $svn, $command, @svn_options, @options );
+ my @commandline = ( $svn, $command, @svn_options, map {"\"$_\""}
@options );
info( " > ", join( " ", @commandline ), "\n" );
--
Michael W Thelen
It is a mistake to think you can solve any major problems just with
potatoes. -- Douglas Adams
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] svncopy.pl fails with spaces in file paths.
Posted by Charles Bailey <ba...@gmail.com>.
On 8/9/05, Michael W Thelen <mi...@pietdepsi.com> wrote:
> Michael W Thelen wrote:
> > Charles Bailey wrote:
> >
> >>On 8/9/05, Michael W Thelen <mi...@pietdepsi.com> wrote:
> >>
> >>
> >>>- my @commandline = ( $svn, $command, @svn_options, @options );
> >>>+ my @commandline = ( $svn, $command, @svn_options, map {"\"$_\""}
> >>>@options );
> >>
> >>Would you do better by using quotemeta() rather than plain quotes?
> >
> > I don't think so, since we're not trying to quote regular expression
> > special characters, and quotemeta doesn't escape spaces.
>
> Um, whoops. Apparently my Perl is a little rusty. Ignore that last
> message. :-) You may be right, quotemeta may be the better solution.
I can see two potential issues:
1. Using quotemeta makes a stronger assumption than just "" that the
command line interpreter will honor Bourne-shell-like quoting
(specifically, that \x reduces to x for all x not in [A-Za-z0-9]).
Since SVNCall already assumes Bourne-shell-like I/O redirection, I
don't think this is much of a problem, but it may trip over some
partially-Bourne-compatible foosh.
2. As a subproblem of 1, it assumes that \x reduces to x for any
locale-specific character x. Again, I don't think this is a problem
for bash and kin, but I don't have the experience of some other list
members with non-C locales. (The locale-friendly way to approach this
problem is s-(\W)-\\$1-g, but that's less secure against shell
metacharacters, of course.)
Overall, I think quotemeta works, unless there's a command line
interpreter with which I'm not acquainted that'll barf on \$ or
somesuch.
--
Regards,
Charles Bailey
Lists: bailey _dot_ charles _at_ gmail _dot_ com
Other: bailey _at_ newman _dot_ upenn _dot_ edu
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] svncopy.pl fails with spaces in file paths.
Posted by Michael W Thelen <mi...@pietdepsi.com>.
Michael W Thelen wrote:
> Charles Bailey wrote:
>
>>On 8/9/05, Michael W Thelen <mi...@pietdepsi.com> wrote:
>>
>>
>>>- my @commandline = ( $svn, $command, @svn_options, @options );
>>>+ my @commandline = ( $svn, $command, @svn_options, map {"\"$_\""}
>>>@options );
>>
>>Would you do better by using quotemeta() rather than plain quotes?
>
> I don't think so, since we're not trying to quote regular expression
> special characters, and quotemeta doesn't escape spaces.
Um, whoops. Apparently my Perl is a little rusty. Ignore that last
message. :-) You may be right, quotemeta may be the better solution.
--
Michael W Thelen
It is a mistake to think you can solve any major problems just with
potatoes. -- Douglas Adams
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] svncopy.pl fails with spaces in file paths.
Posted by Michael W Thelen <mi...@pietdepsi.com>.
Charles Bailey wrote:
> On 8/9/05, Michael W Thelen <mi...@pietdepsi.com> wrote:
>
>>- my @commandline = ( $svn, $command, @svn_options, @options );
>>+ my @commandline = ( $svn, $command, @svn_options, map {"\"$_\""}
>>@options );
>
> Would you do better by using quotemeta() rather than plain quotes?
I don't think so, since we're not trying to quote regular expression
special characters, and quotemeta doesn't escape spaces.
However, this brings up an interesting issue... the patch does not
address any special characters at all (like $ on Linux, % on Windows,
etc.). It also doesn't escape double quote characters themselves, if
they appear in the arguments. For now, this patch punts on those issues
(and maybe punting is good enough, I don't know).
--
Michael W Thelen
It is a mistake to think you can solve any major problems just with
potatoes. -- Douglas Adams
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] svncopy.pl fails with spaces in file paths.
Posted by Charles Bailey <ba...@gmail.com>.
On 8/9/05, Michael W Thelen <mi...@pietdepsi.com> wrote:
>
> - my @commandline = ( $svn, $command, @svn_options, @options );
> + my @commandline = ( $svn, $command, @svn_options, map {"\"$_\""}
> @options );
Would you do better by using quotemeta() rather than plain quotes?
--
Regards,
Charles Bailey
Lists: bailey _dot_ charles _at_ gmail _dot_ com
Other: bailey _at_ newman _dot_ upenn _dot_ edu
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] svncopy.pl fails with spaces in file paths.
Posted by Martin Tomes <li...@tomes.org>.
kfogel@collab.net wrote:
> Michael W Thelen <mi...@pietdepsi.com> writes:
>
>>It looks like nothing ever happened with this patch... is it all right
>>if I commit this change? (I'm not sure how to assign credit for the
>>patch, review, etc. since Martin wrote the original patch and this one
>>is different but accomplishes the same goal.)
>
> It looks good to me. Did you test that it works? (Don't know if it's
> better to use single quotes in order to prevent $-expansion... would
> someone ever *intend* for $-expansion to happen?)
If it is running on Windows (which is where the problem showed up in the
first place) then I think you have to quote with " and there is no
concept of $ expansion, it's %xxx%.
I think the pragmatic approach is to go with the patch as submitted by
Michael W Thelen.
--
Martin Tomes
echo 'martin at tomes x org x uk'\
| sed -e 's/ x /\./g' -e 's/ at /@/'
Visit http://www.subversionary.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] svncopy.pl fails with spaces in file paths.
Posted by kf...@collab.net.
Michael W Thelen <mi...@pietdepsi.com> writes:
> It looks like nothing ever happened with this patch... is it all right
> if I commit this change? (I'm not sure how to assign credit for the
> patch, review, etc. since Martin wrote the original patch and this one
> is different but accomplishes the same goal.)
It looks good to me. Did you test that it works? (Don't know if it's
better to use single quotes in order to prevent $-expansion... would
someone ever *intend* for $-expansion to happen?)
For crediting, I'd do
Patch by: mthelen
Suggested by: martinto
Review by: John Peacock <jp...@rowman.com>
in this circumstance.
> [[[
> Surround command-line arguments with quotes when calling svn via the
> shell, so arguments with spaces are handled correctly.
>
> Patch by: Martin Tomes <li...@tomes.org>
> mthelen
> Review by: John Peacock <jp...@rowman.com>
>
> * contrib/client-side/svncopy.pl.in
> (SVNCall): Surround user-generated command-line options with quotes so
> they are correctly interpreted by the shell.
> ]]]
>
> ==================================================================
> --- contrib/client-side/svncopy.pl.in (revision 21885)
> +++ contrib/client-side/svncopy.pl.in (local)
> @@ -686,7 +686,7 @@
> {
> my ( $command, @options ) = @_;
>
> - my @commandline = ( $svn, $command, @svn_options, @options );
> + my @commandline = ( $svn, $command, @svn_options, map {"\"$_\""}
> @options );
>
> info( " > ", join( " ", @commandline ), "\n" );
>
> --
> Michael W Thelen
> It is a mistake to think you can solve any major problems just with
> potatoes. -- Douglas Adams
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org