SFTP: a posix-rename response should actually return an FXP_STATUS reply, not an...

diff --git a/src/allmydata/frontends/sftpd.py b/src/allmydata/frontends/sftpd.py
index ccf05b24082e9473a5060f7feadd03d8ecaf3abd..b0e9ba2ec4c4857f9a46cb710a2427e6e98636da 100644 (file)
--- 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 84d2c8ad51093501e74ed4a3251ca29fd6c0a363..ca21c3b5c844055861d11ba689abf03156a5ceaf 100644 (file)
--- 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")