Mercurial > hg
changeset 35034:02845f7441af stable
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.
| author | Yuya Nishihara <yuya@tcha.org> |
|---|---|
| date | Sat, 11 Nov 2017 12:40:13 +0900 |
| parents | d3d35a55e03b |
| children | 96dcc78468e3 |
| files | mercurial/commands.py mercurial/dispatch.py tests/test-blackbox.t tests/test-dispatch.t tests/test-merge-subrepos.t tests/test-setdiscovery.t |
| diffstat | 6 files changed, 87 insertions(+), 13 deletions(-) [+] |
line wrap: on
line diff
--- a/mercurial/commands.py Sat Nov 11 17:55:15 2017 +0900 +++ b/mercurial/commands.py Sat Nov 11 12:40:13 2017 +0900 @@ -102,6 +102,10 @@ _("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
--- a/mercurial/dispatch.py Sat Nov 11 17:55:15 2017 +0900 +++ b/mercurial/dispatch.py Sat Nov 11 12:40:13 2017 +0900 @@ -264,7 +264,8 @@ # 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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!"))
--- a/tests/test-blackbox.t Sat Nov 11 17:55:15 2017 +0900 +++ b/tests/test-blackbox.t Sat Nov 11 12:40:13 2017 +0900 @@ -19,7 +19,7 @@ 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
--- a/tests/test-dispatch.t Sat Nov 11 17:55:15 2017 +0900 +++ b/tests/test-dispatch.t Sat Nov 11 12:40:13 2017 +0900 @@ -57,6 +57,62 @@ 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
--- a/tests/test-merge-subrepos.t Sat Nov 11 17:55:15 2017 +0900 +++ b/tests/test-merge-subrepos.t Sat Nov 11 12:40:13 2017 +0900 @@ -61,7 +61,7 @@ 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
--- a/tests/test-setdiscovery.t Sat Nov 11 17:55:15 2017 +0900 +++ b/tests/test-setdiscovery.t Sat Nov 11 12:40:13 2017 +0900 @@ -406,8 +406,8 @@ 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 ..