threads / patch / 24006

patchPut quotes around branch names to prevent special characters from being interpreted by the shell.

Subject: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.

## tl;dr

6 messages between Jun 4, 2010 and Jun 7, 2010. Diffs are folded; open one to read it.

replies: 5people: 5as markdown or json

Benjamin C Meyer· Jun 4, 2010, 15:53 UTC · lore

Perforce branch names with spaces (and other special characters) were causing issues.

Signed-off-by: Benjamin C Meyer <bmeyer@rim.com>
---
 contrib/fast-import/git-p4 |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Show changes to contrib/fast-import/git-p4 +1 −1

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index c1ea643..d42b865 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1228,7 +1228,7 @@ class P4Sync(Command):
         lostAndFoundBranches = set()
 
         for info in p4CmdList("branches"):
-            details = p4Cmd("branch -o %s" % info["branch"])
+            details = p4Cmd("branch -o \"%s\"" % info["branch"])
             viewIdx = 0
             while details.has_key("View%s" % viewIdx):
                 paths = details["View%s" % viewIdx].split(" ")

-- 
1.7.1

Jeff King· Jun 6, 2010, 21:55 UTC · re: Benjamin C Meyer · lore

Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.

On Fri, Jun 04, 2010 at 11:53:20AM -0400, Benjamin C Meyer wrote:

Show 5 quoted lines

> Perforce branch names with spaces (and other special characters) were
> causing issues.
> [...]
> -            details = p4Cmd("branch -o %s" % info["branch"])
> +            details = p4Cmd("branch -o \"%s\"" % info["branch"])

Won't this still fail if your branch name contains quotation marks or backslashes? Does python have some equivalent to perl's quotemeta to just shell-escape the problematic characters (or even a way of just using a straight list in p4Cmd and avoiding the shell altogether)?

I know those characters may not be common, but if we are going to quote, perhaps we should make it foolproof.

-Peff

Jay Soffian· Jun 7, 2010, 05:10 UTC · re: Jeff King · lore

Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.

On Sun, Jun 6, 2010 at 5:55 PM, Jeff King <peff@peff.net> wrote:

Show 11 quoted lines

> On Fri, Jun 04, 2010 at 11:53:20AM -0400, Benjamin C Meyer wrote:
>
>> Perforce branch names with spaces (and other special characters) were
>> causing issues.
>> [...]
>> -            details = p4Cmd("branch -o %s" % info["branch"])
>> +            details = p4Cmd("branch -o \"%s\"" % info["branch"])
>
> Won't this still fail if your branch name contains quotation marks or
> backslashes? Does python have some equivalent to perl's quotemeta to
> just shell-escape the problematic characters

from commands import mkarg mkarg(arg)

will enclose arg in single quotes if it contains none; else it will enclose arg in double quotes and backslash escape shell meta characters (any of \ $ " `).

BTW, quotemeta is technically intended for use with regular expressions, isn't it?

> (or even a way of just
> using a straight list in p4Cmd and avoiding the shell altogether)?

The subprocess module.

> I know those characters may not be common, but if we are going to quote,
> perhaps we should make it foolproof.

+1.

j.

Ævar Arnfjörð Bjarmason· Jun 7, 2010, 05:48 UTC · re: Jay Soffian · lore

Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.

On Mon, Jun 7, 2010 at 05:10, Jay Soffian <jaysoffian@gmail.com> wrote:

> BTW, quotemeta is technically intended for use with regular
> expressions, isn't it?

Yes, it's completely insecure to use it for shell interpolation.

In Perl it's best to use the list form of system() so that the command will escape things for you automatically.

Ian Ward Comfort· Jun 7, 2010, 06:01 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.

On 6 Jun 2010, at 10:48 PM, Ævar Arnfjörð Bjarmason wrote:

Show 9 quoted lines

> On Mon, Jun 7, 2010 at 05:10, Jay Soffian <jaysoffian@gmail.com>  
> wrote:
>> BTW, quotemeta is technically intended for use with regular  
>> expressions, isn't it?
>
> Yes, it's completely insecure to use it for shell interpolation.
>
> In Perl it's best to use the list form of system() so that the  
> command will escape things for you automatically.

Passing a list to system() actually ensures that Perl will call execvp directly, instead of looking for metacharacters and possibly invoking the system shell. (But this is getting a little OT; sorry.)

-- 
Ian Ward Comfort <icomfort@stanford.edu>
Systems Team Lead, Academic Computing Services, Stanford University

Jeff King· Jun 7, 2010, 06:01 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.

On Mon, Jun 07, 2010 at 05:48:11AM +0000, Ævar Arnfjörð Bjarmason wrote:

Show 5 quoted lines

> On Mon, Jun 7, 2010 at 05:10, Jay Soffian <jaysoffian@gmail.com> wrote:
> > BTW, quotemeta is technically intended for use with regular
> > expressions, isn't it?
> 
> Yes, it's completely insecure to use it for shell interpolation.

It's intended for use with regexps, but I don't think it is insecure for shell interpolation. According to perldoc, it quotes 'all characters not matching "/[A-Za-z_0-9]/"'. So it's excessive for shell quoting, but not insecure.

> In Perl it's best to use the list form of system() so that the command
> will escape things for you automatically.

Agreed, but it's not escaping things automatically. It simply skips the shell invocation entirely.

-Peff