From: david-sarah Date: Mon, 17 May 2010 01:26:06 +0000 (-0700) Subject: SFTP: work around a probable bug in twisted.conch.ssh.session:loseConnection(). Also... X-Git-Tag: trac-4400~25 X-Git-Url: https://git.rkrishnan.org/uri//%22news.html/%22?a=commitdiff_plain;h=5f9c10901be1ad51e9d8a0e3c17ec830112dbb03;p=tahoe-lafs%2Ftahoe-lafs.git SFTP: work around a probable bug in twisted.conch.ssh.session:loseConnection(). Also some minor error handling cleanups. --- diff --git a/src/allmydata/frontends/sftpd.py b/src/allmydata/frontends/sftpd.py index e7f98633..24b084bf 100644 --- a/src/allmydata/frontends/sftpd.py +++ b/src/allmydata/frontends/sftpd.py @@ -118,8 +118,8 @@ def _raise_error(err): if err.check(defer.FirstError): _raise_error(err.value.subFailure) - # We assume that the type of error is not anonymity-sensitive. - raise SFTPError(FX_FAILURE, str(err.type)) + # We assume that the error message is not anonymity-sensitive. + raise SFTPError(FX_FAILURE, str(err.value)) def _repr_flags(flags): return "|".join([f for f in @@ -512,7 +512,7 @@ SIZE_THRESHOLD = 1000 def _make_sftp_file(check_abort, flags, convergence, parent=None, childname=None, filenode=None, metadata=None): if noisy: logmsg("_make_sftp_file(%r, %r, %r, parent=%r, childname=%r, filenode=%r, metadata=%r" % - (check_abort, flags, convergence, parent, childname, filenode, metadata), NOISY) + (check_abort, flags, convergence, parent, childname, filenode, metadata), level=NOISY) if not (flags & (FXF_WRITE | FXF_CREAT)) and (flags & FXF_READ) and filenode and \ not filenode.is_mutable() and filenode.get_size() <= SIZE_THRESHOLD: @@ -802,7 +802,7 @@ class Reason: class SFTPUserHandler(ConchUser, PrefixingLogMixin): - implements(ISFTPServer, ISession) + implements(ISFTPServer) def __init__(self, client, rootnode, username): ConchUser.__init__(self) PrefixingLogMixin.__init__(self, facility="tahoe.sftp") @@ -811,48 +811,17 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin): self.channelLookup["session"] = session.SSHSession self.subsystemLookup["sftp"] = FileTransferServer - self.client = client - self.root = rootnode - self.username = username - self.convergence = client.convergence - self.logged_out = False + self._client = client + self._root = rootnode + self._username = username + self._convergence = client.convergence + self._logged_out = False def logout(self): - self.logged_out = True + self._logged_out = True def check_abort(self): - return self.logged_out - - # ISession - # This is needed because some clients may try to issue a 'df' command. - - def getPty(self, terminal, windowSize, attrs): - self.log(".getPty(%r, %r, %r)" % (terminal, windowSize, attrs), level=OPERATIONAL) - - def openShell(self, protocol): - self.log(".openShell(%r)" % (protocol,), level=OPERATIONAL) - protocol.write("This server supports only SFTP, not shell sessions.\n") - protocol.processEnded(Reason(ProcessTerminated(exitCode=1))) - - def execCommand(self, protocol, cmd): - self.log(".execCommand(%r, %r)" % (protocol, cmd), level=OPERATIONAL) - if cmd == "df -P -k /": - protocol.write("Filesystem 1024-blocks Used Available Capacity Mounted on\n" - "tahoe 628318530 314159265 314159265 50% /\n") - protocol.processEnded(Reason(ProcessDone(None))) - else: - protocol.processEnded(Reason(ProcessTerminated(exitCode=1))) - - def windowChanged(self, newWindowSize): - self.log(".windowChanged(%r)" % (newWindowSize,), level=OPERATIONAL) - - def eofReceived(self): - self.log(".eofReceived()", level=OPERATIONAL) - - def closed(self): - self.log(".closed()", level=OPERATIONAL) - - # ISFTPServer + return self._logged_out def gotVersion(self, otherVersion, extData): self.log(".gotVersion(%r, %r)" % (otherVersion, extData), level=OPERATIONAL) @@ -929,7 +898,7 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin): raise SFTPError(FX_PERMISSION_DENIED, "cannot create a file exclusively when it already exists") - return _make_sftp_file(self.check_abort, flags, self.convergence, filenode=root) + return _make_sftp_file(self.check_abort, flags, self._convergence, filenode=root) else: # case 2 childname = path[-1] @@ -984,7 +953,7 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin): raise SFTPError(FX_PERMISSION_DENIED, "cannot open a file for writing when the parent directory is read-only") - return _make_sftp_file(self.check_abort, flags, self.convergence, parent=parent, + return _make_sftp_file(self.check_abort, flags, self._convergence, parent=parent, childname=childname, filenode=filenode, metadata=metadata) def _no_child(f): if noisy: self.log("_no_child(%r)" % (f,), level=NOISY) @@ -999,7 +968,7 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin): raise SFTPError(FX_PERMISSION_DENIED, "cannot create a file when the parent directory is read-only") - return _make_sftp_file(self.check_abort, flags, self.convergence, parent=parent, + return _make_sftp_file(self.check_abort, flags, self._convergence, parent=parent, childname=childname) d2.addCallbacks(_got_child, _no_child) return d2 @@ -1168,7 +1137,7 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin): if extendedName == 'statvfs@openssh.com' or extendedName == 'fstatvfs@openssh.com': # - return struct.pack('>QQQQQQQQQQQ', + return struct.pack('>11Q', 1024, # uint64 f_bsize /* file system block size */ 1024, # uint64 f_frsize /* fundamental fs block size */ 628318530, # uint64 f_blocks /* number of blocks (unit f_frsize) */ @@ -1233,10 +1202,10 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin): def _get_root(self, path): # return (root, remaining_path) if path and path[0] == u"uri": - d = defer.maybeDeferred(self.client.create_node_from_uri, path[1].encode('utf-8')) + d = defer.maybeDeferred(self._client.create_node_from_uri, path[1].encode('utf-8')) d.addCallback(lambda root: (root, path[2:])) else: - d = defer.succeed((self.root, path)) + d = defer.succeed((self._root, path)) return d def _get_parent(self, path): @@ -1316,12 +1285,12 @@ from auth import AccountURLChecker, AccountFileChecker, NeedRootcapLookupScheme class Dispatcher: implements(portal.IRealm) def __init__(self, client): - self.client = client + self._client = client def requestAvatar(self, avatarID, mind, interface): assert interface == IConchUser - rootnode = self.client.create_node_from_uri(avatarID.rootcap) - handler = SFTPUserHandler(self.client, rootnode, avatarID.username) + rootnode = self._client.create_node_from_uri(avatarID.rootcap) + handler = SFTPUserHandler(self._client, rootnode, avatarID.username) return (interface, handler, handler.logout) diff --git a/src/allmydata/test/test_sftp.py b/src/allmydata/test/test_sftp.py index 919c6806..16e68efc 100644 --- a/src/allmydata/test/test_sftp.py +++ b/src/allmydata/test/test_sftp.py @@ -7,6 +7,7 @@ from twisted.internet import defer from twisted.python.failure import Failure from twisted.internet.error import ProcessDone, ProcessTerminated +conch_interfaces = None sftp = None sftpd = None have_pycrypto = False @@ -18,6 +19,7 @@ except ImportError: pass if have_pycrypto: + from twisted.conch import interfaces as conch_interfaces from twisted.conch.ssh import filetransfer as sftp from allmydata.frontends import sftpd @@ -669,6 +671,24 @@ class Handler(GridTestMixin, ShouldFailMixin, unittest.TestCase): d.addCallback(lambda ign: self.root.get(u"zerolength")) d.addCallback(lambda node: download_to_data(node)) d.addCallback(lambda data: self.failUnlessReallyEqual(data, "")) + + # test WRITE | CREAT | EXCL | APPEND + d.addCallback(lambda ign: + self.handler.openFile("exclappend", sftp.FXF_WRITE | sftp.FXF_CREAT | + sftp.FXF_EXCL | sftp.FXF_APPEND, {})) + def _write_excl_append(wf): + d2 = self.root.get(u"exclappend") + d2.addCallback(lambda node: download_to_data(node)) + d2.addCallback(lambda data: self.failUnlessReallyEqual(data, "")) + + d2 = wf.writeChunk(10, "0123456789") + d2.addCallback(wf.writeChunk(5, "01234")) + d2.addCallback(lambda ign: wf.close()) + return d2 + d.addCallback(_write_excl_append) + d.addCallback(lambda ign: self.root.get(u"exclappend")) + d.addCallback(lambda node: download_to_data(node)) + d.addCallback(lambda data: self.failUnlessReallyEqual(data, "012345678901234")) """ # test WRITE | CREAT without TRUNC @@ -917,32 +937,45 @@ class Handler(GridTestMixin, ShouldFailMixin, unittest.TestCase): self.reason = None def write(self, data): self.output += data + return defer.succeed(None) def processEnded(self, reason): self.reason = reason - - protocol_df = FakeProtocol() - protocol_error = FakeProtocol() - protocol_shell = FakeProtocol() + return defer.succeed(None) d = self._set_up("execCommand_and_openShell") - d.addCallback(lambda ign: self.handler.execCommand(protocol_df, "df -P -k /")) - d.addCallback(lambda ign: self.failUnlessIn("1024-blocks", protocol_df.output)) - d.addCallback(lambda ign: self.failUnless(isinstance(protocol_df.reason.value, ProcessDone))) - d.addCallback(lambda ign: self.handler.eofReceived()) - d.addCallback(lambda ign: self.handler.closed()) - - d.addCallback(lambda ign: self.handler.execCommand(protocol_error, "error")) - d.addCallback(lambda ign: self.failUnlessEqual("", protocol_error.output)) - d.addCallback(lambda ign: self.failUnless(isinstance(protocol_error.reason.value, ProcessTerminated))) - d.addCallback(lambda ign: self.failUnlessEqual(protocol_error.reason.value.exitCode, 1)) - d.addCallback(lambda ign: self.handler.closed()) - - d.addCallback(lambda ign: self.handler.openShell(protocol_shell)) - d.addCallback(lambda ign: self.failUnlessIn("only SFTP", protocol_shell.output)) - d.addCallback(lambda ign: self.failUnless(isinstance(protocol_shell.reason.value, ProcessTerminated))) - d.addCallback(lambda ign: self.failUnlessEqual(protocol_shell.reason.value.exitCode, 1)) - d.addCallback(lambda ign: self.handler.closed()) + d.addCallback(lambda ign: conch_interfaces.ISession(self.handler)) + def _exec_df(session): + protocol = FakeProtocol() + d2 = session.execCommand(protocol, "df -P -k /") + d2.addCallback(lambda ign: self.failUnlessIn("1024-blocks", protocol.output)) + d2.addCallback(lambda ign: self.failUnless(isinstance(protocol.reason.value, ProcessDone))) + d2.addCallback(lambda ign: session.eofReceived()) + d2.addCallback(lambda ign: session.closed()) + return d2 + d.addCallback(_exec_df) + + d.addCallback(lambda ign: conch_interfaces.ISession(self.handler)) + def _exec_error(session): + protocol = FakeProtocol() + d2 = session.execCommand(protocol, "error") + d2.addCallback(lambda ign: self.failUnlessEqual("", protocol.output)) + d2.addCallback(lambda ign: self.failUnless(isinstance(protocol.reason.value, ProcessTerminated))) + d2.addCallback(lambda ign: self.failUnlessEqual(protocol.reason.value.exitCode, 1)) + d2.addCallback(lambda ign: session.closed()) + return d2 + d.addCallback(_exec_error) + + d.addCallback(lambda ign: conch_interfaces.ISession(self.handler)) + def _openShell(session): + protocol = FakeProtocol() + d2 = session.openShell(protocol) + d2.addCallback(lambda ign: self.failUnlessIn("only SFTP", protocol.output)) + d2.addCallback(lambda ign: self.failUnless(isinstance(protocol.reason.value, ProcessTerminated))) + d2.addCallback(lambda ign: self.failUnlessEqual(protocol.reason.value.exitCode, 1)) + d2.addCallback(lambda ign: session.closed()) + return d2 + d.addCallback(_exec_error) return d