From e81ce2bbd2eb4aaa5986cb22038aed604414b182 Mon Sep 17 00:00:00 2001 From: david-sarah Date: Mon, 24 May 2010 20:33:23 -0700 Subject: [PATCH] SFTP: a posix-rename response should actually return an FXP_STATUS reply, not an FXP_EXTENDED_REPLY as Twisted Conch assumes. Work around this by raising an SFTPError with code FX_OK. --- src/allmydata/frontends/sftpd.py | 10 ++++++++-- src/allmydata/test/test_sftp.py | 12 ++++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/allmydata/frontends/sftpd.py b/src/allmydata/frontends/sftpd.py index ccf05b24..b0e9ba2e 100644 --- a/src/allmydata/frontends/sftpd.py +++ b/src/allmydata/frontends/sftpd.py @@ -9,7 +9,7 @@ from twisted.application import service, strports from twisted.conch.ssh import factory, keys, session from twisted.conch.ssh.filetransfer import FileTransferServer, SFTPError, \ FX_NO_SUCH_FILE, FX_OP_UNSUPPORTED, FX_PERMISSION_DENIED, FX_EOF, \ - FX_BAD_MESSAGE, FX_FAILURE + FX_BAD_MESSAGE, FX_FAILURE, FX_OK from twisted.conch.ssh.filetransfer import FXF_READ, FXF_WRITE, FXF_APPEND, \ FXF_CREAT, FXF_TRUNC, FXF_EXCL from twisted.conch.interfaces import ISFTPServer, ISFTPFile, IConchUser, ISession @@ -1399,7 +1399,13 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin): fromPathstring = extensionData[4:(4 + fromPathLen)] toPathstring = extensionData[(8 + fromPathLen):] d = self.renameFile(fromPathstring, toPathstring, overwrite=True) - d.addCallback(lambda ign: "") + + # Twisted conch assumes that the response from an extended request is either + # an error, or an FXP_EXTENDED_REPLY. But it happens to do the right thing + # (respond with an FXP_STATUS message) if we return a Failure with code FX_OK. + def _succeeded(ign): + raise SFTPError(FX_OK, "request succeeded") + d.addCallback(_succeeded) return d if extensionName == 'statvfs@openssh.com' or extensionName == 'fstatvfs@openssh.com': diff --git a/src/allmydata/test/test_sftp.py b/src/allmydata/test/test_sftp.py index 84d2c8ad..ca21c3b5 100644 --- a/src/allmydata/test/test_sftp.py +++ b/src/allmydata/test/test_sftp.py @@ -52,7 +52,7 @@ class Handler(GridTestMixin, ShouldFailMixin, unittest.TestCase): (which, expected_code, res.value.code, res)) else: print '@' + '@'.join(s) - self.fail("%s was supposed to raise SFTPError(%d), not get '%s'" % + self.fail("%s was supposed to raise SFTPError(%d), not get %r" % (which, expected_code, res)) d.addBoth(_done) return d @@ -1009,8 +1009,16 @@ class Handler(GridTestMixin, ShouldFailMixin, unittest.TestCase): def _renameFile(fromPathstring, toPathstring): extData = (struct.pack('>L', len(fromPathstring)) + fromPathstring + struct.pack('>L', len(toPathstring)) + toPathstring) + d2 = self.handler.extendedRequest('posix-rename@openssh.com', extData) - d2.addCallback(lambda res: self.failUnlessReallyEqual(res, "")) + def _check(res): + res.trap(sftp.SFTPError) + if res.value.code == sftp.FX_OK: + return None + return res + d2.addCallbacks(lambda res: self.fail("posix-rename request was supposed to " + "raise an SFTPError, not get '%r'" % (res,)), + _check) return d2 d = self._set_up("renameFile_posix") -- 2.45.2