This is an archive of the discontinued Mercurial Phabricator instance.

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

Authored by quark on Nov 21 2017, 2:14 PM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
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 useful for developers. So this BC looks fine.

.. bc::

Certain global flags (`--debugger`, `--profile`, and `--traceback`) are
no longer effective after `--`.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

quark created this revision.Nov 21 2017, 2:14 PM
quark updated this revision to Diff 3740.Nov 21 2017, 2:14 PM
quark edited the summary of this revision. (Show Details)Nov 21 2017, 2:18 PM
quark edited the summary of this revision. (Show Details)Nov 21 2017, 2:20 PM
dlax added a subscriber: dlax.Nov 21 2017, 3:00 PM
dlax added inline comments.
mercurial/dispatch.py
716

I find the algorithm a bit clumsy (usage of both in and .index() for the same value), how about a for loop like that:

for arg in args:
    if arg == optname:
        return True
    if arg == '--':
        return False
return False
quark added inline comments.Nov 21 2017, 3:27 PM
mercurial/dispatch.py
716

Good advice! Will change.

quark updated this revision to Diff 3744.Nov 21 2017, 3:30 PM
quark edited the summary of this revision. (Show Details)Nov 21 2017, 5:21 PM
lothiraldan added inline comments.
mercurial/dispatch.py
707

Just for the sake of testing, could you add a doctest with --debugger before --?

yuja requested changes to this revision.Nov 22 2017, 8:11 AM
yuja added a subscriber: yuja.

A similar patch is already in stable. Can you send a follow-up if you found
a problem?

https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-November/107567.html

This revision now requires changes to proceed.Nov 22 2017, 8:11 AM
quark abandoned this revision.Nov 22 2017, 12:28 PM

I didn't notice it's fixed in stable. Thanks!