From: david-sarah Date: Sun, 23 May 2010 03:25:49 +0000 (-0700) Subject: SFTP: fixes and test cases for renaming of open files. X-Git-Tag: trac-4400~12 X-Git-Url: https://git.rkrishnan.org/somewhere?a=commitdiff_plain;h=38964fb35a44fd7ce0ca0d0471b5767bc83f8352;p=tahoe-lafs%2Ftahoe-lafs.git SFTP: fixes and test cases for renaming of open files. --- diff --git a/src/allmydata/frontends/sftpd.py b/src/allmydata/frontends/sftpd.py index 8fbe2619..01abe114 100644 --- a/src/allmydata/frontends/sftpd.py +++ b/src/allmydata/frontends/sftpd.py @@ -685,6 +685,8 @@ class GeneralSFTPFile(PrefixingLogMixin): if noisy: self.log("__init__ done", level=NOISY) def rename(self, new_parent, new_childname): + self.log(".rename(%r, %r)" % (new_parent, new_childname), level=OPERATIONAL) + self.parent = new_parent self.childname = new_childname @@ -894,34 +896,35 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin): def _add_open_files(self, direntry, files_to_add): if direntry: + if direntry in self._open_files: + self._open_files[direntry] += files_to_add + else: + self._open_files[direntry] = files_to_add + if direntry in all_open_files: (old_files, opentime) = all_open_files[direntry] all_open_files[direntry] = (old_files + files_to_add, opentime) else: all_open_files[direntry] = (files_to_add, time()) - if direntry in self._open_files: - self._open_files[direntry] += files_to_add - else: - self._open_files[direntry] = files_to_add - def _remove_open_files(self, direntry, files_to_remove): if direntry and not self._logged_out: assert direntry in self._open_files, (direntry, self._open_files) assert direntry in all_open_files, (direntry, all_open_files) - (old_files, opentime) = all_open_files[direntry] + old_files = self._open_files[direntry] new_files = [f for f in old_files if f not in files_to_remove] if len(new_files) > 0: - all_open_files[direntry] = (new_files, opentime) + self._open_files[direntry] = new_files else: - del all_open_files[direntry] + del self._open_files[direntry] - files = [f for f in self._open_files[direntry] if f not in files_to_remove] - if len(files) > 0: - self._open_files[direntry] = files + (all_old_files, opentime) = all_open_files[direntry] + all_new_files = [f for f in all_old_files if f not in files_to_remove] + if len(all_new_files) > 0: + all_open_files[direntry] = (all_new_files, opentime) else: - del self._open_files[direntry] + del all_open_files[direntry] def _rename_open_files(self, from_parent, from_childname, to_parent, to_childname): """When an direntry is renamed, any open files for that direntry are also renamed. @@ -932,6 +935,7 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin): if from_direntry in all_open_files: (from_files, opentime) = all_open_files[from_direntry] + del self._open_files[from_direntry] del all_open_files[from_direntry] for file in from_files: file.rename(to_parent, to_childname) @@ -1159,22 +1163,33 @@ class SFTPUserHandler(ConchUser, PrefixingLogMixin): # # "It is an error if there already exists a file with the name specified # by newpath." + # For the standard SSH_FXP_RENAME operation, overwrite=False. + # We also support the extposix-rename@openssh.com extension, which uses overwrite=True. + # FIXME: use move_child_to_path to avoid possible data loss due to #943 - #d = parent.move_child_to_path(from_childname, to_root, to_path, overwrite=False) + #d2 = from_parent.move_child_to_path(from_childname, to_root, to_path, overwrite=overwrite) d2 = from_parent.move_child_to(from_childname, to_parent, to_childname, overwrite=overwrite) - def _handle(err): - if err.check(ExistingChildError): - # OpenSSH SFTP server returns FX_PERMISSION_DENIED - raise SFTPError(FX_PERMISSION_DENIED, "cannot rename to existing path /" + "/".join(to_path)) - if err.check(NoSuchChildError): + def _check(err): + if noisy: self.log("_check(%r) in .renameFile(%r, %r)" % + (err, from_pathstring, to_pathstring), level=NOISY) + + if not isinstance(err, Failure) or err.check(NoSuchChildError): # If there are open files to be written at the 'from' direntry, then ensure # they will now be written at the 'to' direntry instead. + if noisy: self.log("checking open files:\nself._open_files = %r\nall_open_files = %r" % + (self._open_files, all_open_files), level=NOISY) if self._rename_open_files(from_parent, from_childname, to_parent, to_childname): # suppress the NoSuchChildError if any open files were renamed + if noisy: self.log("after renaming:\nself._open_files = %r\nall_open_files = %r" % + (self._open_files, all_open_files), level=NOISY) return None + elif err.check(ExistingChildError): + # OpenSSH SFTP server returns FX_PERMISSION_DENIED + raise SFTPError(FX_PERMISSION_DENIED, "cannot rename to existing path " + to_pathstring) + return err - d2.addErrback(_handle) + d2.addBoth(_check) return d2 d.addCallback(_got) d.addBoth(_convert_error, request) diff --git a/src/allmydata/test/test_sftp.py b/src/allmydata/test/test_sftp.py index 0d546ebc..a192f81d 100644 --- a/src/allmydata/test/test_sftp.py +++ b/src/allmydata/test/test_sftp.py @@ -840,6 +840,40 @@ class Handler(GridTestMixin, ShouldFailMixin, unittest.TestCase): d.addCallback(lambda node: download_to_data(node)) d.addCallback(lambda data: self.failUnlessReallyEqual(data, "012345670123")) + # test WRITE and rename while still open + d.addCallback(lambda ign: + self.handler.openFile("small", sftp.FXF_WRITE, {})) + def _write_rename(wf): + d2 = wf.writeChunk(0, "abcd") + d2.addCallback(lambda ign: self.handler.renameFile("small", "renamed")) + d2.addCallback(lambda ign: wf.writeChunk(4, "efgh")) + d2.addCallback(lambda ign: wf.close()) + return d2 + d.addCallback(_write_rename) + d.addCallback(lambda ign: self.root.get(u"renamed")) + d.addCallback(lambda node: download_to_data(node)) + d.addCallback(lambda data: self.failUnlessReallyEqual(data, "abcdefgh0123")) + d.addCallback(lambda ign: + self.shouldFail(NoSuchChildError, "rename small while open", "small", + self.root.get, u"small")) + + # test WRITE | CREAT | EXCL and rename while still open + d.addCallback(lambda ign: + self.handler.openFile("newexcl", sftp.FXF_WRITE | sftp.FXF_CREAT | sftp.FXF_EXCL, {})) + def _write_creat_excl_rename(wf): + d2 = wf.writeChunk(0, "abcd") + d2.addCallback(lambda ign: self.handler.renameFile("newexcl", "renamedexcl")) + d2.addCallback(lambda ign: wf.writeChunk(4, "efgh")) + d2.addCallback(lambda ign: wf.close()) + return d2 + d.addCallback(_write_creat_excl_rename) + d.addCallback(lambda ign: self.root.get(u"renamedexcl")) + d.addCallback(lambda node: download_to_data(node)) + d.addCallback(lambda data: self.failUnlessReallyEqual(data, "abcdefgh")) + d.addCallback(lambda ign: + self.shouldFail(NoSuchChildError, "rename newexcl while open", "newexcl", + self.root.get, u"newexcl")) + return d def test_removeFile(self):