( )⚙ D3445 dispatch: validate return type from dispatch() return value

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3445

dispatch: validate return type from dispatch() return value
Needs RevisionPublic

Authored by indygreg on May 6 2018, 12:11 AM.

Details

Reviewers
admin
durin42
Group Reviewers
hg-reviewers
Summary

This commit adds type checking to the result of dispatch() so
we catch non-None and non-int values. This prevents crashes
when we later try to "& 255" this value.

The Python 3 test failures introduced previously have all gone away.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.May 6 2018, 12:11 AM
yuja added a subscriber: yuja.May 6 2018, 8:29 AM

IIRC, SystemExit is caught at callcatch and translated to a status code.

+ # The logic here attempts to mimic how CPython's pythonrun.c does things.
+ # Essentially:
+ #
+ # * Exit is controlled by returning a value or raising SystemExit.
+ # * An integer value exits with that exit code.
+ # * A value of `None is interpreted as 0`.
+ # * A non-integer, non-None value results in that value being printed
+ # and an exit code of `1`.
+ # * SystemExit can have a `code` attribute containing the exit value.

try:
  • status = (dispatch(req) or 0)

+ code = dispatch(req) or 0

except error.StdioError as e:
    err = e
  • status = -1

+ code = -1
+ except SystemExit as err:
+ try:
+ code = err.code
+ err = None
+ except AttributeError:

indygreg edited the summary of this revision. (Show Details)May 12 2018, 12:48 AM
indygreg retitled this revision from dispatch: shore up exit handling to dispatch: validate return type from dispatch() return value.
indygreg updated this revision to Diff 8654.
admin requested changes to this revision.May 31 2018, 3:10 PM
admin added a subscriber: admin.

This looks good, but needs rebased.

This revision now requires changes to proceed.May 31 2018, 3:10 PM
durin42 requested changes to this revision.May 31 2018, 3:11 PM
durin42 added a subscriber: durin42.

(derp, that was me, but my password vault mis-fired and I got admin instead of me.)

Revision Contents
Changeset List

PathPackages
Mmercurial/dispatch.py (14 lines)
Mtests/test-dispatch.t (36 lines)

Diff 8654

mercurial/dispatch.py

def run(): def run():
"run the command in sys.argv" "run the command in sys.argv"
initstdio() initstdio()
req = request(pycompat.sysargv[1:]) req = request(pycompat.sysargv[1:])
err = None err = None
try: try:
status = dispatch(req) or 0 status = dispatch(req) or 0
# dispatch() should return an int or None. Python allows the
# value to sys.exit() to be special. We don't want to fall back to
# this handling, so we normalize to an int.
if not isinstance(status, int):
if util.safehasattr(req.ui, 'ferr'):
# If this raises, things are *really* messed up. We're
# OK with a possibly uncaught exception in this case
# because we're already in an exceptional code path.
req.ui.ferr.write('error: received non-integer status '
'code: %r\n' % status)
status = -1
except error.StdioError as e: except error.StdioError as e:
err = e err = e
status = -1 status = -1
# In all cases we try to flush stdio streams. # In all cases we try to flush stdio streams.
if util.safehasattr(req.ui, 'fout'): if util.safehasattr(req.ui, 'fout'):
try: try:
req.ui.fout.flush() req.ui.fout.flush()

tests/test-dispatch.t

> [extensions] > [extensions]
> exitmodes = $TESTTMP/exitmodes.py > exitmodes = $TESTTMP/exitmodes.py
> EOF > EOF
$ hg exit returnint 42 $ hg exit returnint 42
[42] [42]
$ hg exit returnstring 'some message' $ hg exit returnstring 'some message'
Traceback (most recent call last): error: received non-integer status code: 'some message'
File "*/hg", line *, in <module> (glob) [255]
dispatch.run()
File "*/mercurial/dispatch.py", line *, in run (glob)
sys.exit(status & 255)
TypeError: unsupported operand type(s) for &: 'str' and 'int'
[1]
$ hg exit returnnone $ hg exit returnnone
$ hg exit returnempty $ hg exit returnempty
$ hg exit returndict $ hg exit returndict
Traceback (most recent call last): error: received non-integer status code: {'key1': 'value1'}
File "*/hg", line *, in <module> (glob) [255]
dispatch.run()
File "*/mercurial/dispatch.py", line *, in run (glob)
sys.exit(status & 255)
TypeError: unsupported operand type(s) for &: 'dict' and 'int'
[1]
$ hg exit systemexitint 42 $ hg exit systemexitint 42
[42] [42]
$ hg exit systemexitstring 'failure message' $ hg exit systemexitstring 'failure message'
Traceback (most recent call last): error: received non-integer status code: 'failure message'
File "*/hg", line *, in <module> (glob) [255]
dispatch.run()
File "*/mercurial/dispatch.py", line *, in run (glob)
sys.exit(status & 255)
TypeError: unsupported operand type(s) for &: 'str' and 'int'
[1]
$ hg exit systemexitnoarg $ hg exit systemexitnoarg
$ hg exit systemexitnone $ hg exit systemexitnone
$ hg exit systemexitdict $ hg exit systemexitdict
Traceback (most recent call last): error: received non-integer status code: {'key1': 'value1'}
File "*/hg", line *, in <module> (glob) [255]
dispatch.run()
File "*/mercurial/dispatch.py", line *, in run (glob)
sys.exit(status & 255)
TypeError: unsupported operand type(s) for &: 'dict' and 'int'
[1]