This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2033

sshpeer: inline I/O into _validaterepo()
ClosedPublic

Authored by indygreg on Feb 4 2018, 10:35 PM.

Download Raw Diff

Details

Reviewers

yuja

Group Reviewers

hg-reviewers

Commits

rHGa9cffd14aa04: sshpeer: inline I/O into _validaterepo()

Summary

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.

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 4 2018, 10:35 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptFeb 4 2018, 10:35 PM

Herald added a subscriber: mercurial-devel. · View Herald Transcript

indygreg added a child revision: D2034: sshpeer: move handshake outside of sshpeer.Feb 4 2018, 10:35 PM

indygreg updated this revision to Diff 5228.Feb 5 2018, 12:15 PM

indygreg updated this revision to Diff 5234.Feb 5 2018, 5:21 PM

yuja accepted this revision.Feb 6 2018, 8:07 AM

This revision is now accepted and ready to land.Feb 6 2018, 8:07 AM

Closed by commit rHGa9cffd14aa04: sshpeer: inline I/O into _validaterepo() (authored by indygreg). · Explain WhyFeb 6 2018, 8:24 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/sshpeer.py (29 lines)
M			tests/sshprotoext.py (14 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	5191		Feb 4 2018, 10:35 PM	★	★
Diff 2	5228		Feb 5 2018, 12:15 PM	★	★
Diff 3	5234		Feb 5 2018, 5:21 PM	★	★
Diff 4	5250	rHGa9cffd14aa041b146e4099f0223b57e0f503df25	Feb 4 2018, 5:10 PM	★	★

Status	Author	Revision
Closed	indygreg	D2036 sshpeer: remove support for connecting to <0.9.1 servers (BC)
Closed	indygreg	D2035 sshpeer: document the handshake mechanism
Closed	indygreg	D2034 sshpeer: move handshake outside of sshpeer
Closed	indygreg	D2033 sshpeer: inline I/O into _validaterepo()
Closed	indygreg	D2032 sshpeer: clean up API for sshpeer.__init__ (API)
Closed	indygreg	D2031 sshpeer: establish SSH connection before class instantiation
Closed	indygreg	D2030 sshpeer: remove frivolous call to _cleanup()
Closed	indygreg	D2029 sshpeer: extract pipe cleanup logic to own function
Closed	indygreg	D2028 sshpeer: move ssh command and repo creation logic out of __init__
Closed	indygreg	D2027 sshpeer: move URL validation out of sshpeer.__init__
Closed	indygreg	D2026 tests: add low-level SSH protocol tests
Closed	indygreg	D2025 debugcommands: introduce debugpeer command
Closed	indygreg	D2024 sshpeer: make "instance" a function

Diff 5250

mercurial/sshpeer.py


	def _validaterepo(self):			def _validaterepo(self):
	def badresponse():			def badresponse():
	msg = _("no suitable response from remote hg")			msg = _("no suitable response from remote hg")
	hint = self.ui.config("ui", "ssherrorhint")			hint = self.ui.config("ui", "ssherrorhint")
	self._abort(error.RepoError(msg, hint=hint))			self._abort(error.RepoError(msg, hint=hint))

	try:			try:
	# skip any noise generated by remote shell			pairsarg = '%s-%s' % ('0' * 40, '0' * 40)
	self._callstream("hello")
	r = self._callstream("between", pairs=("%s-%s" % ("0"40, "0"40)))			handshake = [
				'hello\n',
				'between\n',
				'pairs %d\n' % len(pairsarg),
				pairsarg,
				]

				requestlog = self.ui.configbool('devel', 'debug.peer-request')

				if requestlog:
				self.ui.debug('devel-peer-request: hello\n')
				self.ui.debug('sending hello command\n')
				if requestlog:
				self.ui.debug('devel-peer-request: between\n')
				self.ui.debug('devel-peer-request: pairs: %d bytes\n' %
				len(pairsarg))
				self.ui.debug('sending between command\n')

				self._pipeo.write(''.join(handshake))
				self._pipeo.flush()
	except IOError:			except IOError:
	badresponse()			badresponse()

	lines = ["", "dummy"]			lines = ["", "dummy"]
	max_noise = 500			max_noise = 500
	while lines[-1] and max_noise:			while lines[-1] and max_noise:
	try:			try:
	l = r.readline()			l = self._pipei.readline()
	self._readerr()			_forwardoutput(self.ui, self._pipee)
	if lines[-1] == "1\n" and l == "\n":			if lines[-1] == "1\n" and l == "\n":
	break			break
	if l:			if l:
	self.ui.debug("remote: ", l)			self.ui.debug("remote: ", l)
	lines.append(l)			lines.append(l)
	max_noise -= 1			max_noise -= 1
	except IOError:			except IOError:
	badresponse()			badresponse()

tests/sshprotoext.py

	assert l == b'between\n'			assert l == b'between\n'
	rsp = wireproto.dispatch(self._repo, self, b'between')			rsp = wireproto.dispatch(self._repo, self, b'between')
	self._handlers[rsp.__class__](self, rsp)			self._handlers[rsp.__class__](self, rsp)

	super(prehelloserver, self).serve_forever()			super(prehelloserver, self).serve_forever()

	class extrahandshakecommandspeer(sshpeer.sshpeer):			class extrahandshakecommandspeer(sshpeer.sshpeer):
	"""An ssh peer that sends extra commands as part of initial handshake."""			"""An ssh peer that sends extra commands as part of initial handshake."""
	# There isn't a good hook point. So we wrap _callstream() and inject			def _validaterepo(self):
	# logic when the peer says "hello".
	def _callstream(self, cmd, **args):
	if cmd != b'hello':
	return super(extrahandshakecommandspeer, self)._callstream(cmd,
	**args)

	mode = self._ui.config(b'sshpeer', b'handshake-mode')			mode = self._ui.config(b'sshpeer', b'handshake-mode')
	if mode == b'pre-no-args':			if mode == b'pre-no-args':
	self._callstream(b'no-args')			self._callstream(b'no-args')
	return super(extrahandshakecommandspeer, self)._callstream(			return super(extrahandshakecommandspeer, self)._validaterepo()
	cmd, **args)
	elif mode == b'pre-multiple-no-args':			elif mode == b'pre-multiple-no-args':
	self._callstream(b'unknown1')			self._callstream(b'unknown1')
	self._callstream(b'unknown2')			self._callstream(b'unknown2')
	self._callstream(b'unknown3')			self._callstream(b'unknown3')
	return super(extrahandshakecommandspeer, self)._callstream(			return super(extrahandshakecommandspeer, self)._validaterepo()
	cmd, **args)
	else:			else:
	raise error.ProgrammingError(b'unknown HANDSHAKECOMMANDMODE: %s' %			raise error.ProgrammingError(b'unknown HANDSHAKECOMMANDMODE: %s' %
	mode)			mode)

	def registercommands():			def registercommands():
	def dummycommand(repo, proto):			def dummycommand(repo, proto):
	raise error.ProgrammingError('this should never be called')			raise error.ProgrammingError('this should never be called')