{"thread":{"id":"24006","subject":"[PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.","startedAt":"2010-06-04T15:53:20Z","lastAt":"2010-06-07T06:01:47Z","messageCount":6,"participants":["Benjamin C Meyer","Jeff King","Jay Soffian","Ævar Arnfjörð Bjarmason","Ian Ward Comfort"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"142989","messageId":"1275666800-31852-1-git-send-email-bmeyer@rim.com","threadId":"24006","inReplyTo":null,"subject":"[PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.","fromName":"Benjamin C Meyer","fromEmail":"bmeyer@rim.com","sentAt":"2010-06-04T15:53:20Z","receivedAt":"2010-06-04T15:53:20Z","isPatch":true,"sender":{"key":"bmeyer@rim.com","avatar":null},"body":"Perforce branch names with spaces (and other special characters) were causing issues.\n\nSigned-off-by: Benjamin C Meyer \n---\n contrib/fast-import/git-p4 | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex c1ea643..d42b865 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -1228,7 +1228,7 @@ class P4Sync(Command):\n lostAndFoundBranches = set()\n \n for info in p4CmdList(\"branches\"):\n- details = p4Cmd(\"branch -o %s\" % info[\"branch\"])\n+ details = p4Cmd(\"branch -o \\\"%s\\\"\" % info[\"branch\"])\n viewIdx = 0\n while details.has_key(\"View%s\" % viewIdx):\n paths = details[\"View%s\" % viewIdx].split(\" \")\n-- \n1.7.1\n"},{"id":"143111","messageId":"20100606215505.GB6993@coredump.intra.peff.net","threadId":"24006","inReplyTo":"1275666800-31852-1-git-send-email-bmeyer@rim.com","subject":"Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-06-06T21:55:05Z","receivedAt":"2010-06-06T21:55:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 04, 2010 at 11:53:20AM -0400, Benjamin C Meyer wrote:\n\n> Perforce branch names with spaces (and other special characters) were\n> causing issues.\n> [...]\n> - details = p4Cmd(\"branch -o %s\" % info[\"branch\"])\n> + details = p4Cmd(\"branch -o \\\"%s\\\"\" % info[\"branch\"])\n\nWon't this still fail if your branch name contains quotation marks or\nbackslashes? Does python have some equivalent to perl's quotemeta to\njust shell-escape the problematic characters (or even a way of just\nusing a straight list in p4Cmd and avoiding the shell altogether)?\n\nI know those characters may not be common, but if we are going to quote,\nperhaps we should make it foolproof.\n\n-Peff\n"},{"id":"143134","messageId":"AANLkTinZc1jiBmTJRsJXDe7A4ZAe001zJIWILGKQ8YoA@mail.gmail.com","threadId":"24006","inReplyTo":"20100606215505.GB6993@coredump.intra.peff.net","subject":"Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2010-06-07T05:10:32Z","receivedAt":"2010-06-07T05:10:32Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sun, Jun 6, 2010 at 5:55 PM, Jeff King wrote:\n> On Fri, Jun 04, 2010 at 11:53:20AM -0400, Benjamin C Meyer wrote:\n>\n>> Perforce branch names with spaces (and other special characters) were\n>> causing issues.\n>> [...]\n>> -            details = p4Cmd(\"branch -o %s\" % info[\"branch\"])\n>> +            details = p4Cmd(\"branch -o \\\"%s\\\"\" % info[\"branch\"])\n>\n> Won't this still fail if your branch name contains quotation marks or\n> backslashes? Does python have some equivalent to perl's quotemeta to\n> just shell-escape the problematic characters\n\nfrom commands import mkarg\nmkarg(arg)\n\nwill enclose arg in single quotes if it contains none; else it will\nenclose arg in double quotes and backslash escape shell meta\ncharacters (any of \\ $ \" `).\n\nBTW, quotemeta is technically intended for use with regular\nexpressions, isn't it?\n\n> (or even a way of just\n> using a straight list in p4Cmd and avoiding the shell altogether)?\n\nThe subprocess module.\n\n> I know those characters may not be common, but if we are going to quote,\n> perhaps we should make it foolproof.\n\n+1.\n\nj.\n"},{"id":"143136","messageId":"AANLkTik7PV-2u24UF78U6rtuffw4XUGS1F4hD2_ElrZZ@mail.gmail.com","threadId":"24006","inReplyTo":"AANLkTinZc1jiBmTJRsJXDe7A4ZAe001zJIWILGKQ8YoA@mail.gmail.com","subject":"Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-06-07T05:48:11Z","receivedAt":"2010-06-07T05:48:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Jun 7, 2010 at 05:10, Jay Soffian wrote:\n> BTW, quotemeta is technically intended for use with regular\n> expressions, isn't it?\n\nYes, it's completely insecure to use it for shell interpolation.\n\nIn Perl it's best to use the list form of system() so that the command\nwill escape things for you automatically.\n"},{"id":"143137","messageId":"EE9EE2D7-B15C-4858-A4C1-B045FD5AC6BA@stanford.edu","threadId":"24006","inReplyTo":"AANLkTik7PV-2u24UF78U6rtuffw4XUGS1F4hD2_ElrZZ@mail.gmail.com","subject":"Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.","fromName":"Ian Ward Comfort","fromEmail":"icomfort@stanford.edu","sentAt":"2010-06-07T06:01:00Z","receivedAt":"2010-06-07T06:01:00Z","isPatch":true,"sender":{"key":"icomfort@stanford.edu","avatar":"https://avatars.githubusercontent.com/u/202841?v=4"},"body":"On 6 Jun 2010, at 10:48 PM, Ævar Arnfjörð Bjarmason wrote:\n> On Mon, Jun 7, 2010 at 05:10, Jay Soffian \n> wrote:\n>> BTW, quotemeta is technically intended for use with regular \n>> expressions, isn't it?\n>\n> Yes, it's completely insecure to use it for shell interpolation.\n>\n> In Perl it's best to use the list form of system() so that the \n> command will escape things for you automatically.\n\nPassing a list to system() actually ensures that Perl will call execvp \ndirectly, instead of looking for metacharacters and possibly invoking \nthe system shell. (But this is getting a little OT; sorry.)\n\n-- \nIan Ward Comfort \nSystems Team Lead, Academic Computing Services, Stanford University\n"},{"id":"143138","messageId":"20100607060147.GA20958@coredump.intra.peff.net","threadId":"24006","inReplyTo":"AANLkTik7PV-2u24UF78U6rtuffw4XUGS1F4hD2_ElrZZ@mail.gmail.com","subject":"Re: [PATCH] Put quotes around branch names to prevent special characters from being interpreted by the shell.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-06-07T06:01:47Z","receivedAt":"2010-06-07T06:01:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 07, 2010 at 05:48:11AM +0000, Ævar Arnfjörð Bjarmason wrote:\n\n> On Mon, Jun 7, 2010 at 05:10, Jay Soffian wrote:\n> > BTW, quotemeta is technically intended for use with regular\n> > expressions, isn't it?\n> \n> Yes, it's completely insecure to use it for shell interpolation.\n\nIt's intended for use with regexps, but I don't think it is insecure for\nshell interpolation. According to perldoc, it quotes 'all characters not\nmatching \"/[A-Za-z_0-9]/\"'. So it's excessive for shell quoting, but not\ninsecure.\n\n> In Perl it's best to use the list form of system() so that the command\n> will escape things for you automatically.\n\nAgreed, but it's not escaping things automatically. It simply skips the\nshell invocation entirely.\n\n-Peff\n"}]}