# [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell. 6 messages from 2010-06-04 to 2010-06-07. Participants: Benjamin C Meyer, Jeff King, Jay Soffian, Ævar Arnfjörð Bjarmason, Ian Ward Comfort. Thread: https://gitlist.dev/t/24006 ## Benjamin C Meyer, 2010-06-04 15:53 Subject: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell. Message-ID: <1275666800-31852-1-git-send-email-bmeyer@rim.com> URL: https://gitlist.dev/e/1275666800-31852-1-git-send-email-bmeyer%40rim.com ``` Perforce branch names with spaces (and other special characters) were causing issues. Signed-off-by: Benjamin C Meyer --- contrib/fast-import/git-p4 | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) 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, 2010-06-06 21:55 Subject: Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell. Message-ID: <20100606215505.GB6993@coredump.intra.peff.net> URL: https://gitlist.dev/e/20100606215505.GB6993%40coredump.intra.peff.net In-Reply-To: <1275666800-31852-1-git-send-email-bmeyer@rim.com> ``` 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 (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, 2010-06-07 05:10 Subject: Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell. Message-ID: URL: https://gitlist.dev/e/AANLkTinZc1jiBmTJRsJXDe7A4ZAe001zJIWILGKQ8YoA%40mail.gmail.com In-Reply-To: <20100606215505.GB6993@coredump.intra.peff.net> ``` On Sun, Jun 6, 2010 at 5:55 PM, Jeff King wrote: > 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, 2010-06-07 05:48 Subject: Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell. Message-ID: URL: https://gitlist.dev/e/AANLkTik7PV-2u24UF78U6rtuffw4XUGS1F4hD2_ElrZZ%40mail.gmail.com In-Reply-To: ``` On Mon, Jun 7, 2010 at 05:10, Jay Soffian 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, 2010-06-07 06:01 Subject: Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell. Message-ID: URL: https://gitlist.dev/e/EE9EE2D7-B15C-4858-A4C1-B045FD5AC6BA%40stanford.edu In-Reply-To: ``` On 6 Jun 2010, at 10:48 PM, Ævar Arnfjörð Bjarmason wrote: > On Mon, Jun 7, 2010 at 05:10, Jay Soffian > 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 Systems Team Lead, Academic Computing Services, Stanford University ``` ## Jeff King, 2010-06-07 06:01 Subject: Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell. Message-ID: <20100607060147.GA20958@coredump.intra.peff.net> URL: https://gitlist.dev/e/20100607060147.GA20958%40coredump.intra.peff.net In-Reply-To: ``` On Mon, Jun 07, 2010 at 05:48:11AM +0000, Ævar Arnfjörð Bjarmason wrote: > On Mon, Jun 7, 2010 at 05:10, Jay Soffian 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 ```