From: david-sarah <david-sarah@jacaranda.org>
Date: Tue, 25 May 2010 03:33:23 +0000 (-0700)
Subject: SFTP: a posix-rename response should actually return an FXP_STATUS reply, not an... 
X-Git-Tag: trac-4400~4
X-Git-Url: https://git.rkrishnan.org/frontends/reliability?a=commitdiff_plain;h=e81ce2bbd2eb4aaa5986cb22038aed604414b182;p=tahoe-lafs%2Ftahoe-lafs.git

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.
---

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