SFTP: improve test coverage. Also make creating a directory fail when permissions...

diff --git a/src/allmydata/frontends/sftpd.py b/src/allmydata/frontends/sftpd.py
index a624dddcb3bae1922e039f8abc2cf78c9b59083c..14ff9d53e4c1d5924b077d37c84c81f293833841 100644 (file)
--- a/src/allmydata/frontends/sftpd.py
+++ b/src/allmydata/frontends/sftpd.py
@@ -1315,7 +1315,7 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
         def _got_root( (root, path) ):
             if root.is_unknown():
                 raise SFTPError(FX_PERMISSION_DENIED,
-                                "cannot open an unknown cap (or child of an unknown directory). "
+                                "cannot open an unknown cap (or child of an unknown object). "
                                 "Upgrading the gateway to a later Tahoe-LAFS version may help")
             if not path:
                 # case 1
@@ -1359,7 +1359,7 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
                     if noisy: self.log("_got_parent(%r)" % (parent,), level=NOISY)
                     if parent.is_unknown():
                         raise SFTPError(FX_PERMISSION_DENIED,
-                                        "cannot open an unknown cap (or child of an unknown directory). "
+                                        "cannot open a child of an unknown object. "
                                         "Upgrading the gateway to a later Tahoe-LAFS version may help")
 
                     parent_readonly = parent.is_readonly()
@@ -1407,15 +1407,9 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
                         if not IFileNode.providedBy(filenode):
                             raise SFTPError(FX_PERMISSION_DENIED,
                                             "cannot open a directory as if it were a file")
-                        if (flags & FXF_WRITE) and filenode.is_mutable() and filenode.is_readonly():
+                        if (flags & FXF_WRITE) and metadata['no-write']:
                             raise SFTPError(FX_PERMISSION_DENIED,
-                                            "cannot open a read-only mutable file for writing")
-                        if (flags & FXF_WRITE) and not filenode.is_mutable() and metadata['no-write']:
-                            raise SFTPError(FX_PERMISSION_DENIED,
-                                            "cannot open an immutable file entry with the no-write flag for writing")
-                        if (flags & FXF_WRITE) and parent_readonly:
-                            raise SFTPError(FX_PERMISSION_DENIED,
-                                            "cannot open a file for writing when the parent directory is read-only")
+                                            "cannot open a non-writeable file for writing")
 
                         return self._make_file(file, userpath, flags, parent=parent, childname=childname,
                                                filenode=filenode, metadata=metadata)
@@ -1525,7 +1519,8 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
         path = self._path_from_string(pathstring)
         metadata = _attrs_to_metadata(attrs)
         if 'no-write' in metadata:
-            del metadata['no-write']
+            def _denied(): raise SFTPError(FX_PERMISSION_DENIED, "cannot create a directory that is initially read-only")
+            return defer.execute(_denied)
 
         d = self._get_root(path)
         d.addCallback(lambda (root, path):
@@ -1764,6 +1759,7 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin):
             return d
 
         if extensionName == 'statvfs@openssh.com' or extensionName == 'fstatvfs@openssh.com':
+            # f_bsize and f_frsize should be the same to avoid a bug in 'df'
             return defer.succeed(struct.pack('>11Q',
                 1024,         # uint64  f_bsize     /* file system block size */
                 1024,         # uint64  f_frsize    /* fundamental fs block size */

diff --git a/src/allmydata/test/test_sftp.py b/src/allmydata/test/test_sftp.py
index 3c81f6145b8b10c41eee2ff1be5e5d2cfcc00979..2679c74a7073ec6c442fbda88ce9e89fee73c55f 100644 (file)
--- a/src/allmydata/test/test_sftp.py
+++ b/src/allmydata/test/test_sftp.py
@@ -339,20 +339,23 @@ class Handler(GridTestMixin, ShouldFailMixin, unittest.TestCase):
         d.addCallback(lambda ign: self._set_up_tree())
 
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_BAD_MESSAGE, "openFile small 0",
+            self.shouldFailWithSFTPError(sftp.FX_BAD_MESSAGE, "openFile small 0 bad",
                                          self.handler.openFile, "small", 0, {}))
 
         # attempting to open a non-existent file should fail
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_NO_SUCH_FILE, "openFile nofile READ",
+            self.shouldFailWithSFTPError(sftp.FX_NO_SUCH_FILE, "openFile nofile READ nosuch",
                                          self.handler.openFile, "nofile", sftp.FXF_READ, {}))
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_NO_SUCH_FILE, "openFile nodir/file READ",
+            self.shouldFailWithSFTPError(sftp.FX_NO_SUCH_FILE, "openFile nodir/file READ nosuch",
                                          self.handler.openFile, "nodir/file", sftp.FXF_READ, {}))
 
         d.addCallback(lambda ign:
             self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile unknown READ denied",
                                          self.handler.openFile, "unknown", sftp.FXF_READ, {}))
+        d.addCallback(lambda ign:
+            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile unknown/file READ denied",
+                                         self.handler.openFile, "unknown/file", sftp.FXF_READ, {}))
         d.addCallback(lambda ign:
             self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile tiny_lit_dir READ denied",
                                          self.handler.openFile, "tiny_lit_dir", sftp.FXF_READ, {}))
@@ -411,10 +414,10 @@ class Handler(GridTestMixin, ShouldFailMixin, unittest.TestCase):
             d2.addCallback(lambda ign: rf.close())
 
             d2.addCallback(lambda ign:
-                self.shouldFailWithSFTPError(sftp.FX_BAD_MESSAGE, "readChunk on closed file",
+                self.shouldFailWithSFTPError(sftp.FX_BAD_MESSAGE, "readChunk on closed file bad",
                                              rf.readChunk, 0, 1))
             d2.addCallback(lambda ign:
-                self.shouldFailWithSFTPError(sftp.FX_BAD_MESSAGE, "getAttrs on closed file",
+                self.shouldFailWithSFTPError(sftp.FX_BAD_MESSAGE, "getAttrs on closed file bad",
                                              rf.getAttrs))
 
             d2.addCallback(lambda ign: rf.close()) # should be no-op
@@ -517,88 +520,100 @@ class Handler(GridTestMixin, ShouldFailMixin, unittest.TestCase):
 
         # '' is an invalid filename
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_NO_SUCH_FILE, "openFile '' WRITE|CREAT|TRUNC",
+            self.shouldFailWithSFTPError(sftp.FX_NO_SUCH_FILE, "openFile '' WRITE|CREAT|TRUNC nosuch",
                                          self.handler.openFile, "", sftp.FXF_WRITE | sftp.FXF_CREAT | sftp.FXF_TRUNC, {}))
 
         # TRUNC is not valid without CREAT if the file does not already exist
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_NO_SUCH_FILE, "openFile newfile WRITE|TRUNC",
+            self.shouldFailWithSFTPError(sftp.FX_NO_SUCH_FILE, "openFile newfile WRITE|TRUNC nosuch",
                                          self.handler.openFile, "newfile", sftp.FXF_WRITE | sftp.FXF_TRUNC, {}))
 
         # EXCL is not valid without CREAT
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_BAD_MESSAGE, "openFile small WRITE|EXCL",
+            self.shouldFailWithSFTPError(sftp.FX_BAD_MESSAGE, "openFile small WRITE|EXCL bad",
                                          self.handler.openFile, "small", sftp.FXF_WRITE | sftp.FXF_EXCL, {}))
 
         # cannot write to an existing directory
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile tiny_lit_dir WRITE",
+            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile tiny_lit_dir WRITE denied",
                                          self.handler.openFile, "tiny_lit_dir", sftp.FXF_WRITE, {}))
 
         # cannot write to an existing unknown
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile unknown WRITE",
+            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile unknown WRITE denied",
                                          self.handler.openFile, "unknown", sftp.FXF_WRITE, {}))
 
+        # cannot create a child of an unknown
+        d.addCallback(lambda ign:
+            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile unknown/newfile WRITE|CREAT denied",
+                                         self.handler.openFile, "unknown/newfile",
+                                         sftp.FXF_WRITE | sftp.FXF_CREAT, {}))
+
         # cannot write to a new file in an immutable directory
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile tiny_lit_dir/newfile WRITE|CREAT|TRUNC",
+            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile tiny_lit_dir/newfile WRITE|CREAT|TRUNC denied",
                                          self.handler.openFile, "tiny_lit_dir/newfile",
                                          sftp.FXF_WRITE | sftp.FXF_CREAT | sftp.FXF_TRUNC, {}))
 
         # cannot write to an existing immutable file in an immutable directory (with or without CREAT and EXCL)
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile tiny_lit_dir/short WRITE",
+            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile tiny_lit_dir/short WRITE denied",
                                          self.handler.openFile, "tiny_lit_dir/short", sftp.FXF_WRITE, {}))
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile tiny_lit_dir/short WRITE|CREAT",
+            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile tiny_lit_dir/short WRITE|CREAT denied",
                                          self.handler.openFile, "tiny_lit_dir/short",
                                          sftp.FXF_WRITE | sftp.FXF_CREAT, {}))
 
         # cannot write to a mutable file via a readonly cap (by path or uri)
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile readonly WRITE",
+            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile readonly WRITE denied",
                                          self.handler.openFile, "readonly", sftp.FXF_WRITE, {}))
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile readonly uri WRITE",
+            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile readonly uri WRITE denied",
                                          self.handler.openFile, "uri/"+self.readonly_uri, sftp.FXF_WRITE, {}))
 
+        # cannot write to a mutable file by uri when no-write permissions are specified
+        d.addCallback(lambda ign:
+            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile mutable uri permissions:0444 WRITE denied",
+                                         self.handler.openFile, "uri/"+self.mutable_uri, sftp.FXF_WRITE,
+                                         {'permissions': 0444}))
+
         # cannot create a file with the EXCL flag if it already exists
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_FAILURE, "openFile small WRITE|CREAT|EXCL",
+            self.shouldFailWithSFTPError(sftp.FX_FAILURE, "openFile small WRITE|CREAT|EXCL failure",
                                          self.handler.openFile, "small",
                                          sftp.FXF_WRITE | sftp.FXF_CREAT | sftp.FXF_EXCL, {}))
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_FAILURE, "openFile mutable WRITE|CREAT|EXCL",
+            self.shouldFailWithSFTPError(sftp.FX_FAILURE, "openFile mutable WRITE|CREAT|EXCL failure",
                                          self.handler.openFile, "mutable",
                                          sftp.FXF_WRITE | sftp.FXF_CREAT | sftp.FXF_EXCL, {}))
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_FAILURE, "openFile mutable uri WRITE|CREAT|EXCL",
+            self.shouldFailWithSFTPError(sftp.FX_FAILURE, "openFile mutable uri WRITE|CREAT|EXCL failure",
                                          self.handler.openFile, "uri/"+self.mutable_uri,
                                          sftp.FXF_WRITE | sftp.FXF_CREAT | sftp.FXF_EXCL, {}))
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_FAILURE, "openFile tiny_lit_dir/short WRITE|CREAT|EXCL",
+            self.shouldFailWithSFTPError(sftp.FX_FAILURE, "openFile tiny_lit_dir/short WRITE|CREAT|EXCL failure",
                                          self.handler.openFile, "tiny_lit_dir/short",
                                          sftp.FXF_WRITE | sftp.FXF_CREAT | sftp.FXF_EXCL, {}))
 
         # cannot write to an immutable file if we don't have its parent (with or without CREAT, TRUNC, or EXCL)
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile small uri WRITE",
+            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile small uri WRITE denied",
                                          self.handler.openFile, "uri/"+self.small_uri, sftp.FXF_WRITE, {}))
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile small uri WRITE|CREAT",
+            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile small uri WRITE|CREAT denied",
                                          self.handler.openFile, "uri/"+self.small_uri,
                                          sftp.FXF_WRITE | sftp.FXF_CREAT, {}))
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile small uri WRITE|CREAT|TRUNC",
+            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile small uri WRITE|CREAT|TRUNC denied",
                                          self.handler.openFile, "uri/"+self.small_uri,
                                          sftp.FXF_WRITE | sftp.FXF_CREAT | sftp.FXF_TRUNC, {}))
         d.addCallback(lambda ign:
-            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile small uri WRITE|CREAT|EXCL",
+            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "openFile small uri WRITE|CREAT|EXCL denied",
                                          self.handler.openFile, "uri/"+self.small_uri,
                                          sftp.FXF_WRITE | sftp.FXF_CREAT | sftp.FXF_EXCL, {}))
 
-        # test creating a new file with truncation
+        # test creating a new file with truncation and extension
         d.addCallback(lambda ign:
                       self.handler.openFile("newfile", sftp.FXF_WRITE | sftp.FXF_CREAT | sftp.FXF_TRUNC, {}))
         def _write(wf):
@@ -617,13 +632,21 @@ class Handler(GridTestMixin, ShouldFailMixin, unittest.TestCase):
             d2.addCallback(lambda ign: wf.setAttrs({}))
 
             d2.addCallback(lambda ign:
-                self.shouldFailWithSFTPError(sftp.FX_BAD_MESSAGE, "setAttrs with negative size",
+                self.shouldFailWithSFTPError(sftp.FX_BAD_MESSAGE, "setAttrs with negative size bad",
                                              wf.setAttrs, {'size': -1}))
 
             d2.addCallback(lambda ign: wf.setAttrs({'size': 14}))
             d2.addCallback(lambda ign: wf.getAttrs())
             d2.addCallback(lambda attrs: self.failUnlessReallyEqual(attrs['size'], 14))
 
+            d2.addCallback(lambda ign: wf.setAttrs({'size': 14}))
+            d2.addCallback(lambda ign: wf.getAttrs())
+            d2.addCallback(lambda attrs: self.failUnlessReallyEqual(attrs['size'], 14))
+
+            d2.addCallback(lambda ign: wf.setAttrs({'size': 17}))
+            d2.addCallback(lambda ign: wf.getAttrs())
+            d2.addCallback(lambda attrs: self.failUnlessReallyEqual(attrs['size'], 17))
+
             d2.addCallback(lambda ign:
                 self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "readChunk on write-only handle denied",
                                              wf.readChunk, 0, 1))
@@ -631,10 +654,10 @@ class Handler(GridTestMixin, ShouldFailMixin, unittest.TestCase):
             d2.addCallback(lambda ign: wf.close())
 
             d2.addCallback(lambda ign:
-                self.shouldFailWithSFTPError(sftp.FX_BAD_MESSAGE, "writeChunk on closed file",
+                self.shouldFailWithSFTPError(sftp.FX_BAD_MESSAGE, "writeChunk on closed file bad",
                                              wf.writeChunk, 0, "a"))
             d2.addCallback(lambda ign:
-                self.shouldFailWithSFTPError(sftp.FX_BAD_MESSAGE, "setAttrs on closed file",
+                self.shouldFailWithSFTPError(sftp.FX_BAD_MESSAGE, "setAttrs on closed file bad",
                                              wf.setAttrs, {'size': 0}))
 
             d2.addCallback(lambda ign: wf.close()) # should be no-op
@@ -642,7 +665,7 @@ class Handler(GridTestMixin, ShouldFailMixin, unittest.TestCase):
         d.addCallback(_write)
         d.addCallback(lambda ign: self.root.get(u"newfile"))
         d.addCallback(lambda node: download_to_data(node))
-        d.addCallback(lambda data: self.failUnlessReallyEqual(data, "012345670123\x00a"))
+        d.addCallback(lambda data: self.failUnlessReallyEqual(data, "012345670123\x00a\x00\x00\x00"))
 
         # test APPEND flag, and also replacing an existing file ("newfile" created by the previous test)
         d.addCallback(lambda ign:
@@ -1123,7 +1146,7 @@ class Handler(GridTestMixin, ShouldFailMixin, unittest.TestCase):
     def test_makeDirectory(self):
         d = self._set_up("makeDirectory")
         d.addCallback(lambda ign: self._set_up_tree())
-            
+
         # making a directory at a correct path should succeed
         d.addCallback(lambda ign: self.handler.makeDirectory("newdir", {'ext_foo': 'bar', 'ctime': 42}))
 
@@ -1162,6 +1185,13 @@ class Handler(GridTestMixin, ShouldFailMixin, unittest.TestCase):
         d.addCallback(lambda ign:
             self.shouldFailWithSFTPError(sftp.FX_FAILURE, "makeDirectory small",
                                          self.handler.makeDirectory, "small", {}))
+
+        # directories cannot be created read-only via SFTP
+        d.addCallback(lambda ign:
+            self.shouldFailWithSFTPError(sftp.FX_PERMISSION_DENIED, "makeDirectory newdir2 permissions:0444 denied",
+                                         self.handler.makeDirectory, "newdir2",
+                                         {'permissions': 0444}))
+
         return d
 
     def test_execCommand_and_openShell(self):