]> git.rkrishnan.org Git - tahoe-lafs/tahoe-lafs.git/blobdiff - src/allmydata/frontends/sftpd.py
SFTP: add a comment about a subtle interaction between OverwriteableFileConsumer...
[tahoe-lafs/tahoe-lafs.git] / src / allmydata / frontends / sftpd.py
index b10f9d14c77c9cdb8a8e8a27c67b52fa112be40c..887786e76f51a54c42c06e2c40d45405f432e077 100644 (file)
@@ -1,5 +1,6 @@
 
-import os, tempfile, heapq, binascii, traceback, array, stat, struct
+import heapq, traceback, array, stat, struct
+from types import NoneType
 from stat import S_IFREG, S_IFDIR
 from time import time, strftime, localtime
 
@@ -29,10 +30,10 @@ from allmydata.util.consumer import download_to_data
 from allmydata.interfaces import IFileNode, IDirectoryNode, ExistingChildError, \
      NoSuchChildError, ChildOfWrongTypeError
 from allmydata.mutable.common import NotWriteableError
+from allmydata.mutable.publish import MutableFileHandle
 from allmydata.immutable.upload import FileHandle
 from allmydata.dirnode import update_metadata
-
-from pycryptopp.cipher.aes import AES
+from allmydata.util.fileutil import EncryptedTemporaryFile
 
 noisy = True
 use_foolscap_logging = True
@@ -77,6 +78,9 @@ def _to_sftp_time(t):
 
 
 def _convert_error(res, request):
+    """If res is not a Failure, return it, otherwise reraise the appropriate
+    SFTPError."""
+
     if not isinstance(res, Failure):
         logged_res = res
         if isinstance(res, str): logged_res = "<data of length %r>" % (len(res),)
@@ -87,11 +91,11 @@ def _convert_error(res, request):
     logmsg("RAISE %r %r" % (request, err.value), level=OPERATIONAL)
     try:
         if noisy: logmsg(traceback.format_exc(err.value), level=NOISY)
-    except:  # pragma: no cover
+    except Exception:  # pragma: no cover
         pass
 
     # The message argument to SFTPError must not reveal information that
-    # might compromise anonymity.
+    # might compromise anonymity, if we are running over an anonymous network.
 
     if err.check(SFTPError):
         # original raiser of SFTPError has responsibility to ensure anonymity
@@ -139,10 +143,10 @@ def _lsLine(name, attrs):
     st_gid = "tahoe"
     st_mtime = attrs.get("mtime", 0)
     st_mode = attrs["permissions"]
-    # TODO: check that clients are okay with this being a "?".
-    # (They should be because the longname is intended for human
-    # consumption.)
-    st_size = attrs.get("size", "?")
+
+    # Some clients won't tolerate '?' in the size field (#1337).
+    st_size = attrs.get("size", 0)
+
     # We don't know how many links there really are to this object.
     st_nlink = 1
 
@@ -191,7 +195,7 @@ def _lsLine(name, attrs):
     return l
 
 
-def _no_write(parent_readonly, child, metadata):
+def _no_write(parent_readonly, child, metadata=None):
     """Whether child should be listed as having read-only permissions in parent."""
 
     if child.is_unknown():
@@ -201,7 +205,7 @@ def _no_write(parent_readonly, child, metadata):
     elif parent_readonly or IDirectoryNode.providedBy(child):
         return True
     else:
-        return metadata.get('no-write', False)
+        return metadata is not None and metadata.get('no-write', False)
 
 
 def _populate_attrs(childnode, metadata, size=None):
@@ -240,16 +244,13 @@ def _populate_attrs(childnode, metadata, size=None):
         # We would prefer to omit atime, but SFTP version 3 can only
         # accept mtime if atime is also set.
         if 'linkmotime' in metadata.get('tahoe', {}):
-            attrs['mtime'] = attrs['atime'] = _to_sftp_time(metadata['tahoe']['linkmotime'])
+            attrs['ctime'] = attrs['mtime'] = attrs['atime'] = _to_sftp_time(metadata['tahoe']['linkmotime'])
         elif 'mtime' in metadata:
-            attrs['mtime'] = attrs['atime'] = _to_sftp_time(metadata['mtime'])
+            attrs['ctime'] = attrs['mtime'] = attrs['atime'] = _to_sftp_time(metadata['mtime'])
 
         if 'linkcrtime' in metadata.get('tahoe', {}):
             attrs['createtime'] = _to_sftp_time(metadata['tahoe']['linkcrtime'])
 
-        if 'ctime' in metadata:
-            attrs['ctime'] = _to_sftp_time(metadata['ctime'])
-
     attrs['permissions'] = perms
 
     # twisted.conch.ssh.filetransfer only implements SFTP version 3,
@@ -275,6 +276,8 @@ def _attrs_to_metadata(attrs):
 
 
 def _direntry_for(filenode_or_parent, childname, filenode=None):
+    assert isinstance(childname, (unicode, NoneType)), childname
+
     if childname is None:
         filenode_or_parent = filenode
 
@@ -288,56 +291,6 @@ def _direntry_for(filenode_or_parent, childname, filenode=None):
     return None
 
 
-class EncryptedTemporaryFile(PrefixingLogMixin):
-    # not implemented: next, readline, readlines, xreadlines, writelines
-
-    def __init__(self):
-        PrefixingLogMixin.__init__(self, facility="tahoe.sftp")
-        self.file = tempfile.TemporaryFile()
-        self.key = os.urandom(16)  # AES-128
-
-    def _crypt(self, offset, data):
-        # TODO: use random-access AES (pycryptopp ticket #18)
-        offset_big = offset // 16
-        offset_small = offset % 16
-        iv = binascii.unhexlify("%032x" % offset_big)
-        cipher = AES(self.key, iv=iv)
-        cipher.process("\x00"*offset_small)
-        return cipher.process(data)
-
-    def close(self):
-        self.file.close()
-
-    def flush(self):
-        self.file.flush()
-
-    def seek(self, offset, whence=os.SEEK_SET):
-        if noisy: self.log(".seek(%r, %r)" % (offset, whence), level=NOISY)
-        self.file.seek(offset, whence)
-
-    def tell(self):
-        offset = self.file.tell()
-        if noisy: self.log(".tell() = %r" % (offset,), level=NOISY)
-        return offset
-
-    def read(self, size=-1):
-        if noisy: self.log(".read(%r)" % (size,), level=NOISY)
-        index = self.file.tell()
-        ciphertext = self.file.read(size)
-        plaintext = self._crypt(index, ciphertext)
-        return plaintext
-
-    def write(self, plaintext):
-        if noisy: self.log(".write(<data of length %r>)" % (len(plaintext),), level=NOISY)
-        index = self.file.tell()
-        ciphertext = self._crypt(index, plaintext)
-        self.file.write(ciphertext)
-
-    def truncate(self, newsize):
-        if noisy: self.log(".truncate(%r)" % (newsize,), level=NOISY)
-        self.file.truncate(newsize)
-
-
 class OverwriteableFileConsumer(PrefixingLogMixin):
     implements(IFinishableConsumer)
     """I act both as a consumer for the download of the original file contents, and as a
@@ -389,14 +342,18 @@ class OverwriteableFileConsumer(PrefixingLogMixin):
             self.overwrite(self.current_size, "\x00" * (size - self.current_size))
         self.current_size = size
 
-        # invariant: self.download_size <= self.current_size
+        # make the invariant self.download_size <= self.current_size be true again
         if size < self.download_size:
             self.download_size = size
+
         if self.downloaded >= self.download_size:
             self.finish()
 
     def registerProducer(self, p, streaming):
         if noisy: self.log(".registerProducer(%r, streaming=%r)" % (p, streaming), level=NOISY)
+        if self.producer is not None:
+            raise RuntimeError("producer is already registered")
+
         self.producer = p
         if streaming:
             # call resumeProducing once to start things off
@@ -470,7 +427,7 @@ class OverwriteableFileConsumer(PrefixingLogMixin):
                 return
             if noisy: self.log("MILESTONE %r %r" % (next, d), level=NOISY)
             heapq.heappop(self.milestones)
-            eventually_callback(d)(None)
+            eventually(d.callback, None)
 
         if milestone >= self.download_size:
             self.finish()
@@ -504,6 +461,12 @@ class OverwriteableFileConsumer(PrefixingLogMixin):
         The caller must perform no more overwrites until the Deferred has fired."""
 
         if noisy: self.log(".read(%r, %r), current_size = %r" % (offset, length, self.current_size), level=NOISY)
+
+        # Note that the overwrite method is synchronous. When a write request is processed
+        # (e.g. a writeChunk request on the async queue of GeneralSFTPFile), overwrite will
+        # be called and will update self.current_size if necessary before returning. Therefore,
+        # self.current_size will be up-to-date for a subsequent call to this read method, and
+        # so it is correct to do the check for a read past the end-of-file here.
         if offset >= self.current_size:
             def _eof(): raise EOFError("read past end of file")
             return defer.execute(_eof)
@@ -543,6 +506,9 @@ class OverwriteableFileConsumer(PrefixingLogMixin):
         return self.done
 
     def finish(self):
+        """Called by the producer when it has finished producing, or when we have
+        received enough bytes, or as a result of a close. Defined by IFinishableConsumer."""
+
         while len(self.milestones) > 0:
             (next, d) = self.milestones[0]
             if noisy: self.log("MILESTONE FINISH %r %r" % (next, d), level=NOISY)
@@ -550,24 +516,19 @@ class OverwriteableFileConsumer(PrefixingLogMixin):
             # The callback means that the milestone has been reached if
             # it is ever going to be. Note that the file may have been
             # truncated to before the milestone.
-            eventually_callback(d)(None)
-
-        # FIXME: causes spurious failures
-        #self.unregisterProducer()
+            eventually(d.callback, None)
 
     def close(self):
         if not self.is_closed:
             self.is_closed = True
             try:
                 self.f.close()
-            except BaseException as e:
+            except Exception, e:
                 self.log("suppressed %r from close of temporary file %r" % (e, self.f), level=WEIRD)
         self.finish()
 
     def unregisterProducer(self):
-        if self.producer:
-            self.producer.stopProducing()
-            self.producer = None
+        pass
 
 
 SIZE_THRESHOLD = 1000
@@ -577,13 +538,14 @@ class ShortReadOnlySFTPFile(PrefixingLogMixin):
     implements(ISFTPFile)
     """I represent a file handle to a particular file on an SFTP connection.
     I am used only for short immutable files opened in read-only mode.
-    The file contents are downloaded to memory when I am created."""
+    When I am created, the file contents start to be downloaded to memory.
+    self.async is used to delay read requests until the download has finished."""
 
     def __init__(self, userpath, filenode, metadata):
         PrefixingLogMixin.__init__(self, facility="tahoe.sftp", prefix=userpath)
         if noisy: self.log(".__init__(%r, %r, %r)" % (userpath, filenode, metadata), level=NOISY)
 
-        assert IFileNode.providedBy(filenode), filenode
+        assert isinstance(userpath, str) and IFileNode.providedBy(filenode), (userpath, filenode)
         self.filenode = filenode
         self.metadata = metadata
         self.async = download_to_data(filenode)
@@ -611,9 +573,9 @@ class ShortReadOnlySFTPFile(PrefixingLogMixin):
             # i.e. we respond with an EOF error iff offset is already at EOF.
 
             if offset >= len(data):
-                eventually_errback(d)(SFTPError(FX_EOF, "read at or past end of file"))
+                eventually(d.errback, SFTPError(FX_EOF, "read at or past end of file"))
             else:
-                eventually_callback(d)(data[offset:min(offset+length, len(data))])
+                eventually(d.callback, data[offset:offset+length])  # truncated if offset+length > len(data)
             return data
         self.async.addCallbacks(_read, eventually_errback(d))
         d.addBoth(_convert_error, request)
@@ -666,6 +628,7 @@ class GeneralSFTPFile(PrefixingLogMixin):
         if noisy: self.log(".__init__(%r, %r = %r, %r, <convergence censored>)" %
                            (userpath, flags, _repr_flags(flags), close_notify), level=NOISY)
 
+        assert isinstance(userpath, str), userpath
         self.userpath = userpath
         self.flags = flags
         self.close_notify = close_notify
@@ -688,6 +651,7 @@ class GeneralSFTPFile(PrefixingLogMixin):
         self.log(".open(parent=%r, childname=%r, filenode=%r, metadata=%r)" %
                  (parent, childname, filenode, metadata), level=OPERATIONAL)
 
+        assert isinstance(childname, (unicode, NoneType)), childname
         # If the file has been renamed, the new (parent, childname) takes precedence.
         if self.parent is None:
             self.parent = parent
@@ -696,7 +660,7 @@ class GeneralSFTPFile(PrefixingLogMixin):
         self.filenode = filenode
         self.metadata = metadata
 
-        assert not self.closed
+        assert not self.closed, self
         tempfile_maker = EncryptedTemporaryFile
 
         if (self.flags & FXF_TRUNC) or not filenode:
@@ -706,32 +670,33 @@ class GeneralSFTPFile(PrefixingLogMixin):
         else:
             assert IFileNode.providedBy(filenode), filenode
 
-            # TODO: use download interface described in #993 when implemented.
-            if filenode.is_mutable():
-                self.async.addCallback(lambda ign: filenode.download_best_version())
-                def _downloaded(data):
-                    self.consumer = OverwriteableFileConsumer(len(data), tempfile_maker)
-                    self.consumer.write(data)
-                    self.consumer.finish()
-                    return None
-                self.async.addCallback(_downloaded)
-            else:
-                download_size = filenode.get_size()
-                assert download_size is not None, "download_size is None"
+            self.async.addCallback(lambda ignored: filenode.get_best_readable_version())
+
+            def _read(version):
+                if noisy: self.log("_read", level=NOISY)
+                download_size = version.get_size()
+                assert download_size is not None
+
                 self.consumer = OverwriteableFileConsumer(download_size, tempfile_maker)
-                def _read(ign):
-                    if noisy: self.log("_read immutable", level=NOISY)
-                    filenode.read(self.consumer, 0, None)
-                self.async.addCallback(_read)
 
-        eventually_callback(self.async)(None)
+                version.read(self.consumer, 0, None)
+            self.async.addCallback(_read)
+
+        eventually(self.async.callback, None)
 
         if noisy: self.log("open done", level=NOISY)
         return self
 
+    def get_userpath(self):
+        return self.userpath
+
+    def get_direntry(self):
+        return _direntry_for(self.parent, self.childname)
+
     def rename(self, new_userpath, new_parent, new_childname):
         self.log(".rename(%r, %r, %r)" % (new_userpath, new_parent, new_childname), level=OPERATIONAL)
 
+        assert isinstance(new_userpath, str) and isinstance(new_childname, unicode), (new_userpath, new_childname)
         self.userpath = new_userpath
         self.parent = new_parent
         self.childname = new_childname
@@ -753,9 +718,6 @@ class GeneralSFTPFile(PrefixingLogMixin):
         d.addBoth(_done)
         return d
 
-    def get_metadata(self):
-        return self.metadata
-
     def readChunk(self, offset, length):
         request = ".readChunk(%r, %r)" % (offset, length)
         self.log(request, level=OPERATIONAL)
@@ -839,7 +801,7 @@ class GeneralSFTPFile(PrefixingLogMixin):
         abandoned = self.abandoned
         parent = self.parent
         childname = self.childname
-        
+
         # has_changed is set when writeChunk is called, not when the write occurs, so
         # it is correct to optimize out the commit if it is False at the close call.
         has_changed = self.has_changed
@@ -857,14 +819,12 @@ class GeneralSFTPFile(PrefixingLogMixin):
         def _close(ign):
             d2 = self.consumer.when_done()
             if self.filenode and self.filenode.is_mutable():
-                self.log("update mutable file %r childname=%r" % (self.filenode, childname), level=OPERATIONAL)
+                self.log("update mutable file %r childname=%r metadata=%r" % (self.filenode, childname, self.metadata), level=OPERATIONAL)
                 if self.metadata.get('no-write', False) and not self.filenode.is_readonly():
                     assert parent and childname, (parent, childname, self.metadata)
                     d2.addCallback(lambda ign: parent.set_metadata_for(childname, self.metadata))
 
-                d2.addCallback(lambda ign: self.consumer.get_current_size())
-                d2.addCallback(lambda size: self.consumer.read(0, size))
-                d2.addCallback(lambda new_contents: self.filenode.overwrite(new_contents))
+                d2.addCallback(lambda ign: self.filenode.overwrite(MutableFileHandle(self.consumer.get_file())))
             else:
                 def _add_file(ign):
                     self.log("_add_file childname=%r" % (childname,), level=OPERATIONAL)
@@ -908,7 +868,7 @@ class GeneralSFTPFile(PrefixingLogMixin):
 
             # self.filenode might be None, but that's ok.
             attrs = _populate_attrs(self.filenode, self.metadata, size=self.consumer.get_current_size())
-            eventually_callback(d)(attrs)
+            eventually(d.callback, attrs)
             return None
         self.async.addCallbacks(_get, eventually_errback(d))
         d.addBoth(_convert_error, request)
@@ -934,14 +894,19 @@ class GeneralSFTPFile(PrefixingLogMixin):
         d = defer.Deferred()
         def _set(ign):
             if noisy: self.log("_set(%r) in %r" % (ign, request), level=NOISY)
-            if only_if_at and only_if_at != _direntry_for(self.parent, self.childname, self.filenode):
+            current_direntry = _direntry_for(self.parent, self.childname, self.filenode)
+            if only_if_at and only_if_at != current_direntry:
+                if noisy: self.log("not setting attributes: current_direntry=%r in %r" %
+                                   (current_direntry, request), level=NOISY)
                 return None
 
             now = time()
             self.metadata = update_metadata(self.metadata, _attrs_to_metadata(attrs), now)
             if size is not None:
+                # TODO: should we refuse to truncate a file opened with FXF_APPEND?
+                # <http://allmydata.org/trac/tahoe-lafs/ticket/1037#comment:20>
                 self.consumer.set_current_size(size)
-            eventually_callback(d)(None)
+            eventually(d.callback, None)
             return None
         self.async.addCallbacks(_set, eventually_errback(d))
         d.addBoth(_convert_error, request)
@@ -974,6 +939,9 @@ class Reason:
 
 all_heisenfiles = {}
 
+def _reload():
+    global all_heisenfiles
+    all_heisenfiles = {}
 
 class SFTPUserHandler(ConchUser, PrefixingLogMixin):
     implements(ISFTPServer)
@@ -1010,27 +978,31 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
             for f in files:
                 f.abandon()
 
-    def _add_heisenfiles_by_path(self, userpath, files_to_add):
-        self.log("._add_heisenfiles_by_path(%r, %r)" % (userpath, files_to_add), level=OPERATIONAL)
+    def _add_heisenfile_by_path(self, file):
+        self.log("._add_heisenfile_by_path(%r)" % (file,), level=OPERATIONAL)
 
+        userpath = file.get_userpath()
         if userpath in self._heisenfiles:
-            self._heisenfiles[userpath] += files_to_add
+            self._heisenfiles[userpath] += [file]
         else:
-            self._heisenfiles[userpath] = files_to_add
+            self._heisenfiles[userpath] = [file]
 
-    def _add_heisenfiles_by_direntry(self, direntry, files_to_add):
-        self.log("._add_heisenfiles_by_direntry(%r, %r)" % (direntry, files_to_add), level=OPERATIONAL)
+    def _add_heisenfile_by_direntry(self, file):
+        self.log("._add_heisenfile_by_direntry(%r)" % (file,), level=OPERATIONAL)
 
+        direntry = file.get_direntry()
         if direntry:
             if direntry in all_heisenfiles:
-                all_heisenfiles[direntry] += files_to_add
+                all_heisenfiles[direntry] += [file]
             else:
-                all_heisenfiles[direntry] = files_to_add
+                all_heisenfiles[direntry] = [file]
 
     def _abandon_any_heisenfiles(self, userpath, direntry):
         request = "._abandon_any_heisenfiles(%r, %r)" % (userpath, direntry)
         self.log(request, level=OPERATIONAL)
 
+        assert isinstance(userpath, str), userpath
+
         # First we synchronously mark all heisenfiles matching the userpath or direntry
         # as abandoned, and remove them from the two heisenfile dicts. Then we .sync()
         # each file that we abandoned.
@@ -1078,6 +1050,12 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
                    (from_userpath, from_parent, from_childname, to_userpath, to_parent, to_childname, overwrite))
         self.log(request, level=OPERATIONAL)
 
+        assert (isinstance(from_userpath, str) and isinstance(from_childname, unicode) and
+                isinstance(to_userpath, str) and isinstance(to_childname, unicode)), \
+               (from_userpath, from_childname, to_userpath, to_childname)
+
+        if noisy: self.log("all_heisenfiles = %r\nself._heisenfiles = %r" % (all_heisenfiles, self._heisenfiles), level=NOISY)
+
         # First we synchronously rename all heisenfiles matching the userpath or direntry.
         # Then we .sync() each file that we renamed.
         #
@@ -1107,8 +1085,12 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
         from_direntry = _direntry_for(from_parent, from_childname)
         to_direntry = _direntry_for(to_parent, to_childname)
 
+        if noisy: self.log("from_direntry = %r, to_direntry = %r, len(all_heisenfiles) = %r, len(self._heisenfiles) = %r in %r" %
+                           (from_direntry, to_direntry, len(all_heisenfiles), len(self._heisenfiles), request), level=NOISY)
+
         if not overwrite and (to_userpath in self._heisenfiles or to_direntry in all_heisenfiles):
             def _existing(): raise SFTPError(FX_PERMISSION_DENIED, "cannot rename to existing path " + to_userpath)
+            if noisy: self.log("existing", level=NOISY)
             return defer.execute(_existing)
 
         from_files = []
@@ -1121,18 +1103,18 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
 
         if noisy: self.log("from_files = %r in %r" % (from_files, request), level=NOISY)
 
-        self._add_heisenfiles_by_direntry(to_direntry, from_files)
-        self._add_heisenfiles_by_path(to_userpath, from_files)
-
         for f in from_files:
             f.rename(to_userpath, to_parent, to_childname)
+            self._add_heisenfile_by_path(f)
+            self._add_heisenfile_by_direntry(f)
 
         d = defer.succeed(None)
         for f in from_files:
             d.addBoth(f.sync)
 
         def _done(ign):
-            self.log("done %r" % (request,), level=OPERATIONAL)
+            if noisy: self.log("done: len(all_heisenfiles) = %r, len(self._heisenfiles) = %r in %r" %
+                               (len(all_heisenfiles), len(self._heisenfiles), request), level=NOISY)
             return len(from_files) > 0
         d.addBoth(_done)
         return d
@@ -1141,6 +1123,8 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
         request = "._update_attrs_for_heisenfiles(%r, %r, %r)" % (userpath, direntry, attrs)
         self.log(request, level=OPERATIONAL)
 
+        assert isinstance(userpath, str) and isinstance(direntry, str), (userpath, direntry)
+
         files = []
         if direntry in all_heisenfiles:
             files = all_heisenfiles[direntry]
@@ -1162,6 +1146,7 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
 
         def _done(ign):
             self.log("done %r" % (request,), level=OPERATIONAL)
+            # TODO: this should not return True if only_if_at caused all files to be skipped.
             return len(files) > 0
         d.addBoth(_done)
         return d
@@ -1170,6 +1155,8 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
         request = "._sync_heisenfiles(%r, %r, ignore=%r)" % (userpath, direntry, ignore)
         self.log(request, level=OPERATIONAL)
 
+        assert isinstance(userpath, str) and isinstance(direntry, (str, NoneType)), (userpath, direntry)
+
         files = []
         if direntry in all_heisenfiles:
             files = all_heisenfiles[direntry]
@@ -1192,6 +1179,8 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
     def _remove_heisenfile(self, userpath, parent, childname, file_to_remove):
         if noisy: self.log("._remove_heisenfile(%r, %r, %r, %r)" % (userpath, parent, childname, file_to_remove), level=NOISY)
 
+        assert isinstance(userpath, str) and isinstance(childname, (unicode, NoneType)), (userpath, childname)
+
         direntry = _direntry_for(parent, childname)
         if direntry in all_heisenfiles:
             all_old_files = all_heisenfiles[direntry]
@@ -1209,12 +1198,15 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
             else:
                 del self._heisenfiles[userpath]
 
+        if noisy: self.log("all_heisenfiles = %r\nself._heisenfiles = %r" % (all_heisenfiles, self._heisenfiles), level=NOISY)
+
     def _make_file(self, existing_file, userpath, flags, parent=None, childname=None, filenode=None, metadata=None):
         if noisy: self.log("._make_file(%r, %r, %r = %r, parent=%r, childname=%r, filenode=%r, metadata=%r)" %
                            (existing_file, userpath, flags, _repr_flags(flags), parent, childname, filenode, metadata),
                            level=NOISY)
 
-        assert metadata is None or 'no-write' in metadata, metadata
+        assert (isinstance(userpath, str) and isinstance(childname, (unicode, NoneType)) and
+                (metadata is None or 'no-write' in metadata)), (userpath, childname, metadata)
 
         writing = (flags & (FXF_WRITE | FXF_CREAT)) != 0
         direntry = _direntry_for(parent, childname, filenode)
@@ -1232,7 +1224,7 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
             def _got_file(file):
                 file.open(parent=parent, childname=childname, filenode=filenode, metadata=metadata)
                 if writing:
-                    self._add_heisenfiles_by_direntry(direntry, [file])
+                    self._add_heisenfile_by_direntry(file)
                 return file
             d.addCallback(_got_file)
         return d
@@ -1273,7 +1265,7 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
 
         if flags & (FXF_WRITE | FXF_CREAT):
             file = GeneralSFTPFile(userpath, flags, self._remove_heisenfile, self._convergence)
-            self._add_heisenfiles_by_path(userpath, [file])
+            self._add_heisenfile_by_path(file)
         else:
             # We haven't decided which file implementation to use yet.
             file = None
@@ -1326,10 +1318,6 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
                 if (flags & FXF_WRITE) and root.is_readonly():
                     raise SFTPError(FX_PERMISSION_DENIED,
                                     "cannot write to a non-writeable filecap without a parent directory")
-                if (flags & FXF_WRITE) and root.is_mutable() and desired_metadata.get('no-write', False):
-                    raise SFTPError(FX_PERMISSION_DENIED,
-                                    "cannot write to a mutable filecap without a parent directory, when the "
-                                    "specified permissions would require the link from the parent to be made read-only")
                 if flags & FXF_EXCL:
                     raise SFTPError(FX_FAILURE,
                                     "cannot create a file exclusively when it already exists")
@@ -1346,7 +1334,7 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
                 # reported as r--r--r--, which is appropriate because an immutable file can't be
                 # written via this path.
 
-                metadata['no-write'] = _no_write(True, root, metadata)
+                metadata['no-write'] = _no_write(True, root)
                 return self._make_file(file, userpath, flags, filenode=root, metadata=metadata)
             else:
                 # case 2
@@ -1398,7 +1386,10 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
                         if noisy: self.log("_got_child( (%r, %r) )" % (filenode, current_metadata), level=NOISY)
 
                         metadata = update_metadata(current_metadata, desired_metadata, time())
-                        metadata['no-write'] = _no_write(parent_readonly, filenode, metadata)
+
+                        # Ignore the permissions of the desired_metadata in an open call. The permissions
+                        # can only be set by setAttrs.
+                        metadata['no-write'] = _no_write(parent_readonly, filenode, current_metadata)
 
                         if filenode.is_unknown():
                             raise SFTPError(FX_PERMISSION_DENIED,
@@ -1471,17 +1462,17 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
             # For the standard SSH_FXP_RENAME operation, overwrite=False.
             # We also support the posix-rename@openssh.com extension, which uses overwrite=True.
 
-            d2 = defer.fail(NoSuchChildError())
+            d2 = defer.succeed(None)
             if not overwrite:
                 d2.addCallback(lambda ign: to_parent.get(to_childname))
-            def _expect_fail(res):
-                if not isinstance(res, Failure):
-                    raise SFTPError(FX_PERMISSION_DENIED, "cannot rename to existing path " + to_userpath)
+                def _expect_fail(res):
+                    if not isinstance(res, Failure):
+                        raise SFTPError(FX_PERMISSION_DENIED, "cannot rename to existing path " + to_userpath)
 
-                # It is OK if we fail for errors other than NoSuchChildError, since that probably
-                # indicates some problem accessing the destination directory.
-                res.trap(NoSuchChildError)
-            d2.addBoth(_expect_fail)
+                    # It is OK if we fail for errors other than NoSuchChildError, since that probably
+                    # indicates some problem accessing the destination directory.
+                    res.trap(NoSuchChildError)
+                d2.addBoth(_expect_fail)
 
             # If there are heisenfiles to be written at the 'from' direntry, then ensure
             # they will now be written at the 'to' direntry instead.
@@ -1708,7 +1699,14 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
                     desired_metadata = _attrs_to_metadata(attrs)
                     if noisy: self.log("desired_metadata = %r" % (desired_metadata,), level=NOISY)
 
-                    return parent_or_node.set_metadata_for(childname, desired_metadata)
+                    d3 = parent_or_node.set_metadata_for(childname, desired_metadata)
+                    def _nosuch(err):
+                        if updated_heisenfiles:
+                            err.trap(NoSuchChildError)
+                        else:
+                            return err
+                    d3.addErrback(_nosuch)
+                    return d3
             d2.addCallback(_update)
             d2.addCallback(lambda ign: None)
             return d2
@@ -1740,6 +1738,7 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
         if extensionName == 'posix-rename@openssh.com':
             def _bad(): raise SFTPError(FX_BAD_MESSAGE, "could not parse posix-rename@openssh.com request")
 
+            if 4 > len(extensionData): return defer.execute(_bad)
             (fromPathLen,) = struct.unpack('>L', extensionData[0:4])
             if 8 + fromPathLen > len(extensionData): return defer.execute(_bad)
 
@@ -1789,6 +1788,8 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
     def _path_from_string(self, pathstring):
         if noisy: self.log("CONVERT %r" % (pathstring,), level=NOISY)
 
+        assert isinstance(pathstring, str), pathstring
+
         # The home directory is the root directory.
         pathstring = pathstring.strip("/")
         if pathstring == "" or pathstring == ".":
@@ -1839,48 +1840,76 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
         return d
 
 
-class SFTPUser(ConchUser, PrefixingLogMixin):
-    implements(ISession)
-    def __init__(self, check_abort, client, rootnode, username, convergence):
-        ConchUser.__init__(self)
-        PrefixingLogMixin.__init__(self, facility="tahoe.sftp")
+class FakeTransport:
+    implements(ITransport)
+    def write(self, data):
+        logmsg("FakeTransport.write(<data of length %r>)" % (len(data),), level=NOISY)
 
-        self.channelLookup["session"] = session.SSHSession
-        self.subsystemLookup["sftp"] = FileTransferServer
+    def writeSequence(self, data):
+        logmsg("FakeTransport.writeSequence(...)", level=NOISY)
+
+    def loseConnection(self):
+        logmsg("FakeTransport.loseConnection()", level=NOISY)
+
+    # getPeer and getHost can just raise errors, since we don't know what to return
 
-        self.check_abort = check_abort
-        self.client = client
-        self.root = rootnode
-        self.username = username
-        self.convergence = convergence
+
+class ShellSession(PrefixingLogMixin):
+    implements(ISession)
+    def __init__(self, userHandler):
+        PrefixingLogMixin.__init__(self, facility="tahoe.sftp")
+        if noisy: self.log(".__init__(%r)" % (userHandler), level=NOISY)
 
     def getPty(self, terminal, windowSize, attrs):
         self.log(".getPty(%r, %r, %r)" % (terminal, windowSize, attrs), level=OPERATIONAL)
-        raise NotImplementedError
 
     def openShell(self, protocol):
         self.log(".openShell(%r)" % (protocol,), level=OPERATIONAL)
-        raise NotImplementedError
+        if hasattr(protocol, 'transport') and protocol.transport is None:
+            protocol.transport = FakeTransport()  # work around Twisted bug
+
+        return self._unsupported(protocol)
 
     def execCommand(self, protocol, cmd):
         self.log(".execCommand(%r, %r)" % (protocol, cmd), level=OPERATIONAL)
-        raise NotImplementedError
+        if hasattr(protocol, 'transport') and protocol.transport is None:
+            protocol.transport = FakeTransport()  # work around Twisted bug
+
+        d = defer.succeed(None)
+        if cmd == "df -P -k /":
+            d.addCallback(lambda ign: protocol.write(
+                          "Filesystem         1024-blocks      Used Available Capacity Mounted on\r\n"
+                          "tahoe                628318530 314159265 314159265      50% /\r\n"))
+            d.addCallback(lambda ign: protocol.processEnded(Reason(ProcessDone(None))))
+        else:
+            d.addCallback(lambda ign: self._unsupported(protocol))
+        return d
+
+    def _unsupported(self, protocol):
+        d = defer.succeed(None)
+        d.addCallback(lambda ign: protocol.errReceived(
+                      "This server supports only the SFTP protocol. It does not support SCP,\r\n"
+                      "interactive shell sessions, or commands other than one needed by sshfs.\r\n"))
+        d.addCallback(lambda ign: protocol.processEnded(Reason(ProcessTerminated(exitCode=1))))
+        return d
 
     def windowChanged(self, newWindowSize):
         self.log(".windowChanged(%r)" % (newWindowSize,), level=OPERATIONAL)
 
-    def eofReceived():
+    def eofReceived(self):
         self.log(".eofReceived()", level=OPERATIONAL)
 
     def closed(self):
         self.log(".closed()", level=OPERATIONAL)
 
 
-# if you have an SFTPUser, and you want something that provides ISFTPServer,
-# then you get SFTPHandler(user)
-components.registerAdapter(SFTPHandler, SFTPUser, ISFTPServer)
+# If you have an SFTPUserHandler and want something that provides ISession, you get
+# ShellSession(userHandler).
+# We use adaptation because this must be a different object to the SFTPUserHandler.
+components.registerAdapter(ShellSession, SFTPUserHandler, ISession)
+
 
-from auth import AccountURLChecker, AccountFileChecker, NeedRootcapLookupScheme
+from allmydata.frontends.auth import AccountURLChecker, AccountFileChecker, NeedRootcapLookupScheme
 
 class Dispatcher:
     implements(portal.IRealm)