finally happy with basics of SFTP support

git-svn-id: http://pyfilesystem.googlecode.com/svn/branches/rfk-ideas@148 67cdc799-7952-0410-af00-57a81ceafa0f

4 files changed, 160 insertions, 69 deletions

diff --git a/fs/expose/sftp.py b/fs/expose/sftp.py
index e81fcff..058a251 100644
--- a/fs/expose/sftp.py
+++ b/fs/expose/sftp.py
@@ -1,52 +1,67 @@
 """
 
-  fs.expose.sftp:  expose an FS object via SFTP, using paramiko.
+  fs.expose.sftp:  expose an FS object over SFTP (via paramiko).
+
+This module provides the necessary interfaces to expose an FS object over
+SFTP, plugging into the infratructure provided by the 'paramiko' module.
+
+For simple usage, the class 'BaseSFTPServer' provides an all-in-one server
+class based on the standard SocketServer module.  Use it like so:
+
+    server = BaseSFTPServer((hostname,port),fs)
+    server.serve_forever()
+
+Note that the base class allows UNAUTHENTICATED ACCESS by default.  For more
+serious work you will probably want to subclass it and override methods such
+as check_auth_password() and get_allowed_auths().
+
+To integrate this module into an existing server framework based on paramiko,
+the 'SFTPServerInterface' class provides a concrete implementation of the
+paramiko.SFTPServerInterface protocol.  If you don't understand what this
+is, you probably don't want to use it.
 
 """
 
 import os
 import stat as statinfo
 import time
-import SocketServer as ss
+import SocketServer as sockserv
 import threading
+from StringIO import StringIO
 
 import paramiko
 
 from fs.errors import *
 from fs.helpers import *
 
+
+# Default host key used by BaseSFTPServer
+#
+DEFAULT_HOST_KEY = paramiko.RSAKey.from_private_key(StringIO("-----BEGIN RSA PRIVATE KEY-----\nMIICXgIBAAKCAIEAl7sAF0x2O/HwLhG68b1uG8KHSOTqe3Cdlj5i/1RhO7E2BJ4B\n3jhKYDYtupRnMFbpu7fb21A24w3Y3W5gXzywBxR6dP2HgiSDVecoDg2uSYPjnlDk\nHrRuviSBG3XpJ/awn1DObxRIvJP4/sCqcMY8Ro/3qfmid5WmMpdCZ3EBeC0CAwEA\nAQKCAIBSGefUs5UOnr190C49/GiGMN6PPP78SFWdJKjgzEHI0P0PxofwPLlSEj7w\nRLkJWR4kazpWE7N/bNC6EK2pGueMN9Ag2GxdIRC5r1y8pdYbAkuFFwq9Tqa6j5B0\nGkkwEhrcFNBGx8UfzHESXe/uE16F+e8l6xBMcXLMJVo9Xjui6QJBAL9MsJEx93iO\nzwjoRpSNzWyZFhiHbcGJ0NahWzc3wASRU6L9M3JZ1VkabRuWwKNuEzEHNK8cLbRl\nTyH0mceWXcsCQQDLDEuWcOeoDteEpNhVJFkXJJfwZ4Rlxu42MDsQQ/paJCjt2ONU\nWBn/P6iYDTvxrt/8+CtLfYc+QQkrTnKn3cLnAkEAk3ixXR0h46Rj4j/9uSOfyyow\nqHQunlZ50hvNz8GAm4TU7v82m96449nFZtFObC69SLx/VsboTPsUh96idgRrBQJA\nQBfGeFt1VGAy+YTLYLzTfnGnoFQcv7+2i9ZXnn/Gs9N8M+/lekdBFYgzoKN0y4pG\n2+Q+Tlr2aNlAmrHtkT13+wJAJVgZATPI5X3UO0Wdf24f/w9+OY+QxKGl86tTQXzE\n4bwvYtUGufMIHiNeWP66i6fYCucXCMYtx6Xgu2hpdZZpFw==\n-----END RSA PRIVATE KEY-----\n"))
+
+
 try:
     from functools import wraps
 except ImportError:
     def wraps(f):
         return f
 
-def debug(func):
-    @wraps(func)
-    def wrapper(*args,**kwds):
-        print func, args[1:], kwds
-        try:
-            res = func(*args,**kwds)
-        except Exception, e:
-            print "EXC:", e
-            raise
-        print "RES:", res
-        return res
-    return wrapper
-
 
 def report_sftp_errors(func):
-    """Decorator to catch and report FS errors as SFTP error codes."""
-    @debug
+    """Decorator to catch and report FS errors as SFTP error codes.
+
+    Any FSError exceptions are caught and translated into an appropriate
+    return code, while other exceptions are passed through untouched.
+    """
     @wraps(func)
     def wrapper(*args,**kwds):
         try:
             return func(*args,**kwds)
-        except ResourceNotFoundError:
+        except ResourceNotFoundError, e:
             return paramiko.SFTP_NO_SUCH_FILE
-        except UnsupportedError:
+        except UnsupportedError, e:
             return paramiko.SFTP_OP_UNSUPPORTED
-        except FSError:
+        except FSError, e:
             return paramiko.SFTP_FAILURE
     return wrapper
 
@@ -55,10 +70,14 @@ class SFTPServerInterface(paramiko.SFTPServerInterface):
     """SFTPServerInferface implementation that exposes an FS object.
 
     This SFTPServerInterface subclass expects a single additional argument,
-    the fs object to be exposed.  Use it to set up a transport like so:
+    the fs object to be exposed.  Use it to set up a transport subsystem
+    handler like so:
 
       t.set_subsystem_handler("sftp",SFTPServer,SFTPServerInterface,fs)
 
+    If this all looks too complicated, you might consider the BaseSFTPServer
+    class also provided by this module - it automatically creates the enclosing
+    paramiko server infrastructure.
     """
 
     def __init__(self,server,fs,*args,**kwds):
@@ -100,7 +119,7 @@ class SFTPServerInterface(paramiko.SFTPServerInterface):
 
     @report_sftp_errors
     def rename(self,oldpath,newpath):
-        if self.fs.isfile(path):
+        if self.fs.isfile(oldpath):
             self.fs.move(oldpath,newpath)
         else:
             self.fs.movedir(oldpath,newpath)
@@ -130,7 +149,11 @@ class SFTPServerInterface(paramiko.SFTPServerInterface):
 
 
 class SFTPHandle(paramiko.SFTPHandle):
-    """SFTP file handler pointing to a file in an FS object."""
+    """SFTP file handler pointing to a file in an FS object.
+
+    This is a simple file wrapper for SFTPServerInterface, passing read
+    and write requests directly through the to underlying file from the FS.
+    """
 
     def __init__(self,owner,path,flags):
         super(SFTPHandle,self).__init__(flags)
@@ -174,7 +197,7 @@ class SFTPHandle(paramiko.SFTPHandle):
     @report_sftp_errors
     def write(self,offset,data):
         self._file.seek(offset)
-        self._file.write(length)
+        self._file.write(data)
         return paramiko.SFTP_OK
 
     def stat(self):
@@ -184,34 +207,64 @@ class SFTPHandle(paramiko.SFTPHandle):
         return self.owner.chattr(self.path,attr)
 
 
+class SFTPRequestHandler(sockserv.StreamRequestHandler):
+    """SockerServer RequestHandler subclass for BaseSFTPServer.
 
-class SFTPRequestHandler(ss.StreamRequestHandler):
-    """SockerServer RequestHandler subclass for our SFTP server."""
+    This RequestHandler subclass creates a paramiko Transport, sets up the
+    sftp subsystem, and hands off the the transport's own request handling
+    thread.  Note that paramiko.Transport uses a separate thread by default,
+    so there is no need to use TreadingMixIn.
+    """
 
     def handle(self):
         t = paramiko.Transport(self.request)
         t.add_server_key(self.server.host_key)
         t.set_subsystem_handler("sftp",paramiko.SFTPServer,SFTPServerInterface,self.server.fs)
-        # Careful - this actually spawns a new thread to handle the requests
+        # Note that this actually spawns a new thread to handle the requests.
+        # (Actually, paramiko.Transport is a subclass of Thread)
         t.start_server(server=self.server)
 
 
-class BaseSFTPServer(ss.TCPServer,paramiko.ServerInterface):
-    """SocketServer.TCPServer subclass exposing an FS via SFTP."""
+class BaseSFTPServer(sockserv.TCPServer,paramiko.ServerInterface):
+    """SocketServer.TCPServer subclass exposing an FS via SFTP.
+
+    BaseSFTPServer combines a simple SocketServer.TCPServer subclass with an
+    implementation of paramiko.ServerInterface, providing everything that's
+    needed to expose an FS via SFTP.
+
+    Operation is in the standard SocketServer style.  The target FS object
+    can be passed into the constructor, or set as an attribute on the server:
+
+        server = BaseSFTPServer((hostname,port),fs)
+        server.serve_forever()
+
+    It is also possible to specify the host key used by the sever by setting
+    the 'host_key' attribute.  If this is not specified, it will default to
+    the key found in the DEFAULT_HOST_KEY variable.
+
+    Note that this base class allows UNAUTHENTICATED ACCESS to the exposed
+    FS.  This is intentional, since we can't guess what your authentication
+    needs are.  To protect the exposed FS, override the following methods:
+
+        get_allowed_auths:      determine the allowed auth modes
+        check_auth_none:        check auth with no credentials
+        check_auth_password:    check auth with a password
+        check_auth_publickey:   check auth with a public key
+
+    """
 
     def __init__(self,address,fs=None,host_key=None,RequestHandlerClass=None):
         self.fs = fs
         if host_key is None:
-            self.host_key = paramiko.RSAKey.generate(1024)
-        else:
-            self.host_key = host_key
+            host_key = DEFAULT_HOST_KEY
+        self.host_key = host_key
         if RequestHandlerClass is None:
             RequestHandlerClass = SFTPRequestHandler
-        ss.TCPServer.__init__(self,address,RequestHandlerClass)
+        sockserv.TCPServer.__init__(self,address,RequestHandlerClass)
 
     def close_request(self,request):
-        #  paramiko.Transport closes itself when finished,
-        #  so there's  no need for us to do it.
+        #  paramiko.Transport closes itself when finished.
+        #  If we close it here, we'll break the Transport thread.
         pass
 
     def check_channel_request(self,kind,chanid):
@@ -220,33 +273,31 @@ class BaseSFTPServer(ss.TCPServer,paramiko.ServerInterface):
         return paramiko.OPEN_FAILED_ADMINISTRATIVELY_PROHIBITED
 
     def check_auth_none(self,username):
-        if True:
-            return paramiko.AUTH_SUCCESSFUL
-        return paramiko.AUTH_FAILED
+        """Check whether the user can proceed without authentication."""
+        return paramiko.AUTH_SUCCESSFUL
 
     def check_auth_publickey(self,username,key):
-        if True:
-            return paramiko.AUTH_SUCCESSFUL
+        """Check whether the given public key is valid for authentication."""
         return paramiko.AUTH_FAILED
 
     def check_auth_password(self,username,password):
-        if True:
-            return paramiko.AUTH_SUCCESSFUL
+        """Check whether the given password is valid for authentication."""
         return paramiko.AUTH_FAILED
 
     def get_allowed_auths(self,username):
-        return ("none","publickey","password")
+        """Return list of allowed auth modes.
 
+        The available modes are  "node", "password" and "publickey".
+        """
+        return ("none",)
 
 
-def serve(addr,fs,host_key=None):
-    """Serve the given FS on the given address."""
-    server = BaseSFTPServer(addr,fs)
-    server.serve_forever()
-
-
+#  When called from the command-line, expose a TempFS for testing purposes
 if __name__ == "__main__":
     from fs.tempfs import TempFS
-    serve(("localhost",8023),TempFS())
-
+    server = BaseSFTPServer(("localhost",8022),TempFS())
+    try:
+        server.serve_forever()
+    except (SystemExit,KeyboardInterrupt):
+        server.shutdown()
 
diff --git a/fs/osfs.py b/fs/osfs.py
index fae9a01..8fc1838 100644
--- a/fs/osfs.py
+++ b/fs/osfs.py
@@ -144,6 +144,8 @@ class OSFS(FS):
         try:
             stats = os.stat(sys_path)
         except OSError, e:
+            if e.errno == 2:
+                raise ResourceNotFoundError(path)
             raise ResourceError(path, details=e)
         info = dict((k, getattr(stats, k)) for k in dir(stats) if not k.startswith('__') )
         info['size'] = info['st_size']
diff --git a/fs/sftpfs.py b/fs/sftpfs.py
index fcfe169..72855af 100644
--- a/fs/sftpfs.py
+++ b/fs/sftpfs.py
@@ -121,7 +121,7 @@ class SFTPFS(FS):
             if getattr(e,"errno",None) == 2:
                 return False
             raise OperationFailedError("isdir",path,details=e)
-        return statinfo.S_ISDIR(stat)
+        return statinfo.S_ISDIR(stat.st_mode)
 
     def isfile(self,path):
         npath = self._normpath(path)
@@ -131,7 +131,7 @@ class SFTPFS(FS):
             if getattr(e,"errno",None) == 2:
                 return False
             raise OperationFailedError("isfile",path,details=e)
-        return statinfo.S_ISREG(stat)
+        return statinfo.S_ISREG(stat.st_mode)
 
     def listdir(self,path="./",wildcard=None,full=False,absolute=False,dirs_only=False,files_only=False):
         npath = self._normpath(path)
@@ -139,9 +139,11 @@ class SFTPFS(FS):
             paths = self.client.listdir(npath)
         except IOError, e:
             if getattr(e,"errno",None) == 2:
-                if self.exists(path):
+                if self.isfile(path):
                     raise ResourceInvalidError(path,msg="Can't list directory contents of a file: %(path)s")
                 raise ResourceNotFoundError(path)
+            elif self.isfile(path):
+                raise ResourceInvalidError(path,msg="Can't list directory contents of a file: %(path)s")
             raise OperationFailedError("list directory", path=path, details=e, msg="Unable to get directory listing: %(path)s - (%(details)s)")
         return self._listdir_helper(path, paths, wildcard, full, absolute, dirs_only, files_only)
 
@@ -150,19 +152,26 @@ class SFTPFS(FS):
         try:
             self.client.mkdir(npath)
         except IOError, e:
-            if getattr(e,"errno",None) == 2:
-               if recursive:
-                  self.makedir(dirname(path),recursive=True,allow_recreate=True)
-                  self.makedir(path,allow_recreate=allow_recreate)
-               else:
-                  raise ParentDirectoryMissingError(path)
-            elif getattr(e,"errno",None) is not None:
-                raise OperationFailedError("make directory",path=path,details=e)
+            # Error code is unreliable, try to figure out what went wrong
+            try:
+                stat = self.client.stat(npath)
+            except IOError:
+                if not self.isdir(dirname(path)):
+                    # Parent dir is missing
+                    if not recursive:
+                        raise ParentDirectoryMissingError(path)
+                    self.makedir(dirname(path),recursive=True)
+                    self.makedir(path,allow_recreate=allow_recreate)
+                else:
+                    # Undetermined error
+                    raise OperationFailedError("make directory",path=path,details=e)
             else:
-                if self.isfile(path):
-                    raise ResourceInvalidError(path,msg="Cannot create directory, there's already a file of that name: %(path)s")
-                if not allow_recreate:
-                    raise DestinationExistsError(path,msg="Can not create a directory that already exists (try allow_recreate=True): %(path)s")
+                # Destination exists
+                if statinfo.S_ISDIR(stat.st_mode):
+                    if not allow_recreate:
+                        raise DestinationExistsError(path,msg="Can't create a directory that already exists (try allow_recreate=True): %(path)s")
+                else:
+                    raise ResourceInvalidError(path,msg="Can't create directory, there's already a file of that name: %(path)s")
 
     def remove(self,path):
         npath = self._normpath(path)
diff --git a/fs/tests.py b/fs/tests.py
index f739fa6..495c872 100644
--- a/fs/tests.py
+++ b/fs/tests.py
@@ -5,6 +5,10 @@
 
 """
 
+import sys
+import logging
+logging.basicConfig(level=logging.ERROR,stream=sys.stdout)
+
 import unittest
 
 import shutil
@@ -789,18 +793,43 @@ class TestRPCFS(unittest.TestCase,FSTestCases):
 
 
 import sftpfs
+from fs.expose.sftp import BaseSFTPServer
 class TestSFTPFS(unittest.TestCase,FSTestCases):
 
-    creds = dict(username="rfk",password="obviously-not-my-real-password")
-
     def setUp(self):
         self.temp_fs = tempfs.TempFS()
-        self.fs = sftpfs.SFTPFS("localhost",self.temp_fs.root_path,**self.creds)
+        self.port = 8000
+        self.server = None
+        while not self.server:
+          try:
+            self.server = BaseSFTPServer(("localhost",self.port),self.temp_fs)
+          except socket.error, e:
+            if e.args[1] == "Address already in use":
+              self.port += 1
+            else:
+              raise
+        self.server_thread = threading.Thread(target=self._run_server)
+        self.server_thread.start()
+        self.fs = sftpfs.SFTPFS(("localhost",self.port))
+
+    def _run_server(self):
+        """Run the server, swallowing shutdown-related execptions."""
+        try:
+            self.server.serve_forever()
+        except:
+            pass
 
     def tearDown(self):
+        try:
+            self.server.shutdown()
+            self.fs.exists("/")
+        except Exception:
+            pass
         self.temp_fs.close()
 
 
+####################
+
 from fs.wrappers.xattr import SimulateXAttr
 class TestSimulateXAttr(unittest.TestCase,FSTestCases,XAttrTestCases):