From b67f8b66c87c03807baae16a1d928cfa66b6e171 Mon Sep 17 00:00:00 2001 From: david-sarah Date: Fri, 11 Jun 2010 20:07:37 -0700 Subject: [PATCH] SFTP: further small improvements to test coverage. Also ensure that after a test failure, later tests don't fail spuriously due to the checks for heisenfile leaks. --- src/allmydata/frontends/sftpd.py | 31 ++++++++++++++------------- src/allmydata/test/test_sftp.py | 36 +++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/src/allmydata/frontends/sftpd.py b/src/allmydata/frontends/sftpd.py index b44879fe..e4f906e6 100644 --- a/src/allmydata/frontends/sftpd.py +++ b/src/allmydata/frontends/sftpd.py @@ -765,9 +765,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) @@ -869,7 +866,7 @@ 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)) @@ -946,7 +943,10 @@ 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() @@ -988,6 +988,9 @@ class Reason: all_heisenfiles = {} +def _reload(): + global all_heisenfiles + all_heisenfiles = {} class SFTPUserHandler(ConchUser, PrefixingLogMixin): implements(ISFTPServer) @@ -1510,17 +1513,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) - - # 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) + 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) # If there are heisenfiles to be written at the 'from' direntry, then ensure # they will now be written at the 'to' direntry instead. diff --git a/src/allmydata/test/test_sftp.py b/src/allmydata/test/test_sftp.py index 40541943..a743d382 100644 --- a/src/allmydata/test/test_sftp.py +++ b/src/allmydata/test/test_sftp.py @@ -73,6 +73,7 @@ class Handler(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, unittest.TestCas def _created_root(node): self.root = node self.root_uri = node.get_uri() + sftpd._reload() self.handler = sftpd.SFTPUserHandler(self.client, self.root, self.username) d.addCallback(_created_root) return d @@ -828,6 +829,8 @@ class Handler(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, unittest.TestCas d.addCallback(lambda node: download_to_data(node)) d.addCallback(lambda data: self.failUnlessReallyEqual(data, "abcde56789")) + d.addCallback(lambda ign: self.root.set_node(u"mutable2", self.mutable)) + # test writing to a mutable file d.addCallback(lambda ign: self.handler.openFile("mutable", sftp.FXF_WRITE, {})) @@ -839,6 +842,7 @@ class Handler(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, unittest.TestCas d.addCallback(lambda ign: self.root.get(u"mutable")) def _check_same_file(node): self.failUnless(node.is_mutable()) + self.failIf(node.is_readonly()) self.failUnlessReallyEqual(node.get_uri(), self.mutable_uri) return node.download_best_version() d.addCallback(_check_same_file) @@ -852,13 +856,19 @@ 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 + # ... and with a setAttrs call that diminishes the parent link to read-only, first by path 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.root.get(u"mutable")) + d2.addCallback(lambda node: self.failUnless(node.is_readonly())) + + d2.addCallback(lambda ign: wf.getAttrs()) + d2.addCallback(lambda attrs: self.failUnlessReallyEqual(attrs['permissions'], S_IFREG | 0666)) d2.addCallback(lambda ign: self.handler.getAttrs("mutable", followLinks=0)) d2.addCallback(lambda attrs: self.failUnlessReallyEqual(attrs['permissions'], S_IFREG | 0444)) @@ -875,6 +885,30 @@ class Handler(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, unittest.TestCas d.addCallback(_check_readonly_file) d.addCallback(lambda data: self.failUnlessReallyEqual(data, "mutable read-only link from parent")) + # ... and then by handle + d.addCallback(lambda ign: + self.handler.openFile("mutable2", sftp.FXF_WRITE, {})) + def _write_mutable2_setattr(wf): + d2 = wf.writeChunk(7, "2") + + d2.addCallback(lambda ign: wf.setAttrs({'permissions': 0444, 'size': 8})) + + # The link isn't made read-only until the file is closed. + d2.addCallback(lambda ign: self.root.get(u"mutable2")) + d2.addCallback(lambda node: self.failIf(node.is_readonly())) + + d2.addCallback(lambda ign: wf.getAttrs()) + d2.addCallback(lambda attrs: self.failUnlessReallyEqual(attrs['permissions'], S_IFREG | 0444)) + d2.addCallback(lambda ign: self.handler.getAttrs("mutable2", followLinks=0)) + d2.addCallback(lambda attrs: self.failUnlessReallyEqual(attrs['permissions'], S_IFREG | 0666)) + + d2.addCallback(lambda ign: wf.close()) + return d2 + d.addCallback(_write_mutable2_setattr) + d.addCallback(lambda ign: self.root.get(u"mutable2")) + d.addCallback(_check_readonly_file) # from above + d.addCallback(lambda data: self.failUnlessReallyEqual(data, "mutable2")) + # test READ | WRITE without CREAT or TRUNC d.addCallback(lambda ign: self.handler.openFile("small", sftp.FXF_READ | sftp.FXF_WRITE, {})) -- 2.45.2