[PATCH 3 of 5 STABLE RFC] dispatch: verify result of early command parsing

Yuya Nishihara yuya at tcha.org
Wed Nov 15 07:54:22 EST 2017


# HG changeset patch
# User Yuya Nishihara <yuya at tcha.org>
# Date 1510371613 -32400
#      Sat Nov 11 12:40:13 2017 +0900
# Branch stable
# Node ID 5838a6130d07b588e4640542865e3c175a124bff
# Parent  d0f8a7dc0ed66fd66f6df49a8ac6820dbc17c0ef
dispatch: verify result of early command parsing

Before, early options were stripped from args, and because of this, some
kind of parsing errors weren't reported. For example,

  $ hg ci -m -Ra file

would execute "hg ci -m file" in repository "a".

This patch fixes the issue by parsing early options again by real getopt-based
parser, and verifying the results. If the early parsing appears wrong, hg just
aborts. The current error message seems not nice, and should be improved, maybe
in V2 or follow-up.

Note that this isn't a security feature because we can still do anything by
using shell aliases.

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -102,6 +102,10 @@ globalopts = [
      _("when to paginate (boolean, always, auto, or never)"), _('TYPE')),
 ]
 
+# options which must be pre-parsed before loading configs and extensions
+# TODO: perhaps --debugger should be included
+earlyoptflags = ("--cwd", "-R", "--repository", "--repo", "--config")
+
 dryrunopts = cmdutil.dryrunopts
 remoteopts = cmdutil.remoteopts
 walkopts = cmdutil.walkopts
diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -264,7 +264,8 @@ def _runcatch(req):
 
             # read --config before doing anything else
             # (e.g. to change trust settings for reading .hg/hgrc)
-            cfgs = _parseconfig(req.ui, _earlygetopt(['--config'], req.args))
+            cfgs = _parseconfig(req.ui,
+                                _earlyreqopt(req, 'config', ['--config']))
 
             if req.repo:
                 # copy configs that were passed on the cmdline (--config) to
@@ -468,7 +469,7 @@ class cmdalias(object):
         self.cmdname = cmd = args.pop(0)
         self.givenargs = args
 
-        for invalidarg in ("--cwd", "-R", "--repository", "--repo", "--config"):
+        for invalidarg in commands.earlyoptflags:
             if _earlygetopt([invalidarg], args):
                 self.badalias = (_("error in definition for alias '%s': %s may "
                                    "only be given on the command line")
@@ -729,6 +730,18 @@ def _earlygetopt(aliases, args, strip=Tr
             pos += 1
     return values
 
+def _earlyreqopt(req, name, aliases):
+    """Peek a list option without using a full options table"""
+    values = _earlygetopt(aliases, req.args, strip=False)
+    req.earlyoptions[name] = values
+    return values
+
+def _earlyreqoptstr(req, name, aliases):
+    """Peek a string option without using a full options table"""
+    value = (_earlygetopt(aliases, req.args, strip=False) or [''])[-1]
+    req.earlyoptions[name] = value
+    return value
+
 def _earlyreqoptbool(req, name, aliases):
     """Peek a boolean option without using a full options table
 
@@ -819,6 +832,9 @@ def _checkshellalias(lui, ui, args):
     fn = entry[0]
 
     if cmd and util.safehasattr(fn, 'shell'):
+        # shell alias shouldn't receive early options which are consumed by hg
+        args = args[:]
+        _earlygetopt(commands.earlyoptflags, args, strip=True)
         d = lambda: fn(ui, *args[1:])
         return lambda: runcommand(lui, None, cmd, args[:1], ui, options, d,
                                   [], {})
@@ -828,13 +844,11 @@ def _dispatch(req):
     ui = req.ui
 
     # check for cwd
-    cwd = _earlygetopt(['--cwd'], args)
-    cwd = cwd and cwd[-1] or ''
+    cwd = _earlyreqoptstr(req, 'cwd', ['--cwd'])
     if cwd:
         os.chdir(cwd)
 
-    rpath = _earlygetopt(["-R", "--repository", "--repo"], args)
-    rpath = rpath and rpath[-1] or ''
+    rpath = _earlyreqoptstr(req, 'repository', ["-R", "--repository", "--repo"])
     path, lui = _getlocal(ui, rpath)
 
     uis = {ui, lui}
@@ -874,11 +888,11 @@ def _dispatch(req):
         fullargs = args
         cmd, func, args, options, cmdoptions = _parse(lui, args)
 
-        if options["config"]:
+        if options["config"] != req.earlyoptions["config"]:
             raise error.Abort(_("option --config may not be abbreviated!"))
-        if options["cwd"]:
+        if options["cwd"] != req.earlyoptions["cwd"]:
             raise error.Abort(_("option --cwd may not be abbreviated!"))
-        if options["repository"]:
+        if options["repository"] != req.earlyoptions["repository"]:
             raise error.Abort(_(
                 "option -R has to be separated from other options (e.g. not "
                 "-qR) and --repository may only be abbreviated as --repo!"))
diff --git a/tests/test-blackbox.t b/tests/test-blackbox.t
--- a/tests/test-blackbox.t
+++ b/tests/test-blackbox.t
@@ -19,7 +19,7 @@ command, exit codes, and duration
   1970/01/01 00:00:00 bob @0000000000000000000000000000000000000000 (5000)> init blackboxtest exited 0 after * seconds (glob)
   1970/01/01 00:00:00 bob @0000000000000000000000000000000000000000 (5000)> add a
   1970/01/01 00:00:00 bob @0000000000000000000000000000000000000000 (5000)> add a exited 0 after * seconds (glob)
-  1970/01/01 00:00:00 bob @0000000000000000000000000000000000000000+ (5000)> blackbox
+  1970/01/01 00:00:00 bob @0000000000000000000000000000000000000000+ (5000)> blackbox --config *blackbox.dirty=True* (glob)
 
 alias expansion is logged
   $ rm ./.hg/blackbox.log
diff --git a/tests/test-dispatch.t b/tests/test-dispatch.t
--- a/tests/test-dispatch.t
+++ b/tests/test-dispatch.t
@@ -57,6 +57,62 @@ Unparsable form of early options:
   abort: option --debugger may not be abbreviated!
   [255]
 
+Parsing failure of early options should be detected before executing the
+command:
+
+  $ hg log -b '--config=hooks.pre-log=false' default
+  abort: option --config may not be abbreviated!
+  [255]
+  $ hg log -b -R. default
+  abort: option -R has to be separated from other options (e.g. not -qR) and --repository may only be abbreviated as --repo!
+  [255]
+  $ hg log --cwd .. -b --cwd=. default
+  abort: option --cwd may not be abbreviated!
+  [255]
+
+However, we can't prevent it from loading extensions and configs:
+
+  $ cat <<EOF > bad.py
+  > raise Exception('bad')
+  > EOF
+  $ hg log -b '--config=extensions.bad=bad.py' default
+  *** failed to import extension bad from bad.py: bad
+  abort: option --config may not be abbreviated!
+  [255]
+
+  $ mkdir -p badrepo/.hg
+  $ echo 'invalid-syntax' > badrepo/.hg/hgrc
+  $ hg log -b -Rbadrepo default
+  hg: parse error at badrepo/.hg/hgrc:1: invalid-syntax
+  [255]
+
+  $ hg log -b --cwd=inexistent default
+  abort: No such file or directory: 'inexistent'
+  [255]
+
+  $ hg log -b '--config=ui.traceback=yes' 2>&1 | grep '^Traceback'
+  Traceback (most recent call last):
+  $ hg log -b '--config=profiling.enabled=yes' 2>&1 | grep -i sample
+  Sample count: .*|No samples recorded\. (re)
+
+Early options can't be specified in [aliases] and [defaults] because they are
+applied before the command name is resolved:
+
+  $ hg log -b '--config=alias.log=log --config=hooks.pre-log=false'
+  hg log: option -b not recognized
+  error in definition for alias 'log': --config may only be given on the command
+  line
+  [255]
+
+  $ hg log -b '--config=defaults.log=--config=hooks.pre-log=false'
+  abort: option --config may not be abbreviated!
+  [255]
+
+Shell aliases bypass any command parsing rules but for the early one:
+
+  $ hg log -b '--config=alias.log=!echo howdy'
+  howdy
+
 [defaults]
 
   $ hg cat a
diff --git a/tests/test-merge-subrepos.t b/tests/test-merge-subrepos.t
--- a/tests/test-merge-subrepos.t
+++ b/tests/test-merge-subrepos.t
@@ -61,7 +61,7 @@ A deleted subrepo file is flagged as dir
   9bfe45a197d7+ tip
   $ cat .hg/blackbox.log
   * @9bfe45a197d7b0ab09bf287729dd57e9619c9da5+ (*)> serve --cmdserver chgunix * (glob) (chg !)
-  * @9bfe45a197d7b0ab09bf287729dd57e9619c9da5+ (*)> id (glob)
+  * @9bfe45a197d7b0ab09bf287729dd57e9619c9da5+ (*)> id --config *extensions.blackbox=* --config *blackbox.dirty=True* (glob)
   * @9bfe45a197d7b0ab09bf287729dd57e9619c9da5+ (*)> id --config *extensions.blackbox=* --config *blackbox.dirty=True* exited 0 * (glob)
 
 TODO: a deleted file should be listed as such, like the top level repo
diff --git a/tests/test-setdiscovery.t b/tests/test-setdiscovery.t
--- a/tests/test-setdiscovery.t
+++ b/tests/test-setdiscovery.t
@@ -406,8 +406,8 @@ fixed in 86c35b7ae300:
   101 102 103 104 105 106 107 108 109 110  (no-eol)
   $ hg -R r1 --config extensions.blackbox= blackbox
   * @5d0b986a083e0d91f116de4691e2aaa54d5bbec0 (*)> serve --cmdserver chgunix * (glob) (chg !)
-  * @5d0b986a083e0d91f116de4691e2aaa54d5bbec0 (*)> outgoing r2 *-T{rev} * (glob)
+  * @5d0b986a083e0d91f116de4691e2aaa54d5bbec0 (*)> -R r1 outgoing r2 *-T{rev} * --config *extensions.blackbox=* (glob)
   * @5d0b986a083e0d91f116de4691e2aaa54d5bbec0 (*)> found 101 common and 1 unknown server heads, 2 roundtrips in *.????s (glob)
   * @5d0b986a083e0d91f116de4691e2aaa54d5bbec0 (*)> -R r1 outgoing r2 *-T{rev} * --config *extensions.blackbox=* exited 0 after *.?? seconds (glob)
-  * @5d0b986a083e0d91f116de4691e2aaa54d5bbec0 (*)> blackbox (glob)
+  * @5d0b986a083e0d91f116de4691e2aaa54d5bbec0 (*)> -R r1 --config *extensions.blackbox=* blackbox (glob)
   $ cd ..


More information about the Mercurial-devel mailing list