Gregory Szorc <gregory.szorc@gmail.com> [Sun, 04 Feb 2018 14:44:04 -0800] rev 35939
sshpeer: document the handshake mechanism
The mechanism by which SSH peers establish connections with remotes
is wonky and requires a bit of code archeology to understand. While
it is already documented in `hg help internals.wireproto`, it helps
to have documentation in the code as well.
Differential Revision: https://phab.mercurial-scm.org/D2035
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 05 Feb 2018 09:14:32 -0800] rev 35938
sshpeer: move handshake outside of sshpeer
With the handshake now performed before a peer class is instantiated,
we can now instantiate a different peer class depending on the results
of the handshake.
Our test extension had to change to cope with the new API. Because
we now issue the command via raw I/O calls and don't call
_callstream(), we no longer have to register the fake command.
(_callstream() uses the command registration to see what args to
send).
Differential Revision: https://phab.mercurial-scm.org/D2034
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 04 Feb 2018 14:10:56 -0800] rev 35937
sshpeer: inline I/O into _validaterepo()
We want to move the handshake code out of the peer class so the
peer factory function can perform the handshake and instantiate
a proper class depending on the results. To make that refactor
easier to read, we first inline I/O functionality into
_validaterepo().
Test output for low-level protocol tests didn't change, thus
hopefully demonstrating that this refactor didn't change any
material behavior.
Because we no longer call _callstream(), our test extension for
monkeypatching the peer had to change its hook point.
Differential Revision: https://phab.mercurial-scm.org/D2033
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 05 Feb 2018 14:17:24 -0800] rev 35936
sshpeer: clean up API for sshpeer.__init__ (API)
Our refactoring left the state of sshpeer.__init__ in a poor
state. "create" was no longer used. Process/pipe arguments were
passed poorly. "name" was really a URL.
This commit cleans all that up.
.. api::
sshpeer.sshpeer.__init__ now receives arguments describing an
existing connection instead of creating a connection itself.
Differential Revision: https://phab.mercurial-scm.org/D2032
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 05 Feb 2018 14:05:59 -0800] rev 35935
sshpeer: establish SSH connection before class instantiation
We want to move the handshake to before peers are created so
we can instantiate a different peer class depending on the
results of the handshake. This necessitates moving the SSH
process invocation to outside the peer class.
As part of the code move, some variables were renamed for
clarity. util.popen4() returns stdin, stdout, and stderr in
their typical file descriptor order. However, stdin and stdout
were being mapped to "pipeo" and "pipei" respectively. "o"
for "stdin" and "i" for "stdout" is a bit confusing. Although
it does make sense for "output" and "input" from the perspective
of the client. But in the context of the new function, it makes
sense to refer to these as their file descriptor names.
In addition, the last use of self._path disappeared, so we stop
setting that attribute and we can delete the redundant URL
parsing necessary to set it.
Differential Revision: https://phab.mercurial-scm.org/D2031
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 04 Feb 2018 11:40:13 -0800] rev 35934
sshpeer: remove frivolous call to _cleanup()
_validaterepo() is called once during __init__. _cleanup()
no-ops if the self._pipe* attributes aren't set. These attributes
are set during _validaterepo(). So the call to _cleanup() isn't
necessary.
But just to be on the safe side, we add an assertion.
Differential Revision: https://phab.mercurial-scm.org/D2030
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 04 Feb 2018 11:37:19 -0800] rev 35933
sshpeer: extract pipe cleanup logic to own function
So it can be used outside of instantiated classes. This is
needed to support pipe creation before __init__ is called.
Differential Revision: https://phab.mercurial-scm.org/D2029
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 04 Feb 2018 19:23:40 -0800] rev 35932
sshpeer: move ssh command and repo creation logic out of __init__
It was easier to move both of these at once because repository
creation requires various variables and I didn't want to add
tons of arguments and code to __init__ that will soon be deleted
anyway. We do add an extra argument so we can proxy values to the
_validaterepo() call. But this is minimally invasive.
Some callers of self._abort() were converted to just raise. Like
before, the _abort() call wasn't necessary because self._pipe*
aren't populated this early in the object's lifetime.
As part of this, various private attributes derived from the parsed
URL are no longer used. So we no longer set them.
Differential Revision: https://phab.mercurial-scm.org/D2028
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 04 Feb 2018 12:55:18 -0800] rev 35931
sshpeer: move URL validation out of sshpeer.__init__
We will soon have another SSH peer class to support the new version
of the SSH protocol. However, we won't know which peer class to
instantiate until we perform a handshake on an active connection.
This means that we need to move connection establishment and handshake
code out of sshpeer.__init__.
This commit starts the process of migrating peer creation code
out of sshpeer.__init__ into instance(), which is the API for
creating peers.
The moved code no longer calls _abort(). _abort() runs _cleanup() and
raises. _cleanup() only performs actions on self._pipe*. These objects
aren't instantiated until we actually connect to the peer. So _abort()
was not necessary in the old code.
To keep the API the same, __init__() now makes a redundant call to
util.url(). This will be fixed in subsequent commits.
Differential Revision: https://phab.mercurial-scm.org/D2027
Gregory Szorc <gregory.szorc@gmail.com> [Sun, 04 Feb 2018 14:02:41 -0800] rev 35930
tests: add low-level SSH protocol tests
We don't really have good low-level tests for the behavior of the SSH
wire protocol. This commit attempts to establish some.
The added tests consist of a mixture of starting a server
with `hg serve --stdio` and sending bytes to it and using
`hg debugpeer` to go through the official client code. Having
insight into what raw bytes are exchanged as well as what the peer
does is useful.
We also introduce a test extension for modifying the behavior of
the SSH server and peer. For example, we change the server to
not recognize the "hello" command, simulating behavior of <0.9.1
servers.
These tests are generally useful to have. But the impetus for creating
them now is they will be needed for verifying behavior of old clients
and servers when a new SSH protocol is introduced.
Differential Revision: https://phab.mercurial-scm.org/D2026
# no-check-commit because of serve_forever()