From patchwork Mon Nov  3 08:14:22 2014
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: extdiff: quote user-supplied options passed to shell
From: Michael Fyles <mf@vorston.net>
X-Patchwork-Id: 6552
Message-Id: <bfc95af76f734996ece2.1415002462@dev0.mafconsulting.com>
To: mercurial-devel@selenic.com
Date: Mon, 03 Nov 2014 08:14:22 +0000

# HG changeset patch
# User Michael Fyles <mf@vorston.net>
# Date 1343299093 -3600
#      Thu Jul 26 11:38:13 2012 +0100
# Node ID bfc95af76f734996ece21bdded55c715c6a6c855
# Parent  f10019d2ee0afa2cc629a5e5409803e14fb40697
extdiff: quote user-supplied options passed to shell

$ hg extdiff -p cmd -o "name <user@example.com>"
resulted in a shell redirection error (due to the less-than sign),
rather than passing the single option to cmd. This was due to options
not being quoted for passing to the shell, via util.system(). Apply
util.shellquote() to each of the user-specified options (-o) to the
comparison program before they are concatenated and passed to
util.system(). The requested external diff command (-p) and the
files/directories being compared are already quoted correctly.

The discussion at the time of changeset be98c5ce4022 correctly noted
that this course of action breaks whitespace-separated options specified
for external diff commands in the configuration. The lower part of the
patch corrects this by lexing options read from the configuration file
into separate options rather than reading them all into the first
option.

Update test to cover these conditions.

Related changesets (reverse-chronological):
- be98c5ce4022 (fix reverted to make configuration file options work)
- 453097750fbf (issue fixed but without fix for configuration file)

diff -r f10019d2ee0a -r bfc95af76f73 hgext/extdiff.py
--- a/hgext/extdiff.py	Fri Oct 17 02:17:36 2014 -0700
+++ b/hgext/extdiff.py	Thu Jul 26 11:38:13 2012 +0100
@@ -121,7 +121,7 @@
 
     revs = opts.get('rev')
     change = opts.get('change')
-    args = ' '.join(diffopts)
+    args = ' '.join(map(util.shellquote, diffopts))
     do3way = '$parent2' in args
 
     if revs and change:
@@ -280,8 +280,7 @@
             cmd = cmd[4:]
             if not path:
                 path = cmd
-            diffopts = ui.config('extdiff', 'opts.' + cmd, '')
-            diffopts = diffopts and [diffopts] or []
+            diffopts = shlex.split(ui.config('extdiff', 'opts.' + cmd, ''))
         elif cmd.startswith('opts.'):
             continue
         else:
diff -r f10019d2ee0a -r bfc95af76f73 tests/test-extdiff.t
--- a/tests/test-extdiff.t	Fri Oct 17 02:17:36 2014 -0700
+++ b/tests/test-extdiff.t	Thu Jul 26 11:38:13 2012 +0100
@@ -19,6 +19,8 @@
   $ echo "[extdiff]" >> $HGRCPATH
   $ echo "cmd.falabala=echo" >> $HGRCPATH
   $ echo "opts.falabala=diffing" >> $HGRCPATH
+  $ echo "cmd.edspace=echo" >> $HGRCPATH
+  $ echo 'opts.edspace="name  <user@example.com>"' >> $HGRCPATH
 
   $ hg falabala
   diffing a.000000000000 a
@@ -168,6 +170,16 @@
   diffing this */extdiff.*/a.8a5febb7f867/a a.34eed99112ab/a (glob)
   [1]
 
+Test extdiff's handling of options with spaces in them:
+
+  $ hg edspace -c 1
+  name  <user@example.com> */extdiff.*/a.8a5febb7f867/a a.34eed99112ab/a (glob)
+  [1]
+
+  $ hg extdiff -p echo -o "name  <user@example.com>" -c 1
+  name  <user@example.com> */extdiff.*/a.8a5febb7f867/a a.34eed99112ab/a (glob)
+  [1]
+
 Test with revsets:
 
   $ hg extdif -p echo -c "rev(1)"