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

Lint Skipped

Unit

Unit Tests Skipped

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 5234

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')