From e81ce2bbd2eb4aaa5986cb22038aed604414b182 Mon Sep 17 00:00:00 2001
From: david-sarah <david-sarah@jacaranda.org>
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