D1483: globalopts: make special flags ineffective after '--' (BC)

[quark (Jun Wu)]

quark created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Flags after '--' should not be effective. That's a universal rule across
  common software. People should be able to hg log a file named `--debugger`.
  Besides, hg ... -- --profile --traceback` should not enable profiling or
  traceback.
  
  This patch changes the handling of `--debugger`, `--profile`, `--traceback`
  so they are ineffective after `--`. This makes other software easier to deal
  with command line flags injection by adding `--` before user input. See
  https://hackerone.com/reports/288704
  
  Those flags are only used for developers. So this BC looks fine.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1483

AFFECTED FILES
  mercurial/dispatch.py
  tests/test-globalopts.t

CHANGE DETAILS

diff --git a/tests/test-globalopts.t b/tests/test-globalopts.t
--- a/tests/test-globalopts.t
+++ b/tests/test-globalopts.t
@@ -459,3 +459,10 @@
 
 Not tested: --debugger
 
+Flags should not be effective after --:
+
+  $ hg add -- --debugger --profile --traceback
+  --debugger: No such file or directory
+  --profile: No such file or directory
+  --traceback: No such file or directory
+  [1]
diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -147,7 +147,7 @@
     try:
         if not req.ui:
             req.ui = uimod.ui.load()
-        if '--traceback' in req.args:
+        if _earlypeekboolopt('--traceback', req.args):
             req.ui.setconfig('ui', 'traceback', 'on', '--traceback')
 
         # set ui streams from the request
@@ -250,6 +250,7 @@
                     _('potentially unsafe serve --stdio invocation: %r') %
                     (req.args,))
 
+        usedebugger = _earlypeekboolopt('--debugger', req.args)
         try:
             debugger = 'pdb'
             debugtrace = {
@@ -263,6 +264,7 @@
             # (e.g. to change trust settings for reading .hg/hgrc)
             cfgs = _parseconfig(req.ui, _earlygetopt(['--config'], req.args))
 
+
             if req.repo:
                 # copy configs that were passed on the cmdline (--config) to
                 # the repo ui
@@ -275,7 +277,7 @@
             if not debugger or ui.plain():
                 # if we are in HGPLAIN mode, then disable custom debugging
                 debugger = 'pdb'
-            elif '--debugger' in req.args:
+            elif usedebugger:
                 # This import can be slow for fancy debuggers, so only
                 # do it when absolutely necessary, i.e. when actual
                 # debugging has been requested
@@ -289,7 +291,7 @@
             debugmortem[debugger] = debugmod.post_mortem
 
             # enter the debugger before command execution
-            if '--debugger' in req.args:
+            if usedebugger:
                 ui.warn(_("entering debugger - "
                         "type c to continue starting hg or h for help\n"))
 
@@ -305,7 +307,7 @@
                 ui.flush()
         except: # re-raises
             # enter the debugger when we hit an exception
-            if '--debugger' in req.args:
+            if usedebugger:
                 traceback.print_exc()
                 debugmortem[debugger](sys.exc_info()[2])
             raise
@@ -693,6 +695,26 @@
             pos += 1
     return values
 
+def _earlypeekboolopt(optname, args):
+    """Return True or False for given boolean flag. Do not modify args.
+
+    >>> args = [b'x', b'--debugger', b'y']
+    >>> _earlypeekboolopt(b'--debugger', args)
+    True
+
+    >>> args = [b'x', b'--', b'--debugger', b'y']
+    >>> _earlypeekboolopt(b'--debugger', args)
+    False
+    """
+    if optname in args:
+        pos = args.index(optname)
+        if '--' in args:
+            argcount = args.index('--')
+            return pos < argcount
+        else:
+            return True
+    return False
+
 def runcommand(lui, repo, cmd, fullargs, ui, options, d, cmdpats, cmdoptions):
     # run pre-hook, and abort if it fails
     hook.hook(lui, repo, "pre-%s" % cmd, True, args=" ".join(fullargs),
@@ -780,7 +802,7 @@
     if req.repo:
         uis.add(req.repo.ui)
 
-    if '--profile' in args:
+    if _earlypeekboolopt('--profile', args):
         for ui_ in uis:
             ui_.setconfig('profiling', 'enabled', 'true', '--profile')
 



To: quark, #hg-reviewers
Cc: mercurial-devel