This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2383

sshpeer: defer pipe buffering and stderr sidechannel binding
ClosedPublic

Authored by indygreg on Feb 21 2018, 5:08 PM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
Commits
rHG11ba1a96f946: sshpeer: defer pipe buffering and stderr sidechannel binding
Summary

The doublepipe and bufferedinputpipe types facilitate polling
multiple pipes without blocking and for automatically forwarding
output from the SSH server's stderr pipe to the ui as "remote: "
output. This all happens automatically and callers don't need
to worry about reading from multiple pipes.

An upcoming change to version 2 of the SSH wire protocol will
eliminate the use of stderr and move side-channel output into
the "main" pipe. The SSH wire protocol will use a pair of
unidirectional pipes - just like the HTTP protocol. In this
future world, the doublepipe primitive isn't necessary because
the stderr pipe won't be used.

To prepare for eventually not using doublepipe, we delay the
construction of this primitive from immediately after
connection establishment to inside construction of the peer
instance. The handshake occurs between these two events. So
we had to teach the handshake code to read from stderr so
any stderr output from the server is still attended to early in
the connection lifetime.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

indygreg created this revision.Feb 21 2018, 5:08 PM
yuja accepted this revision.Feb 22 2018, 9:05 AM
This revision is now accepted and ready to land.Feb 22 2018, 9:05 AM
Closed by commit rHG11ba1a96f946: sshpeer: defer pipe buffering and stderr sidechannel binding (authored by indygreg). · Explain WhyFeb 22 2018, 9:18 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/sshpeer.py (15 lines)
Mtests/test-check-interfaces.py (12 lines)

Diff 6001

mercurial/sshpeer.py

ui.debug('running %s\n' % cmd) ui.debug('running %s\n' % cmd)
cmd = util.quotecommand(cmd) cmd = util.quotecommand(cmd)
# no buffer allow the use of 'select' # no buffer allow the use of 'select'
# feel free to remove buffering and select usage when we ultimately # feel free to remove buffering and select usage when we ultimately
# move to threading. # move to threading.
stdin, stdout, stderr, proc = util.popen4(cmd, bufsize=0, env=sshenv) stdin, stdout, stderr, proc = util.popen4(cmd, bufsize=0, env=sshenv)
stdout = doublepipe(ui, util.bufferedinputpipe(stdout), stderr)
stdin = doublepipe(ui, stdin, stderr)
return proc, stdin, stdout, stderr return proc, stdin, stdout, stderr
def _performhandshake(ui, stdin, stdout, stderr): def _performhandshake(ui, stdin, stdout, stderr):
def badresponse(): def badresponse():
# Flush any output on stderr.
_forwardoutput(ui, stderr)
msg = _('no suitable response from remote hg') msg = _('no suitable response from remote hg')
hint = ui.config('ui', 'ssherrorhint') hint = ui.config('ui', 'ssherrorhint')
raise error.RepoError(msg, hint=hint) raise error.RepoError(msg, hint=hint)
# The handshake consists of sending wire protocol commands in reverse # The handshake consists of sending wire protocol commands in reverse
# order of protocol implementation and then sniffing for a response # order of protocol implementation and then sniffing for a response
# to one of them. # to one of them.
# #
# #
# 1. Remote isn't a Mercurial server # 1. Remote isn't a Mercurial server
# 2. Remote is a <0.9.1 Mercurial server # 2. Remote is a <0.9.1 Mercurial server
# 3. Remote is a future Mercurial server that dropped ``hello`` # 3. Remote is a future Mercurial server that dropped ``hello``
# and other attempted handshake mechanisms. # and other attempted handshake mechanisms.
if not caps: if not caps:
badresponse() badresponse()
# Flush any output on stderr before proceeding.
_forwardoutput(ui, stderr)
return protoname, caps return protoname, caps
class sshv1peer(wireproto.wirepeer): class sshv1peer(wireproto.wirepeer):
def __init__(self, ui, url, proc, stdin, stdout, stderr, caps): def __init__(self, ui, url, proc, stdin, stdout, stderr, caps):
"""Create a peer from an existing SSH connection. """Create a peer from an existing SSH connection.
``proc`` is a handle on the underlying SSH process. ``proc`` is a handle on the underlying SSH process.
``stdin``, ``stdout``, and ``stderr`` are handles on the stdio ``stdin``, ``stdout``, and ``stderr`` are handles on the stdio
pipes for that process. pipes for that process.
``caps`` is a set of capabilities supported by the remote. ``caps`` is a set of capabilities supported by the remote.
""" """
self._url = url self._url = url
self._ui = ui self._ui = ui
# self._subprocess is unused. Keeping a handle on the process # self._subprocess is unused. Keeping a handle on the process
# holds a reference and prevents it from being garbage collected. # holds a reference and prevents it from being garbage collected.
self._subprocess = proc self._subprocess = proc
# And we hook up our "doublepipe" wrapper to allow querying
# stderr any time we perform I/O.
stdout = doublepipe(ui, util.bufferedinputpipe(stdout), stderr)
stdin = doublepipe(ui, stdin, stderr)
self._pipeo = stdin self._pipeo = stdin
self._pipei = stdout self._pipei = stdout
self._pipee = stderr self._pipee = stderr
self._caps = caps self._caps = caps
# Commands that have a "framed" response where the first line of the # Commands that have a "framed" response where the first line of the
# response contains the length of that response. # response contains the length of that response.
_FRAMED_COMMANDS = { _FRAMED_COMMANDS = {

tests/test-check-interfaces.py

class badpeer(httppeer.httppeer): class badpeer(httppeer.httppeer):
def __init__(self): def __init__(self):
super(badpeer, self).__init__(uimod.ui(), 'http://localhost') super(badpeer, self).__init__(uimod.ui(), 'http://localhost')
self.badattribute = True self.badattribute = True
def badmethod(self): def badmethod(self):
pass pass
class dummypipe(object):
def close(self):
pass
def main(): def main():
ui = uimod.ui() ui = uimod.ui()
checkobject(badpeer()) checkobject(badpeer())
checkobject(httppeer.httppeer(ui, 'http://localhost')) checkobject(httppeer.httppeer(ui, 'http://localhost'))
checkobject(localrepo.localpeer(dummyrepo())) checkobject(localrepo.localpeer(dummyrepo()))
checkobject(sshpeer.sshv1peer(ui, 'ssh://localhost/foo', None, None, None, checkobject(sshpeer.sshv1peer(ui, 'ssh://localhost/foo', None, dummypipe(),
None, None)) dummypipe(), None, None))
checkobject(sshpeer.sshv2peer(ui, 'ssh://localhost/foo', None, None, None, checkobject(sshpeer.sshv2peer(ui, 'ssh://localhost/foo', None, dummypipe(),
None, None)) dummypipe(), None, None))
checkobject(bundlerepo.bundlepeer(dummyrepo())) checkobject(bundlerepo.bundlepeer(dummyrepo()))
checkobject(statichttprepo.statichttppeer(dummyrepo())) checkobject(statichttprepo.statichttppeer(dummyrepo()))
checkobject(unionrepo.unionpeer(dummyrepo())) checkobject(unionrepo.unionpeer(dummyrepo()))
main() main()