* [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.
@ 2010-06-04 15:53 Benjamin C Meyer
2010-06-06 21:55 ` Jeff King
0 siblings, 1 reply; 6+ messages in thread
From: Benjamin C Meyer @ 2010-06-04 15:53 UTC (permalink / raw)
To: git; +Cc: Benjamin C Meyer
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(-)
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.
2010-06-04 15:53 [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell Benjamin C Meyer
@ 2010-06-06 21:55 ` Jeff King
2010-06-07 5:10 ` Jay Soffian
0 siblings, 1 reply; 6+ messages in thread
From: Jeff King @ 2010-06-06 21:55 UTC (permalink / raw)
To: Benjamin C Meyer; +Cc: git
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.
2010-06-06 21:55 ` Jeff King
@ 2010-06-07 5:10 ` Jay Soffian
2010-06-07 5:48 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 6+ messages in thread
From: Jay Soffian @ 2010-06-07 5:10 UTC (permalink / raw)
To: Jeff King; +Cc: Benjamin C Meyer, git
On Sun, Jun 6, 2010 at 5:55 PM, Jeff King <peff@peff.net> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.
2010-06-07 5:10 ` Jay Soffian
@ 2010-06-07 5:48 ` Ævar Arnfjörð Bjarmason
2010-06-07 6:01 ` Ian Ward Comfort
2010-06-07 6:01 ` Jeff King
0 siblings, 2 replies; 6+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-06-07 5:48 UTC (permalink / raw)
To: Jay Soffian; +Cc: Jeff King, Benjamin C Meyer, git
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.
2010-06-07 5:48 ` Ævar Arnfjörð Bjarmason
@ 2010-06-07 6:01 ` Ian Ward Comfort
2010-06-07 6:01 ` Jeff King
1 sibling, 0 replies; 6+ messages in thread
From: Ian Ward Comfort @ 2010-06-07 6:01 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Jay Soffian, Jeff King, Benjamin C Meyer, git
On 6 Jun 2010, at 10:48 PM, Ævar Arnfjörð Bjarmason wrote:
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.
2010-06-07 5:48 ` Ævar Arnfjörð Bjarmason
2010-06-07 6:01 ` Ian Ward Comfort
@ 2010-06-07 6:01 ` Jeff King
1 sibling, 0 replies; 6+ messages in thread
From: Jeff King @ 2010-06-07 6:01 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Jay Soffian, Benjamin C Meyer, git
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 <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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-06-07 6:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-04 15:53 [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell Benjamin C Meyer
2010-06-06 21:55 ` Jeff King
2010-06-07 5:10 ` Jay Soffian
2010-06-07 5:48 ` Ævar Arnfjörð Bjarmason
2010-06-07 6:01 ` Ian Ward Comfort
2010-06-07 6:01 ` Jeff King
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).