From 61e56cfeabbc6cabf3cf341ae9be9eb7646e0d07 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Fri, 27 May 2016 21:18:34 -0700 Subject: [PATCH] Fix small rename corner case and add test cases for timestamps in many rename corner cases --- src/cryfs/filesystem/CryNode.cpp | 2 + src/fspp/fstest/FsppNodeTest_Rename.h | 59 +++-- src/fspp/fstest/FsppNodeTest_Timestamps.h | 225 +++++++++++++++++- .../fstest/testutils/TimestampTestUtils.h | 31 ++- 4 files changed, 283 insertions(+), 34 deletions(-) diff --git a/src/cryfs/filesystem/CryNode.cpp b/src/cryfs/filesystem/CryNode.cpp index dd801631..597f8a67 100644 --- a/src/cryfs/filesystem/CryNode.cpp +++ b/src/cryfs/filesystem/CryNode.cpp @@ -72,6 +72,8 @@ void CryNode::rename(const bf::path &to) { targetDir->AddOrOverwriteChild(to.filename().native(), oldEntry.key(), oldEntry.type(), oldEntry.mode(), oldEntry.uid(), oldEntry.gid(), oldEntry.lastAccessTime(), oldEntry.lastModificationTime(), onOverwritten); (*_parent)->RemoveChild(oldEntry.name()); + // targetDir is now the new parent for this node. Adapt to it, so we can call further operations on this node object. + _parent = cpputils::to_unique_ptr(std::move(targetDir)); } } diff --git a/src/fspp/fstest/FsppNodeTest_Rename.h b/src/fspp/fstest/FsppNodeTest_Rename.h index 711871e2..31f75cae 100644 --- a/src/fspp/fstest/FsppNodeTest_Rename.h +++ b/src/fspp/fstest/FsppNodeTest_Rename.h @@ -8,7 +8,7 @@ template class FsppNodeTest_Rename: public FsppNodeTest { public: - void Test_TargetParentDirDoesntExist() { + void Test_Error_TargetParentDirDoesntExist() { auto node = this->CreateNode("/oldname"); try { node->rename("/notexistingdir/newname"); @@ -20,7 +20,7 @@ public: EXPECT_NE(boost::none, this->device->Load("/oldname")); } - void Test_TargetParentDirIsFile() { + void Test_Error_TargetParentDirIsFile() { auto node = this->CreateNode("/oldname"); this->CreateFile("/somefile"); try { @@ -34,6 +34,16 @@ public: EXPECT_NE(boost::none, this->device->Load("/somefile")); } + void Test_Error_RootDir() { + auto root = this->LoadDir("/"); + try { + root->rename("/newname"); + EXPECT_TRUE(false); // expect throws + } catch (const fspp::fuse::FuseErrnoException &e) { + EXPECT_EQ(EBUSY, e.getErrno()); + } + } + void Test_InRoot() { auto node = this->CreateNode("/oldname"); node->rename("/newname"); @@ -105,16 +115,6 @@ public: EXPECT_NE(boost::none, this->device->Load("/oldname")); } - void Test_RootDir() { - auto root = this->LoadDir("/"); - try { - root->rename("/newname"); - EXPECT_TRUE(false); // expect throws - } catch (const fspp::fuse::FuseErrnoException &e) { - EXPECT_EQ(EBUSY, e.getErrno()); - } - } - void Test_Overwrite_InSameDir() { auto node = this->CreateNode("/oldname"); this->CreateNode("/newname"); @@ -141,7 +141,7 @@ public: EXPECT_EQ(3u, this->LoadDir("/")->children()->size()); // 3, because of '.' and '..' } - void Test_Overwrite_DirWithFile_InSameDir() { + void Test_Overwrite_Error_DirWithFile_InSameDir() { auto file = this->CreateFile("/oldname"); this->CreateDir("/newname"); try { @@ -154,7 +154,7 @@ public: EXPECT_NE(boost::none, this->device->Load("/newname")); } - void Test_Overwrite_DirWithFile_InDifferentDir() { + void Test_Overwrite_Error_DirWithFile_InDifferentDir() { this->CreateDir("/parent1"); this->CreateDir("/parent2"); auto file = this->CreateFile("/parent1/oldname"); @@ -169,7 +169,7 @@ public: EXPECT_NE(boost::none, this->device->Load("/parent2/newname")); } - void Test_Overwrite_FileWithDir_InSameDir() { + void Test_Overwrite_Error_FileWithDir_InSameDir() { auto dir = this->CreateDir("/oldname"); this->CreateFile("/newname"); try { @@ -182,7 +182,7 @@ public: EXPECT_NE(boost::none, this->device->Load("/newname")); } - void Test_Overwrite_FileWithDir_InDifferentDir() { + void Test_Overwrite_Error_FileWithDir_InDifferentDir() { this->CreateDir("/parent1"); this->CreateDir("/parent2"); auto dir = this->CreateDir("/parent1/oldname"); @@ -196,11 +196,24 @@ public: EXPECT_NE(boost::none, this->device->Load("/parent1/oldname")); EXPECT_NE(boost::none, this->device->Load("/parent2/newname")); } + + void Test_CanRenameTwice() { + // Test that the node object stays valid after a rename, even if it now points to an entry of a different parent directory. + this->CreateDir("/mydir1"); + this->CreateDir("/mydir2"); + auto node = this->CreateNode("/oldname"); + node->rename("/mydir1/newname"); + node->rename("/mydir2/newname"); + EXPECT_EQ(boost::none, this->device->Load("/oldname")); + EXPECT_EQ(boost::none, this->device->Load("/mydir1/newname")); + EXPECT_NE(boost::none, this->device->Load("/mydir2/newname")); + } }; REGISTER_NODE_TEST_CASE(FsppNodeTest_Rename, - TargetParentDirDoesntExist, - TargetParentDirIsFile, + Error_TargetParentDirDoesntExist, + Error_TargetParentDirIsFile, + Error_RootDir, InRoot, InNested, RootToNested_SameName, @@ -210,14 +223,14 @@ REGISTER_NODE_TEST_CASE(FsppNodeTest_Rename, NestedToNested_SameName, NestedToNested_NewName, ToItself, - RootDir, Overwrite_InSameDir, Overwrite_InDifferentDir, Overwrite_DoesntHaveSameEntryTwice, - Overwrite_DirWithFile_InSameDir, - Overwrite_DirWithFile_InDifferentDir, - Overwrite_FileWithDir_InSameDir, - Overwrite_FileWithDir_InDifferentDir + Overwrite_Error_DirWithFile_InSameDir, + Overwrite_Error_DirWithFile_InDifferentDir, + Overwrite_Error_FileWithDir_InSameDir, + Overwrite_Error_FileWithDir_InDifferentDir, + CanRenameTwice ); #endif diff --git a/src/fspp/fstest/FsppNodeTest_Timestamps.h b/src/fspp/fstest/FsppNodeTest_Timestamps.h index 87b92c9c..5e7cc7f0 100644 --- a/src/fspp/fstest/FsppNodeTest_Timestamps.h +++ b/src/fspp/fstest/FsppNodeTest_Timestamps.h @@ -62,17 +62,209 @@ public: EXPECT_OPERATION_DOESNT_UPDATE_TIMESTAMPS(*node, operation); } - void Test_Rename() { - auto node = this->CreateNode("/mynode"); + void Test_Rename_Error_TargetParentDirDoesntExist() { + auto node = this->CreateNode("/oldname"); auto operation = [&node] () { - node->rename("newnodename"); + try { + node->rename("/notexistingdir/newname"); + EXPECT_TRUE(false); // expect rename to fail + } catch (const fspp::fuse::FuseErrnoException &e) { + EXPECT_EQ(ENOENT, e.getErrno()); //Rename fails, everything is ok. + } }; - EXPECT_OPERATION_DOESNT_UPDATE_ACCESS_TIMESTAMP(*node, operation); - EXPECT_OPERATION_DOESNT_UPDATE_MODIFICATION_TIMESTAMP(*node, operation); - EXPECT_OPERATION_UPDATES_METADATACHANGE_TIMESTAMP(*node, operation); + EXPECT_OPERATION_DOESNT_UPDATE_TIMESTAMPS(*node, operation); } - // TODO Other rename cases (e.g. failed renames/error paths, moving to different dir, ...) from FsppNodeTest_Rename + void Test_Rename_Error_TargetParentDirIsFile() { + auto node = this->CreateNode("/oldname"); + this->CreateFile("/somefile"); + auto operation = [&node] () { + try { + node->rename("/somefile/newname"); + EXPECT_TRUE(false); // expect rename to fail + } catch (const fspp::fuse::FuseErrnoException &e) { + EXPECT_EQ(ENOTDIR, e.getErrno()); //Rename fails, everything is ok. + } + }; + EXPECT_OPERATION_DOESNT_UPDATE_TIMESTAMPS(*node, operation); + } + + void Test_Rename_Error_RootDir() { + // TODO Re-enable this test once the root dir stores timestamps correctly + /* + auto root = this->LoadDir("/"); + auto operation = [&root] () { + try { + root->rename("/newname"); + EXPECT_TRUE(false); // expect throws + } catch (const fspp::fuse::FuseErrnoException &e) { + EXPECT_EQ(EBUSY, e.getErrno()); //Rename fails, everything is ok. + } + }; + EXPECT_OPERATION_DOESNT_UPDATE_TIMESTAMPS(*root, operation); + */ + } + + void Test_Rename_InRoot() { + auto node = this->CreateNode("/oldname"); + auto operation = [&node] () { + node->rename("/newname"); + }; + EXPECT_OPERATION_ONLY_UPDATES_METADATACHANGE_TIMESTAMP(*node, operation); + } + + void Test_Rename_InNested() { + this->CreateDir("/mydir"); + auto node = this->CreateNode("/mydir/oldname"); + auto operation = [&node] () { + node->rename("/mydir/newname"); + }; + EXPECT_OPERATION_ONLY_UPDATES_METADATACHANGE_TIMESTAMP(*node, operation); + } + + void Test_Rename_RootToNested_SameName() { + this->CreateDir("/mydir"); + auto node = this->CreateNode("/oldname"); + auto operation = [&node] () { + node->rename("/mydir/oldname"); + }; + EXPECT_OPERATION_ONLY_UPDATES_METADATACHANGE_TIMESTAMP(*node, operation); + } + + void Test_Rename_RootToNested_NewName() { + this->CreateDir("/mydir"); + auto node = this->CreateNode("/oldname"); + auto operation = [&node] () { + node->rename("/mydir/newname"); + }; + EXPECT_OPERATION_ONLY_UPDATES_METADATACHANGE_TIMESTAMP(*node, operation); + } + + void Test_Rename_NestedToRoot_SameName() { + this->CreateDir("/mydir"); + auto node = this->CreateNode("/mydir/oldname"); + auto operation = [&node] () { + node->rename("/oldname"); + }; + EXPECT_OPERATION_ONLY_UPDATES_METADATACHANGE_TIMESTAMP(*node, operation); + } + + void Test_Rename_NestedToRoot_NewName() { + this->CreateDir("/mydir"); + auto node = this->CreateNode("/mydir/oldname"); + auto operation = [&node] () { + node->rename("/newname"); + }; + EXPECT_OPERATION_ONLY_UPDATES_METADATACHANGE_TIMESTAMP(*node, operation); + } + + void Test_Rename_NestedToNested_SameName() { + this->CreateDir("/mydir1"); + this->CreateDir("/mydir2"); + auto node = this->CreateNode("/mydir1/oldname"); + auto operation = [&node] () { + node->rename("/mydir2/oldname"); + }; + EXPECT_OPERATION_ONLY_UPDATES_METADATACHANGE_TIMESTAMP(*node, operation); + } + + void Test_Rename_NestedToNested_NewName() { + this->CreateDir("/mydir1"); + this->CreateDir("/mydir2"); + auto node = this->CreateNode("/mydir1/oldname"); + auto operation = [&node] () { + node->rename("/mydir2/newname"); + }; + EXPECT_OPERATION_ONLY_UPDATES_METADATACHANGE_TIMESTAMP(*node, operation); + } + + void Test_Rename_ToItself() { + auto node = this->CreateNode("/oldname"); + auto operation = [&node] () { + node->rename("/oldname"); + }; + EXPECT_OPERATION_ONLY_UPDATES_METADATACHANGE_TIMESTAMP(*node, operation); + } + + void Test_Rename_Overwrite_InSameDir() { + auto node = this->CreateNode("/oldname"); + this->CreateNode("/newname"); + auto operation = [&node] () { + node->rename("/newname"); + }; + EXPECT_OPERATION_ONLY_UPDATES_METADATACHANGE_TIMESTAMP(*node, operation); + } + + void Test_Rename_Overwrite_InDifferentDir() { + this->CreateDir("/mydir1"); + this->CreateDir("/mydir2"); + this->CreateNode("/mydir2/newname"); + auto node = this->CreateNode("/mydir1/oldname"); + auto operation = [&node] () { + node->rename("/mydir2/newname"); + }; + EXPECT_OPERATION_ONLY_UPDATES_METADATACHANGE_TIMESTAMP(*node, operation); + } + + void Test_Rename_Overwrite_Error_DirWithFile_InSameDir() { + auto node = this->CreateFile("/oldname"); + this->CreateDir("/newname"); + auto operation = [&node] () { + try { + node->rename("/newname"); + EXPECT_TRUE(false); // expect rename to fail + } catch (const fspp::fuse::FuseErrnoException &e) { + EXPECT_EQ(EISDIR, e.getErrno()); //Rename fails, everything is ok. + } + }; + EXPECT_OPERATION_DOESNT_UPDATE_TIMESTAMPS(*node, operation); + } + + void Test_Rename_Overwrite_Error_DirWithFile_InDifferentDir() { + this->CreateDir("/mydir1"); + this->CreateDir("/mydir2"); + auto node = this->CreateFile("/mydir1/oldname"); + this->CreateDir("/mydir2/newname"); + auto operation = [&node] () { + try { + node->rename("/mydir2/newname"); + EXPECT_TRUE(false); // expect rename to fail + } catch (const fspp::fuse::FuseErrnoException &e) { + EXPECT_EQ(EISDIR, e.getErrno());//Rename fails, everything is ok. + } + }; + EXPECT_OPERATION_DOESNT_UPDATE_TIMESTAMPS(*node, operation); + } + + void Test_Rename_Overwrite_Error_FileWithDir_InSameDir() { + auto node = this->CreateDir("/oldname"); + this->CreateFile("/newname"); + auto operation = [&node] () { + try { + node->rename("/newname"); + EXPECT_TRUE(false); // expect rename to fail + } catch (const fspp::fuse::FuseErrnoException &e) { + EXPECT_EQ(ENOTDIR, e.getErrno()); //Rename fails, everything is ok. + } + }; + EXPECT_OPERATION_DOESNT_UPDATE_TIMESTAMPS(*node, operation); + } + + void Test_Rename_Overwrite_Error_FileWithDir_InDifferentDir() { + this->CreateDir("/mydir1"); + this->CreateDir("/mydir2"); + auto node = this->CreateDir("/mydir1/oldname"); + this->CreateFile("/mydir2/newname"); + auto operation = [&node] () { + try { + node->rename("/mydir2/newname"); + EXPECT_TRUE(false); // expect rename to fail + } catch (const fspp::fuse::FuseErrnoException &e) { + EXPECT_EQ(ENOTDIR, e.getErrno()); //Rename fails, everything is ok. + } + }; + EXPECT_OPERATION_DOESNT_UPDATE_TIMESTAMPS(*node, operation); + } void Test_Utimens() { auto node = this->CreateNode("/mynode"); @@ -93,7 +285,24 @@ REGISTER_NODE_TEST_CASE(FsppNodeTest_Timestamps, Chmod, Chown, Access, - Rename, + Rename_Error_TargetParentDirDoesntExist, + Rename_Error_TargetParentDirIsFile, + Rename_Error_RootDir, + Rename_InRoot, + Rename_InNested, + Rename_RootToNested_SameName, + Rename_RootToNested_NewName, + Rename_NestedToRoot_SameName, + Rename_NestedToRoot_NewName, + Rename_NestedToNested_SameName, + Rename_NestedToNested_NewName, + Rename_ToItself, + Rename_Overwrite_InSameDir, + Rename_Overwrite_InDifferentDir, + Rename_Overwrite_Error_DirWithFile_InSameDir, + Rename_Overwrite_Error_DirWithFile_InDifferentDir, + Rename_Overwrite_Error_FileWithDir_InSameDir, + Rename_Overwrite_Error_FileWithDir_InDifferentDir, Utimens ); diff --git a/src/fspp/fstest/testutils/TimestampTestUtils.h b/src/fspp/fstest/testutils/TimestampTestUtils.h index 49c97065..4280541a 100644 --- a/src/fspp/fstest/testutils/TimestampTestUtils.h +++ b/src/fspp/fstest/testutils/TimestampTestUtils.h @@ -70,9 +70,34 @@ public: } void EXPECT_OPERATION_DOESNT_UPDATE_TIMESTAMPS(const fspp::Node &node, std::function operation) { - EXPECT_OPERATION_DOESNT_UPDATE_ACCESS_TIMESTAMP(node, operation); - EXPECT_OPERATION_DOESNT_UPDATE_MODIFICATION_TIMESTAMP(node, operation); - EXPECT_OPERATION_DOESNT_UPDATE_METADATACHANGE_TIMESTAMP(node, operation); + // equivalent to the following, but implemented separately because operation() should only be called once. + // EXPECT_OPERATION_DOESNT_UPDATE_ACCESS_TIMESTAMP(node, operation); + // EXPECT_OPERATION_DOESNT_UPDATE_MODIFICATION_TIMESTAMP(node, operation); + // EXPECT_OPERATION_DOESNT_UPDATE_METADATACHANGE_TIMESTAMP(node, operation); + ensureNodeTimestampsAreOld(node); + struct stat oldStat = stat(node); + operation(); + struct stat newStat = stat(node); + EXPECT_EQ(oldStat.st_atim, newStat.st_atim); + EXPECT_EQ(oldStat.st_mtim, newStat.st_mtim); + EXPECT_LE(oldStat.st_ctim, newStat.st_ctim); + } + + void EXPECT_OPERATION_ONLY_UPDATES_METADATACHANGE_TIMESTAMP(const fspp::Node &node, std::function operation) { + // equivalent to the following, but implemented separately because operation() should only be called once. + // EXPECT_OPERATION_DOESNT_UPDATE_ACCESS_TIMESTAMP(node, operation); + // EXPECT_OPERATION_DOESNT_UPDATE_MODIFICATION_TIMESTAMP(node, operation); + // EXPECT_OPERATION_UPDATES_METADATACHANGE_TIMESTAMP(node, operation); + ensureNodeTimestampsAreOld(node); + struct stat oldStat = stat(node); + timespec lowerBound = cpputils::time::now(); + operation(); + timespec upperBound = cpputils::time::now(); + struct stat newStat = stat(node); + EXPECT_EQ(oldStat.st_atim, newStat.st_atim); + EXPECT_EQ(oldStat.st_mtim, newStat.st_mtim); + EXPECT_LE(lowerBound, newStat.st_ctim); + EXPECT_GE(upperBound, newStat.st_ctim); } struct stat stat(const fspp::Node &node) {