From 52f87904ed84baad038e982e488e851343b9b72d Mon Sep 17 00:00:00 2001 From: david-sarah Date: Fri, 11 Jun 2010 13:57:52 -0700 Subject: [PATCH] SFTP: improve test coverage for no-write on mutable files, and check for heisenfile table leaks in all relevant tests. Delete test_memory_leak since it is now redundant. --- src/allmydata/frontends/sftpd.py | 4 +- src/allmydata/test/test_sftp.py | 80 ++++++++++++++++---------------- 2 files changed, 44 insertions(+), 40 deletions(-) diff --git a/src/allmydata/frontends/sftpd.py b/src/allmydata/frontends/sftpd.py index 10741823..a96ae3ec 100644 --- a/src/allmydata/frontends/sftpd.py +++ b/src/allmydata/frontends/sftpd.py @@ -952,6 +952,8 @@ class GeneralSFTPFile(PrefixingLogMixin): 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? + # self.consumer.set_current_size(size) eventually_callback(d)(None) return None @@ -1159,7 +1161,7 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin): def _done(ign): if noisy: self.log("done %r\nall_heisenfiles = %r\nself._heisenfiles = %r" % (request, all_heisenfiles, self._heisenfiles), level=OPERATIONAL) - else: + else: # pragma: no cover self.log("done %r" % (request,), level=OPERATIONAL) return len(from_files) > 0 d.addBoth(_done) diff --git a/src/allmydata/test/test_sftp.py b/src/allmydata/test/test_sftp.py index 55f76019..7b69297d 100644 --- a/src/allmydata/test/test_sftp.py +++ b/src/allmydata/test/test_sftp.py @@ -333,6 +333,8 @@ class Handler(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, unittest.TestCas self.shouldFailWithSFTPError(sftp.FX_OP_UNSUPPORTED, "setAttrs size", self.handler.setAttrs, "small", {'size': 0})) + d.addCallback(lambda ign: self.failUnlessEqual(sftpd.all_heisenfiles, {})) + d.addCallback(lambda ign: self.failUnlessEqual(self.handler._heisenfiles, {})) return d def test_openFile_read(self): @@ -513,6 +515,8 @@ class Handler(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, unittest.TestCas return d2 d.addCallback(_read_short) + d.addCallback(lambda ign: self.failUnlessEqual(sftpd.all_heisenfiles, {})) + d.addCallback(lambda ign: self.failUnlessEqual(self.handler._heisenfiles, {})) return d def test_openFile_write(self): @@ -641,6 +645,8 @@ class Handler(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, unittest.TestCas 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.handler.getAttrs("newfile", followLinks=0)) + 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", @@ -842,6 +848,29 @@ class Handler(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, unittest.TestCas d.addCallback(_check_same_file) d.addCallback(lambda data: self.failUnlessReallyEqual(data, "mutable new! contents")) + # ... and with a setAttrs call that diminishes the parent link to read-only + d.addCallback(lambda ign: + self.handler.openFile("mutable", sftp.FXF_WRITE, {})) + def _write_mutable_setattr(wf): + d2 = wf.writeChunk(8, "read-only link from parent") + + d2.addCallback(lambda ign: self.handler.setAttrs("mutable", {'permissions': 0444})) + d2.addCallback(lambda ign: self.handler.getAttrs("mutable", followLinks=0)) + d2.addCallback(lambda attrs: self.failUnlessReallyEqual(attrs['permissions'], S_IFREG | 0444)) + + d2.addCallback(lambda ign: wf.close()) + return d2 + d.addCallback(_write_mutable_setattr) + d.addCallback(lambda ign: self.root.get(u"mutable")) + def _check_readonly_file(node): + self.failUnless(node.is_mutable()) + self.failUnless(node.is_readonly()) + self.failUnlessReallyEqual(node.get_write_uri(), None) + self.failUnlessReallyEqual(node.get_storage_index(), self.mutable.get_storage_index()) + return node.download_best_version() + d.addCallback(_check_readonly_file) + d.addCallback(lambda data: self.failUnlessReallyEqual(data, "mutable read-only link from parent")) + # test READ | WRITE without CREAT or TRUNC d.addCallback(lambda ign: self.handler.openFile("small", sftp.FXF_READ | sftp.FXF_WRITE, {})) @@ -913,6 +942,8 @@ class Handler(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, unittest.TestCas self.shouldFail(NoSuchChildError, "rename new while open", "new", self.root.get, u"new")) + d.addCallback(lambda ign: self.failUnlessEqual(sftpd.all_heisenfiles, {})) + d.addCallback(lambda ign: self.failUnlessEqual(self.handler._heisenfiles, {})) return d def test_removeFile(self): @@ -998,6 +1029,8 @@ class Handler(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, unittest.TestCas self.shouldFail(NoSuchChildError, "removeFile tempfile3", "tempfile3", self.root.get, u"tempfile3")) + d.addCallback(lambda ign: self.failUnlessEqual(sftpd.all_heisenfiles, {})) + d.addCallback(lambda ign: self.failUnlessEqual(self.handler._heisenfiles, {})) return d def test_removeDirectory(self): @@ -1033,6 +1066,8 @@ class Handler(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, unittest.TestCas self.shouldFail(NoSuchChildError, "removeDirectory unknown", "unknown", self.root.get, u"unknown")) + d.addCallback(lambda ign: self.failUnlessEqual(sftpd.all_heisenfiles, {})) + d.addCallback(lambda ign: self.failUnlessEqual(self.handler._heisenfiles, {})) return d def test_renameFile(self): @@ -1099,6 +1134,8 @@ class Handler(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, unittest.TestCas d.addCallback(lambda ign: self.root.get(u"new_unknown")) d.addCallback(lambda node: self.failUnlessReallyEqual(node.get_uri(), self.unknown_uri)) + d.addCallback(lambda ign: self.failUnlessEqual(sftpd.all_heisenfiles, {})) + d.addCallback(lambda ign: self.failUnlessEqual(self.handler._heisenfiles, {})) return d def test_renameFile_posix(self): @@ -1183,6 +1220,8 @@ class Handler(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, unittest.TestCas d.addCallback(lambda ign: self.root.get(u"new_unknown")) d.addCallback(lambda node: self.failUnlessReallyEqual(node.get_uri(), self.unknown_uri)) + d.addCallback(lambda ign: self.failUnlessEqual(sftpd.all_heisenfiles, {})) + d.addCallback(lambda ign: self.failUnlessEqual(self.handler._heisenfiles, {})) return d def test_makeDirectory(self): @@ -1234,6 +1273,8 @@ class Handler(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, unittest.TestCas self.handler.makeDirectory, "newdir2", {'permissions': 0444})) + d.addCallback(lambda ign: self.failUnlessEqual(sftpd.all_heisenfiles, {})) + d.addCallback(lambda ign: self.failUnlessEqual(self.handler._heisenfiles, {})) return d def test_execCommand_and_openShell(self): @@ -1300,42 +1341,3 @@ class Handler(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, unittest.TestCas self.handler.extendedRequest, "foo", "bar")) return d - - def test_memory_leak(self): - d0 = self._set_up("memory_leak") - - def _leaky(ign, i): - new_i = "new_%r" % (i,) - d = defer.succeed(None) - # Copied from test_openFile_write above: - - # it should be possible to rename even before the open has completed - def _open_and_rename_race(ign): - slow_open = defer.Deferred() - reactor.callLater(1, slow_open.callback, None) - d2 = self.handler.openFile("new", sftp.FXF_WRITE | sftp.FXF_CREAT, {}, delay=slow_open) - - # deliberate race between openFile and renameFile - d3 = self.handler.renameFile("new", new_i) - del d3 - return d2 - d.addCallback(_open_and_rename_race) - def _write_rename_race(wf): - d2 = wf.writeChunk(0, "abcd") - d2.addCallback(lambda ign: wf.close()) - return d2 - d.addCallback(_write_rename_race) - d.addCallback(lambda ign: self.root.get(unicode(new_i))) - d.addCallback(lambda node: download_to_data(node)) - d.addCallback(lambda data: self.failUnlessReallyEqual(data, "abcd")) - d.addCallback(lambda ign: - self.shouldFail(NoSuchChildError, "rename new while open", "new", - self.root.get, u"new")) - return d - - for index in range(3): - d0.addCallback(_leaky, index) - - d0.addCallback(lambda ign: self.failUnlessEqual(sftpd.all_heisenfiles, {})) - d0.addCallback(lambda ign: self.failUnlessEqual(self.handler._heisenfiles, {})) - return d0 -- 2.45.2