diff --git a/ChangeLog.txt b/ChangeLog.txt index e2a84c1f..a5931ad9 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -8,6 +8,7 @@ Version 0.9.3 (unreleased) * The ciphertext block size is configurable. You can use the "--blocksize-bytes" command line argument. If not specified, CryFS will ask you for a block size when creating a file system. * Fix fstat (a bug in the fstat implementation caused problems with some text editors (e.g. nano) falsely thinking a file changed since they opened it). * Fix rename (when trying to rename a file to an already existing file name, a bug deleted it instead). +* Rename operation allows overwriting existing files. Version 0.9.2 --------------- diff --git a/src/cryfs/filesystem/CryNode.cpp b/src/cryfs/filesystem/CryNode.cpp index 0d666085..f6bed7f5 100644 --- a/src/cryfs/filesystem/CryNode.cpp +++ b/src/cryfs/filesystem/CryNode.cpp @@ -63,8 +63,8 @@ void CryNode::rename(const bf::path &to) { throw FuseErrnoException(EIO); } std::string oldName = old->name(); // Store, because if targetDir == *_parent, then the 'old' object will be invalidated after we add something to targetDir - if (oldName != to.filename().native()) { - targetDir->AddChild(to.filename().native(), old->key(), old->type(), old->mode(), old->uid(), old->gid(), + if (targetDir->key() != (*_parent)->key() || oldName != to.filename().native()) { + targetDir->AddOrOverwriteChild(to.filename().native(), old->key(), old->type(), old->mode(), old->uid(), old->gid(), old->lastAccessTime(), old->lastModificationTime()); (*_parent)->RemoveChild(oldName); } diff --git a/src/cryfs/filesystem/cachingfsblobstore/DirBlobRef.h b/src/cryfs/filesystem/cachingfsblobstore/DirBlobRef.h index cee266b0..a64cd780 100644 --- a/src/cryfs/filesystem/cachingfsblobstore/DirBlobRef.h +++ b/src/cryfs/filesystem/cachingfsblobstore/DirBlobRef.h @@ -42,9 +42,9 @@ public: return _base->flush(); } - void AddChild(const std::string &name, const blockstore::Key &blobKey, fspp::Dir::EntryType type, + void AddOrOverwriteChild(const std::string &name, const blockstore::Key &blobKey, fspp::Dir::EntryType type, mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime) { - return _base->AddChild(name, blobKey, type, mode, uid, gid, lastAccessTime, lastModificationTime); + return _base->AddOrOverwriteChild(name, blobKey, type, mode, uid, gid, lastAccessTime, lastModificationTime); } void statChild(const blockstore::Key &key, struct ::stat *result) const { diff --git a/src/cryfs/filesystem/fsblobstore/DirBlob.cpp b/src/cryfs/filesystem/fsblobstore/DirBlob.cpp index a7f266b1..4a9091c2 100644 --- a/src/cryfs/filesystem/fsblobstore/DirBlob.cpp +++ b/src/cryfs/filesystem/fsblobstore/DirBlob.cpp @@ -65,24 +65,31 @@ void DirBlob::_readEntriesFromBlob() { } void DirBlob::AddChildDir(const std::string &name, const Key &blobKey, mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime) { - AddChild(name, blobKey, fspp::Dir::EntryType::DIR, mode, uid, gid, lastAccessTime, lastModificationTime); + _addChild(name, blobKey, fspp::Dir::EntryType::DIR, mode, uid, gid, lastAccessTime, lastModificationTime); } void DirBlob::AddChildFile(const std::string &name, const Key &blobKey, mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime) { - AddChild(name, blobKey, fspp::Dir::EntryType::FILE, mode, uid, gid, lastAccessTime, lastModificationTime); + _addChild(name, blobKey, fspp::Dir::EntryType::FILE, mode, uid, gid, lastAccessTime, lastModificationTime); } void DirBlob::AddChildSymlink(const std::string &name, const blockstore::Key &blobKey, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime) { - AddChild(name, blobKey, fspp::Dir::EntryType::SYMLINK, S_IFLNK | S_IRUSR | S_IWUSR | S_IXUSR | S_IRGRP | S_IWGRP | S_IXGRP | S_IROTH | S_IWOTH | S_IXOTH, uid, gid, lastAccessTime, lastModificationTime); + _addChild(name, blobKey, fspp::Dir::EntryType::SYMLINK, S_IFLNK | S_IRUSR | S_IWUSR | S_IXUSR | S_IRGRP | S_IWGRP | S_IXGRP | S_IROTH | S_IWOTH | S_IXOTH, uid, gid, lastAccessTime, lastModificationTime); } -void DirBlob::AddChild(const std::string &name, const Key &blobKey, +void DirBlob::_addChild(const std::string &name, const Key &blobKey, fspp::Dir::EntryType entryType, mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime) { std::unique_lock lock(_mutex); _entries.add(name, blobKey, entryType, mode, uid, gid, lastAccessTime, lastModificationTime); _changed = true; } +void DirBlob::AddOrOverwriteChild(const std::string &name, const Key &blobKey, + fspp::Dir::EntryType entryType, mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime) { + std::unique_lock lock(_mutex); + _entries.addOrOverwrite(name, blobKey, entryType, mode, uid, gid, lastAccessTime, lastModificationTime); + _changed = true; +} + boost::optional DirBlob::GetChild(const string &name) const { std::unique_lock lock(_mutex); return _entries.get(name); diff --git a/src/cryfs/filesystem/fsblobstore/DirBlob.h b/src/cryfs/filesystem/fsblobstore/DirBlob.h index 592c276e..28c0f476 100644 --- a/src/cryfs/filesystem/fsblobstore/DirBlob.h +++ b/src/cryfs/filesystem/fsblobstore/DirBlob.h @@ -43,7 +43,7 @@ namespace cryfs { void AddChildSymlink(const std::string &name, const blockstore::Key &blobKey, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime); - void AddChild(const std::string &name, const blockstore::Key &blobKey, fspp::Dir::EntryType type, + void AddOrOverwriteChild(const std::string &name, const blockstore::Key &blobKey, fspp::Dir::EntryType type, mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime); void RemoveChild(const std::string &name); @@ -66,6 +66,8 @@ namespace cryfs { private: + void _addChild(const std::string &name, const blockstore::Key &blobKey, fspp::Dir::EntryType type, + mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime); void _readEntriesFromBlob(); void _writeEntriesToBlob(); diff --git a/src/cryfs/filesystem/fsblobstore/utils/DirEntryList.cpp b/src/cryfs/filesystem/fsblobstore/utils/DirEntryList.cpp index 200b5722..fbebc04b 100644 --- a/src/cryfs/filesystem/fsblobstore/utils/DirEntryList.cpp +++ b/src/cryfs/filesystem/fsblobstore/utils/DirEntryList.cpp @@ -43,10 +43,7 @@ void DirEntryList::deserializeFrom(const void *data, uint64_t size) { } bool DirEntryList::_hasChild(const string &name) const { - auto found = std::find_if(_entries.begin(), _entries.end(), [&name] (const DirEntry &entry) { - return entry.name() == name; - }); - return found != _entries.end(); + return _entries.end() != _findByName(name); } void DirEntryList::add(const string &name, const Key &blobKey, fspp::Dir::EntryType entryType, mode_t mode, @@ -54,10 +51,40 @@ void DirEntryList::add(const string &name, const Key &blobKey, fspp::Dir::EntryT if (_hasChild(name)) { throw fspp::fuse::FuseErrnoException(EEXIST); } + _add(name, blobKey, entryType, mode, uid, gid, lastAccessTime, lastModificationTime); +} + +void DirEntryList::_add(const string &name, const Key &blobKey, fspp::Dir::EntryType entryType, mode_t mode, + uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime) { auto insert_pos = _findUpperBound(blobKey); _entries.emplace(insert_pos, entryType, name, blobKey, mode, uid, gid, lastAccessTime, lastModificationTime, time::now()); } +void DirEntryList::addOrOverwrite(const string &name, const Key &blobKey, fspp::Dir::EntryType entryType, mode_t mode, + uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime) { + auto found = _findByName(name); + if (found != _entries.end()) { + _overwrite(&*found, name, blobKey, entryType, mode, uid, gid, lastAccessTime, lastModificationTime); + } else { + _add(name, blobKey, entryType, mode, uid, gid, lastAccessTime, lastModificationTime); + } +} + +void DirEntryList::_overwrite(DirEntry *entry, const string &name, const Key &blobKey, fspp::Dir::EntryType entryType, mode_t mode, + uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime) { + if (entry->type() != entryType) { + if (entry->type() == fspp::Dir::EntryType::DIR) { + // new path is an existing directory, but old path is not a directory + throw fspp::fuse::FuseErrnoException(EISDIR); + } + if (entryType == fspp::Dir::EntryType::DIR) { + // oldpath is a directory, and newpath exists but is not a directory. + throw fspp::fuse::FuseErrnoException(ENOTDIR); + } + } + *entry = DirEntry(entryType, name, blobKey, mode, uid, gid, lastAccessTime, lastModificationTime, time::now()); +} + boost::optional DirEntryList::get(const string &name) const { auto found = _findByName(name); if (found == _entries.end()) { @@ -90,12 +117,16 @@ void DirEntryList::remove(const Key &key) { _entries.erase(lowerBound, upperBound); } -vector::const_iterator DirEntryList::_findByName(const string &name) const { +vector::iterator DirEntryList::_findByName(const string &name) { return std::find_if(_entries.begin(), _entries.end(), [&name] (const DirEntry &entry) { return entry.name() == name; }); } +vector::const_iterator DirEntryList::_findByName(const string &name) const { + return const_cast(this)->_findByName(name); +} + vector::iterator DirEntryList::_findByKey(const Key &key) { auto found = _findLowerBound(key); if (found == _entries.end() || found->key() != key) { diff --git a/src/cryfs/filesystem/fsblobstore/utils/DirEntryList.h b/src/cryfs/filesystem/fsblobstore/utils/DirEntryList.h index 40a8541d..de8dc4b6 100644 --- a/src/cryfs/filesystem/fsblobstore/utils/DirEntryList.h +++ b/src/cryfs/filesystem/fsblobstore/utils/DirEntryList.h @@ -21,6 +21,8 @@ namespace cryfs { void add(const std::string &name, const blockstore::Key &blobKey, fspp::Dir::EntryType entryType, mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime); + void addOrOverwrite(const std::string &name, const blockstore::Key &blobKey, fspp::Dir::EntryType entryType, + mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime); boost::optional get(const std::string &name) const; boost::optional get(const blockstore::Key &key) const; void remove(const std::string &name); @@ -37,12 +39,17 @@ namespace cryfs { private: uint64_t _serializedSize() const; bool _hasChild(const std::string &name) const; + std::vector::iterator _findByName(const std::string &name); std::vector::const_iterator _findByName(const std::string &name) const; std::vector::iterator _findByKey(const blockstore::Key &key); std::vector::const_iterator _findByKey(const blockstore::Key &key) const; std::vector::iterator _findUpperBound(const blockstore::Key &key); std::vector::iterator _findLowerBound(const blockstore::Key &key); std::vector::iterator _findFirst(const blockstore::Key &hint, std::function pred); + void _add(const std::string &name, const blockstore::Key &blobKey, fspp::Dir::EntryType entryType, + mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime); + void _overwrite(DirEntry *entry, const std::string &name, const blockstore::Key &blobKey, fspp::Dir::EntryType entryType, + mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime); std::vector _entries; diff --git a/src/cryfs/filesystem/parallelaccessfsblobstore/DirBlobRef.h b/src/cryfs/filesystem/parallelaccessfsblobstore/DirBlobRef.h index 3fd9efe7..d1097148 100644 --- a/src/cryfs/filesystem/parallelaccessfsblobstore/DirBlobRef.h +++ b/src/cryfs/filesystem/parallelaccessfsblobstore/DirBlobRef.h @@ -38,9 +38,9 @@ public: return _base->flush(); } - void AddChild(const std::string &name, const blockstore::Key &blobKey, fspp::Dir::EntryType type, + void AddOrOverwriteChild(const std::string &name, const blockstore::Key &blobKey, fspp::Dir::EntryType type, mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime) { - return _base->AddChild(name, blobKey, type, mode, uid, gid, lastAccessTime, lastModificationTime); + return _base->AddOrOverwriteChild(name, blobKey, type, mode, uid, gid, lastAccessTime, lastModificationTime); } void statChild(const blockstore::Key &key, struct ::stat *result) const { diff --git a/src/fspp/fstest/FsppNodeTest.h b/src/fspp/fstest/FsppNodeTest.h index 35e2032d..c6759aa3 100644 --- a/src/fspp/fstest/FsppNodeTest.h +++ b/src/fspp/fstest/FsppNodeTest.h @@ -54,7 +54,15 @@ public: EXPECT_NE(boost::none, this->device->Load("/mydir/newname")); } - void Test_Rename_RootToNested() { + void Test_Rename_RootToNested_SameName() { + CreateDir("/mydir"); + auto node = CreateNode("/oldname"); + node->rename("/mydir/oldname"); + EXPECT_EQ(boost::none, this->device->Load("/oldname")); + EXPECT_NE(boost::none, this->device->Load("/mydir/oldname")); + } + + void Test_Rename_RootToNested_NewName() { CreateDir("/mydir"); auto node = CreateNode("/oldname"); node->rename("/mydir/newname"); @@ -62,7 +70,15 @@ public: EXPECT_NE(boost::none, this->device->Load("/mydir/newname")); } - void Test_Rename_NestedToRoot() { + void Test_Rename_NestedToRoot_SameName() { + CreateDir("/mydir"); + auto node = CreateNode("/mydir/oldname"); + node->rename("/oldname"); + EXPECT_EQ(boost::none, this->device->Load("/mydir/oldname")); + EXPECT_NE(boost::none, this->device->Load("/oldname")); + } + + void Test_Rename_NestedToRoot_NewName() { CreateDir("/mydir"); auto node = CreateNode("/mydir/oldname"); node->rename("/newname"); @@ -70,7 +86,16 @@ public: EXPECT_NE(boost::none, this->device->Load("/newname")); } - void Test_Rename_NestedToNested() { + void Test_Rename_NestedToNested_SameName() { + CreateDir("/mydir"); + CreateDir("/mydir2"); + auto node = CreateNode("/mydir/oldname"); + node->rename("/mydir2/oldname"); + EXPECT_EQ(boost::none, this->device->Load("/mydir/oldname")); + EXPECT_NE(boost::none, this->device->Load("/mydir2/oldname")); + } + + void Test_Rename_NestedToNested_NewName() { CreateDir("/mydir"); CreateDir("/mydir2"); auto node = CreateNode("/mydir/oldname"); @@ -95,22 +120,70 @@ public: } } -private: - void CreateDir(const boost::filesystem::path &path) { - this->LoadDir(path.parent_path())->createDir(path.filename().native(), this->MODE_PUBLIC, 0, 0); + void Test_Rename_Overwrite() { + auto node = CreateNode("/oldname"); + CreateNode("/newname"); + node->rename("/newname"); + EXPECT_EQ(boost::none, this->device->Load("/oldname")); + EXPECT_NE(boost::none, this->device->Load("/newname")); } - void CreateFile(const boost::filesystem::path &path) { - this->LoadDir(path.parent_path())->createAndOpenFile(path.filename().native(), this->MODE_PUBLIC, 0, 0); + void Test_Rename_Overwrite_DoesntHaveSameEntryTwice() { + auto node = CreateNode("/oldname"); + CreateNode("/newname"); + EXPECT_EQ(4u, this->LoadDir("/")->children()->size()); // 4, because of '.' and '..' + node->rename("/newname"); + EXPECT_EQ(3u, this->LoadDir("/")->children()->size()); // 3, because of '.' and '..' } + + void Test_Rename_Overwrite_DirWithFile() { + auto file = CreateFile("/oldname"); + CreateDir("/newname"); + try { + file->rename("/newname"); + EXPECT_TRUE(false); // expect throw + } catch (const fspp::fuse::FuseErrnoException &e) { + EXPECT_EQ(EISDIR, e.getErrno()); + } + EXPECT_NE(boost::none, this->device->Load("/oldname")); + EXPECT_NE(boost::none, this->device->Load("/newname")); + } + + void Test_Rename_Overwrite_FileWithDir() { + auto dir = CreateDir("/oldname"); + CreateFile("/newname"); + try { + dir->rename("/newname"); + EXPECT_TRUE(false); // expect throw + } catch (const fspp::fuse::FuseErrnoException &e) { + EXPECT_EQ(ENOTDIR, e.getErrno()); + } + EXPECT_NE(boost::none, this->device->Load("/oldname")); + EXPECT_NE(boost::none, this->device->Load("/newname")); + } + +protected: + cpputils::unique_ref CreateDir(const boost::filesystem::path &path) { + this->LoadDir(path.parent_path())->createDir(path.filename().native(), this->MODE_PUBLIC, 0, 0); + return this->LoadDir(path); + } + + cpputils::unique_ref CreateFile(const boost::filesystem::path &path) { + this->LoadDir(path.parent_path())->createAndOpenFile(path.filename().native(), this->MODE_PUBLIC, 0, 0); + return this->LoadFile(path); + } + + cpputils::unique_ref CreateSymlink(const boost::filesystem::path &path) { + this->LoadDir(path.parent_path())->createSymlink(path.filename().native(), "/my/symlink/target", 0, 0); + return this->LoadSymlink(path); + }; }; template class FsppNodeTest_File: public FsppNodeTest { public: cpputils::unique_ref CreateNode(const boost::filesystem::path &path) override { - this->LoadDir(path.parent_path())->createAndOpenFile(path.filename().native(), this->MODE_PUBLIC, 0, 0); - return this->LoadFile(path); + return this->CreateFile(path); } }; @@ -118,8 +191,7 @@ template class FsppNodeTest_Dir: public FsppNodeTest { public: cpputils::unique_ref CreateNode(const boost::filesystem::path &path) override { - this->LoadDir(path.parent_path())->createDir(path.filename().native(), this->MODE_PUBLIC, 0, 0); - return this->LoadDir(path); + return this->CreateDir(path); } }; @@ -127,8 +199,7 @@ template class FsppNodeTest_Symlink: public FsppNodeTest { public: cpputils::unique_ref CreateNode(const boost::filesystem::path &path) override { - this->LoadDir(path.parent_path())->createSymlink(path.filename().native(), "/my/symlink/target", 0, 0); - return this->LoadSymlink(path); + return this->CreateSymlink(path); } }; @@ -155,11 +226,18 @@ REGISTER_NODE_TEST_CASES( Rename_TargetParentDirIsFile, Rename_InRoot, Rename_InNested, - Rename_RootToNested, - Rename_NestedToRoot, - Rename_NestedToNested, + Rename_RootToNested_SameName, + Rename_RootToNested_NewName, + Rename_NestedToRoot_SameName, + Rename_NestedToRoot_NewName, + Rename_NestedToNested_SameName, + Rename_NestedToNested_NewName, Rename_ToItself, - Rename_RootDir + Rename_RootDir, + Rename_Overwrite, + Rename_Overwrite_DoesntHaveSameEntryTwice, + Rename_Overwrite_DirWithFile, + Rename_Overwrite_FileWithDir ); #endif