Patchwork [4,of,4,V4] posix: quote the specified string only when it may have to be quoted

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Dec. 25, 2014, 2:46 p.m.
Message ID <1dd199e9c8129e0005cb.1419518777@juju>
Download mbox | patch
Permalink /patch/7235/
State Accepted
Commit 5edb387158a137f7c447c9bbccfc834c833a2de8
Headers show

Comments

Katsunori FUJIWARA - Dec. 25, 2014, 2:46 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1419518006 -32400
#      Thu Dec 25 23:33:26 2014 +0900
# Node ID 1dd199e9c8129e0005cb76fae65f2ee9200807ae
# Parent  f5178f9404e829694a1abd1a713f441dac7201e3
posix: quote the specified string only when it may have to be quoted

This patch makes "posix.shellquote" examine the specified string and
quote it only when it may have to be quoted for safety, like as the
previous patch for "windows.shellquote".

In fact, on POSIX environment, quoting itself doesn't cause issues
like issue4463. But (almost) equivalent quoting policy can avoid
examining test result differently on POSIX and Windows (even though
showing command line with "%r" causes such examination in
"test-extdiff.t").

The last hunk for "test-extdiff.t" in this patch isn't needed for the
previous patch for "windows.shellquote", because the code path of it
is executed only "#if execbit" (= avoided on Windows).
Matt Mackall - Dec. 29, 2014, 10:13 p.m.
On Thu, 2014-12-25 at 23:46 +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1419518006 -32400
> #      Thu Dec 25 23:33:26 2014 +0900
> # Node ID 1dd199e9c8129e0005cb76fae65f2ee9200807ae
> # Parent  f5178f9404e829694a1abd1a713f441dac7201e3
> posix: quote the specified string only when it may have to be quoted

These are queued for default, thanks.

Patch

diff --git a/mercurial/posix.py b/mercurial/posix.py
--- a/mercurial/posix.py
+++ b/mercurial/posix.py
@@ -8,7 +8,7 @@ 
 from i18n import _
 import encoding
 import os, sys, errno, stat, getpass, pwd, grp, socket, tempfile, unicodedata
-import fcntl
+import fcntl, re
 
 posixfile = open
 normpath = os.path.normpath
@@ -315,9 +315,16 @@  if sys.platform == 'cygwin':
     def checklink(path):
         return False
 
+_needsshellquote = None
 def shellquote(s):
     if os.sys.platform == 'OpenVMS':
         return '"%s"' % s
+    global _needsshellquote
+    if _needsshellquote is None:
+        _needsshellquote = re.compile(r'[^a-zA-Z0-9._/-]').search
+    if not _needsshellquote(s):
+        # "s" shouldn't have to be quoted
+        return s
     else:
         return "'%s'" % s.replace("'", "'\\''")
 
diff --git a/tests/test-extdiff.t b/tests/test-extdiff.t
--- a/tests/test-extdiff.t
+++ b/tests/test-extdiff.t
@@ -114,11 +114,11 @@  issue4463: usage of command line configu
   running '*echo* *\\a *\\a' in */extdiff.* (glob)
 #else
   $ hg --debug 4463a | grep '^running'
-  running '\'echo\' a-naked \'single quoted\' "double quoted" \'*/a\' \'$TESTTMP/a/a\'' in */extdiff.* (glob)
+  running 'echo a-naked \'single quoted\' "double quoted" */a $TESTTMP/a/a' in */extdiff.* (glob)
   $ hg --debug 4463b | grep '^running'
-  running 'echo b-naked \'single quoted\' "double quoted" \'*/a\' \'$TESTTMP/a/a\'' in */extdiff.* (glob)
+  running 'echo b-naked \'single quoted\' "double quoted" */a $TESTTMP/a/a' in */extdiff.* (glob)
   $ hg --debug echo | grep '^running'
-  running "'*echo*' '*/a' '$TESTTMP/a/a'" in */extdiff.* (glob)
+  running '*echo */a $TESTTMP/a/a' in */extdiff.* (glob)
 #endif
 
 (getting options from other than extdiff section)
@@ -149,15 +149,15 @@  issue4463: usage of command line configu
   running 'echo echo-naked "being quoted" *\\a *\\a' in */extdiff.* (glob)
 #else
   $ hg --debug 4463b2 | grep '^running'
-  running 'echo b2-naked \'single quoted\' "double quoted" \'*/a\' \'$TESTTMP/a/a\'' in */extdiff.* (glob)
+  running 'echo b2-naked \'single quoted\' "double quoted" */a $TESTTMP/a/a' in */extdiff.* (glob)
   $ hg --debug 4463b3 | grep '^running'
-  running 'echo b3-naked \'single quoted\' "double quoted" \'*/a\' \'$TESTTMP/a/a\'' in */extdiff.* (glob)
+  running 'echo b3-naked \'single quoted\' "double quoted" */a $TESTTMP/a/a' in */extdiff.* (glob)
   $ hg --debug 4463b4 | grep '^running'
-  running "echo '*/a' '$TESTTMP/a/a'" in */extdiff.* (glob)
-  $ hg --debug 4463b4 --option 'being quoted' | grep '^running'
-  running "echo 'being quoted' '*/a' '$TESTTMP/a/a'" in */extdiff.* (glob)
-  $ hg --debug extdiff -p echo --option 'being quoted' | grep '^running'
-  running "'echo' 'being quoted' '*/a' '$TESTTMP/a/a'" in */extdiff.* (glob)
+  running 'echo */a $TESTTMP/a/a' in */extdiff.* (glob)
+  $ hg --debug 4463b4 --option b4-naked --option 'being quoted' | grep '^running'
+  running "echo b4-naked 'being quoted' */a $TESTTMP/a/a" in */extdiff.* (glob)
+  $ hg --debug extdiff -p echo --option echo-naked --option 'being quoted' | grep '^running'
+  running "echo echo-naked 'being quoted' */a $TESTTMP/a/a" in */extdiff.* (glob)
 #endif
 
 #if execbit
@@ -273,7 +273,7 @@  Fallback to merge-tools.tool.executable|
   making snapshot of 2 files from working directory
     a
     b
-  running "'$TESTTMP/a/dir/tool.sh' 'a.*' 'a'" in */extdiff.* (glob)
+  running '$TESTTMP/a/dir/tool.sh a.* a' in */extdiff.* (glob)
   ** custom diff **
   cleaning up temp directory
   [1]