hardcoresushi/libcryfs
1
0
Fork 0
Code Issues Pull Requests Releases Activity

- Fix rename bug: When trying to rename a file to a name that already exists, it was deleted instead. This is now fixed.

Browse Source 
- Additional test cases for rename
This commit is contained in:
Sebastian Messmer 2016-03-23 18:03:30 +00:00
parent db53b597d4
commit 2ac47f480b
16 changed files with 222 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
src/cryfs/filesystem/CryNode.cpp
View File

@ -57,22 +57,17 @@ void CryNode::rename(const bf::path &to) {
//We are the root direcory.
throw FuseErrnoException(EBUSY);
}
//TODO More efficient implementation possible: directly rename when it's actually not moved to a different directory
// It's also quite ugly code because in the parent==targetDir case, it depends on _parent not overriding the changes made by targetDir.
auto optOld = (*_parent)->GetChild(_key);
if (optOld == boost::none) {
throw FuseErrnoException(ENOENT);
}
const auto &old = *optOld;
auto mode = old.mode();
auto uid = old.uid();
auto gid = old.gid();
auto lastAccessTime = old.lastAccessTime();
auto lastModificationTime = old.lastModificationTime();
(*_parent)->RemoveChild(_key);
(*_parent)->flush();
auto targetDir = _device->LoadDirBlob(to.parent_path());
targetDir->AddChild(to.filename().native(), _key, getType(), mode, uid, gid, lastAccessTime, lastModificationTime);
auto old = (*_parent)->GetChild(_key);
if (old == boost::none) {
throw FuseErrnoException(EIO);
}
std::string oldName = old->name(); // Store, because the next line invalidates the 'old' object
if (oldName != to.filename().native()) {
targetDir->AddChild(to.filename().native(), old->key(), old->type(), old->mode(), old->uid(), old->gid(),
old->lastAccessTime(), old->lastModificationTime());
(*_parent)->RemoveChild(oldName);
}
}
void CryNode::utimens(timespec lastAccessTime, timespec lastModificationTime) {

4
src/cryfs/filesystem/cachingfsblobstore/DirBlobRef.h
View File

@ -34,6 +34,10 @@ public:
return _base->RemoveChild(key);
}
void RemoveChild(const std::string &name) {
return _base->RemoveChild(name);
}
void flush() {
return _base->flush();
}

6
src/cryfs/filesystem/fsblobstore/DirBlob.cpp
View File

@ -93,6 +93,12 @@ boost::optional<const DirEntry&> DirBlob::GetChild(const Key &key) const {
return _entries.get(key);
}
void DirBlob::RemoveChild(const string &name) {
std::unique_lock<std::mutex> lock(_mutex);
_entries.remove(name);
_changed = true;
}
void DirBlob::RemoveChild(const Key &key) {
std::unique_lock<std::mutex> lock(_mutex);
_entries.remove(key);

2
src/cryfs/filesystem/fsblobstore/DirBlob.h
View File

@ -46,6 +46,8 @@ namespace cryfs {
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 RemoveChild(const std::string &name);
void RemoveChild(const blockstore::Key &key);
void flush();

39
src/cryfs/filesystem/fsblobstore/utils/DirEntryList.cpp
View File

@ -59,9 +59,7 @@ void DirEntryList::add(const string &name, const Key &blobKey, fspp::Dir::EntryT
}
boost::optional<const DirEntry&> DirEntryList::get(const string &name) const {
auto found = std::find_if(_entries.begin(), _entries.end(), [&name] (const DirEntry &entry) {
return entry.name() == name;
});
auto found = _findByName(name);
if (found == _entries.end()) {
return boost::none;
}
@ -69,19 +67,36 @@ boost::optional<const DirEntry&> DirEntryList::get(const string &name) const {
}
boost::optional<const DirEntry&> DirEntryList::get(const Key &key) const {
auto found = _find(key);
auto found = _findByKey(key);
if (found == _entries.end()) {
return boost::none;
}
return *found;
}
void DirEntryList::remove(const Key &key) {
auto found = _find(key);
void DirEntryList::remove(const string &name) {
auto found = _findByName(name);
if (found == _entries.end()) {
throw fspp::fuse::FuseErrnoException(ENOENT);
}
_entries.erase(found);
}
vector<DirEntry>::iterator DirEntryList::_find(const Key &key) {
void DirEntryList::remove(const Key &key) {
auto lowerBound = _findLowerBound(key);
auto upperBound = std::find_if(lowerBound, _entries.end(), [&key] (const DirEntry &entry) {
return entry.key() != key;
});
_entries.erase(lowerBound, upperBound);
}
vector<DirEntry>::const_iterator DirEntryList::_findByName(const string &name) const {
return std::find_if(_entries.begin(), _entries.end(), [&name] (const DirEntry &entry) {
return entry.name() == name;
});
}
vector<DirEntry>::iterator DirEntryList::_findByKey(const Key &key) {
auto found = _findLowerBound(key);
if (found == _entries.end() || found->key() != key) {
throw fspp::fuse::FuseErrnoException(ENOENT);
@ -118,8 +133,8 @@ vector<DirEntry>::iterator DirEntryList::_findFirst(const Key &hint, std::functi
return iter;
}
vector<DirEntry>::const_iterator DirEntryList::_find(const Key &key) const {
return const_cast<DirEntryList*>(this)->_find(key);
vector<DirEntry>::const_iterator DirEntryList::_findByKey(const Key &key) const {
return const_cast<DirEntryList*>(this)->_findByKey(key);
}
size_t DirEntryList::size() const {
@ -135,13 +150,13 @@ DirEntryList::const_iterator DirEntryList::end() const {
}
void DirEntryList::setMode(const Key &key, mode_t mode) {
auto found = _find(key);
auto found = _findByKey(key);
ASSERT ((S_ISREG(mode) && S_ISREG(found->mode())) || (S_ISDIR(mode) && S_ISDIR(found->mode())) || (S_ISLNK(mode)), "Unknown mode in entry");
found->setMode(mode);
}
bool DirEntryList::setUidGid(const Key &key, uid_t uid, gid_t gid) {
auto found = _find(key);
auto found = _findByKey(key);
bool changed = false;
if (uid != (uid_t)-1) {
found->setUid(uid);
@ -155,7 +170,7 @@ bool DirEntryList::setUidGid(const Key &key, uid_t uid, gid_t gid) {
}
void DirEntryList::setAccessTimes(const blockstore::Key &key, timespec lastAccessTime, timespec lastModificationTime) {
auto found = _find(key);
auto found = _findByKey(key);
found->setLastAccessTime(lastAccessTime);
found->setLastModificationTime(lastModificationTime);
}

6
src/cryfs/filesystem/fsblobstore/utils/DirEntryList.h
View File

@ -23,6 +23,7 @@ namespace cryfs {
mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime);
boost::optional<const DirEntry&> get(const std::string &name) const;
boost::optional<const DirEntry&> get(const blockstore::Key &key) const;
void remove(const std::string &name);
void remove(const blockstore::Key &key);
size_t size() const;
@ -36,8 +37,9 @@ namespace cryfs {
private:
uint64_t _serializedSize() const;
bool _hasChild(const std::string &name) const;
std::vector<DirEntry>::iterator _find(const blockstore::Key &key);
std::vector<DirEntry>::const_iterator _find(const blockstore::Key &key) const;
std::vector<DirEntry>::const_iterator _findByName(const std::string &name) const;
std::vector<DirEntry>::iterator _findByKey(const blockstore::Key &key);
std::vector<DirEntry>::const_iterator _findByKey(const blockstore::Key &key) const;
std::vector<DirEntry>::iterator _findUpperBound(const blockstore::Key &key);
std::vector<DirEntry>::iterator _findLowerBound(const blockstore::Key &key);
std::vector<DirEntry>::iterator _findFirst(const blockstore::Key &hint, std::function<bool (const DirEntry&)> pred);

4
src/cryfs/filesystem/parallelaccessfsblobstore/DirBlobRef.h
View File

@ -30,6 +30,10 @@ public:
return _base->RemoveChild(key);
}
void RemoveChild(const std::string &name) {
return _base->RemoveChild(name);
}
void flush() {
return _base->flush();
}

2
src/fspp/fs_interface/Node.h
View File

@ -16,7 +16,7 @@ public:
virtual void chmod(mode_t mode) = 0;
virtual void chown(uid_t uid, gid_t gid) = 0;
virtual void access(int mask) const = 0;
virtual void rename(const boost::filesystem::path &to) = 0;
virtual void rename(const boost::filesystem::path &to) = 0; // 'to' will always be an absolute path
virtual void utimens(const timespec lastAccessTime, const timespec lastModificationTime) = 0;
virtual void remove() = 0;
};

14
src/fspp/fstest/FsTest.h
View File

@ -7,13 +7,17 @@
#include "FsppDirTest.h"
#include "FsppFileTest.h"
#include "FsppSymlinkTest.h"
#include "FsppNodeTest.h"
#include "FsppOpenFileTest.h"
#define FSPP_ADD_FILESYTEM_TESTS(FS_NAME, FIXTURE) \
INSTANTIATE_TYPED_TEST_CASE_P(FS_NAME, FsppDeviceTest, FIXTURE); \
INSTANTIATE_TYPED_TEST_CASE_P(FS_NAME, FsppDirTest, FIXTURE); \
INSTANTIATE_TYPED_TEST_CASE_P(FS_NAME, FsppFileTest, FIXTURE); \
INSTANTIATE_TYPED_TEST_CASE_P(FS_NAME, FsppSymlinkTest, FIXTURE); \
INSTANTIATE_TYPED_TEST_CASE_P(FS_NAME, FsppOpenFileTest, FIXTURE); \
INSTANTIATE_TYPED_TEST_CASE_P(FS_NAME, FsppDeviceTest, FIXTURE); \
INSTANTIATE_TYPED_TEST_CASE_P(FS_NAME, FsppDirTest, FIXTURE); \
INSTANTIATE_TYPED_TEST_CASE_P(FS_NAME, FsppFileTest, FIXTURE); \
INSTANTIATE_TYPED_TEST_CASE_P(FS_NAME, FsppSymlinkTest, FIXTURE); \
INSTANTIATE_TYPED_TEST_CASE_P(FS_NAME, FsppNodeTest_File, FIXTURE); \
INSTANTIATE_TYPED_TEST_CASE_P(FS_NAME, FsppNodeTest_Dir, FIXTURE); \
INSTANTIATE_TYPED_TEST_CASE_P(FS_NAME, FsppNodeTest_Symlink, FIXTURE); \
INSTANTIATE_TYPED_TEST_CASE_P(FS_NAME, FsppOpenFileTest, FIXTURE); \
#endif

6
src/fspp/fstest/FsppDirTest.h
View File

@ -246,8 +246,6 @@ TYPED_TEST_P(FsppDirTest, CreateDir_AlreadyExisting) {
);
}
REGISTER_TYPED_TEST_CASE_P(FsppDirTest,
Children_RootDir_Empty,
Children_RootDir_OneFile_Directly,
@ -283,7 +281,7 @@ REGISTER_TYPED_TEST_CASE_P(FsppDirTest,
//TODO chmod
//TODO chown
//TODO mkdir with uid/gid
//TODO Test permission flags
//TODO createAndOpenFile: all stat values correctly set (1. in the OpenFile instance returned from createAndOpenFile and 2. on an lstat on the file object afterwards)
//TODO Test all operations do (or don't) affect dir timestamps correctly
#endif

4
src/fspp/fstest/FsppFileTest.h
View File

@ -271,7 +271,7 @@ REGISTER_TYPED_TEST_CASE_P(FsppFileTest,
//TODO access
//TODO rename
//TODO unlink
//TODO createAndOpenFile (all stat values correctly set)
//TODO Test all operations do (or don't) affect file timestamps correctly
//TODO Test all operations do (or don't) affect file timestamps correctly (including rename, which shouldn't modify access/modify time, but inode change time)
//TODO (here and in other fstest operations): Test error paths
#endif

135
src/fspp/fstest/FsppNodeTest.h Normal file
View File

@ -0,0 +1,135 @@
#pragma once
#ifndef MESSMER_FSPP_FSTEST_FSPPNODETEST_H_
#define MESSMER_FSPP_FSTEST_FSPPNODETEST_H_
#include "testutils/FileSystemTest.h"
#include <boost/preprocessor/cat.hpp>
#include <boost/preprocessor/variadic/to_seq.hpp>
#include <boost/preprocessor/seq/for_each.hpp>
#include "../fuse/FuseErrnoException.h"
template<class ConcreteFileSystemTestFixture>
class FsppNodeTest: public FileSystemTest<ConcreteFileSystemTestFixture> {
public:
virtual cpputils::unique_ref<fspp::Node> CreateNode(const boost::filesystem::path &path) = 0;
void Test_Rename_TargetParentDirDoesntExist() {
auto node = CreateNode("/oldname");
try {
node->rename("/notexistingdir/newname");
EXPECT_TRUE(false); // Expect it throws an exception
} catch (const fspp::fuse::FuseErrnoException &e) {
EXPECT_EQ(ENOENT, e.getErrno());
}
//Old file should still exist
EXPECT_NE(boost::none, this->device->Load("/oldname"));
}
void Test_Rename_InRoot() {
auto node = CreateNode("/oldname");
node->rename("/newname");
EXPECT_EQ(boost::none, this->device->Load("/oldname"));
EXPECT_NE(boost::none, this->device->Load("/newname"));
}
void Test_Rename_InNested() {
CreateDir("/mydir");
auto node = CreateNode("/mydir/oldname");
node->rename("/mydir/newname");
EXPECT_EQ(boost::none, this->device->Load("/mydir/oldname"));
EXPECT_NE(boost::none, this->device->Load("/mydir/newname"));
}
void Test_Rename_RootToNested() {
CreateDir("/mydir");
auto node = CreateNode("/oldname");
node->rename("/mydir/newname");
EXPECT_EQ(boost::none, this->device->Load("/oldname"));
EXPECT_NE(boost::none, this->device->Load("/mydir/newname"));
}
void Test_Rename_NestedToRoot() {
CreateDir("/mydir");
auto node = CreateNode("/mydir/oldname");
node->rename("/newname");
EXPECT_EQ(boost::none, this->device->Load("/mydir/oldname"));
EXPECT_NE(boost::none, this->device->Load("/newname"));
}
void Test_Rename_NestedToNested() {
CreateDir("/mydir");
CreateDir("/mydir2");
auto node = CreateNode("/mydir/oldname");
node->rename("/mydir2/newname");
EXPECT_EQ(boost::none, this->device->Load("/mydir/oldname"));
EXPECT_NE(boost::none, this->device->Load("/mydir2/newname"));
}
void Test_Rename_ToItself() {
auto node = CreateNode("/oldname");
node->rename("/oldname");
EXPECT_NE(boost::none, this->device->Load("/oldname"));
}
private:
void CreateDir(const boost::filesystem::path &path) {
this->LoadDir(path.parent_path())->createDir(path.filename().native(), this->MODE_PUBLIC, 0, 0);
}
};
template<class ConcreteFileSystemTestFixture>
class FsppNodeTest_File: public FsppNodeTest<ConcreteFileSystemTestFixture> {
public:
cpputils::unique_ref<fspp::Node> 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);
}
};
template<class ConcreteFileSystemTestFixture>
class FsppNodeTest_Dir: public FsppNodeTest<ConcreteFileSystemTestFixture> {
public:
cpputils::unique_ref<fspp::Node> 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);
}
};
template<class ConcreteFileSystemTestFixture>
class FsppNodeTest_Symlink: public FsppNodeTest<ConcreteFileSystemTestFixture> {
public:
cpputils::unique_ref<fspp::Node> 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);
}
};
#define REGISTER_NODE_TEST_CASE(r, Class, Name) \
TYPED_TEST_P(Class, Name) { \
this->BOOST_PP_CAT(Test_,Name)(); \
} \
#define REGISTER_NODE_TEST_CASES_FOR_CLASS(Class, ...) \
TYPED_TEST_CASE_P(Class); \
BOOST_PP_SEQ_FOR_EACH(REGISTER_NODE_TEST_CASE, Class, BOOST_PP_VARIADIC_TO_SEQ(__VA_ARGS__)); \
REGISTER_TYPED_TEST_CASE_P(Class, __VA_ARGS__); \
#define REGISTER_NODE_TEST_CASES(...) \
REGISTER_NODE_TEST_CASES_FOR_CLASS(FsppNodeTest_File, __VA_ARGS__); \
REGISTER_NODE_TEST_CASES_FOR_CLASS(FsppNodeTest_Dir, __VA_ARGS__); \
REGISTER_NODE_TEST_CASES_FOR_CLASS(FsppNodeTest_Symlink, __VA_ARGS__); \
/*
* Register each of the given test cases for all three classes: FsppNodeTest_File, FsppNodeTest_Dir and FsppNodeTest_Symlink
*/
REGISTER_NODE_TEST_CASES(
Rename_TargetParentDirDoesntExist,
Rename_InRoot,
Rename_InNested,
Rename_RootToNested,
Rename_NestedToRoot,
Rename_NestedToNested,
Rename_ToItself
);
#endif

2
src/fspp/fstest/FsppOpenFileTest.h
View File

@ -26,5 +26,7 @@ REGISTER_TYPED_TEST_CASE_P(FsppOpenFileTest,
//TODO Test flush
//TODO Test fsync
//TODO Test fdatasync
//TODO Test stat on file that was just created (i.e. the OpenFile instance returned by createAndOpenFile)
//TODO Test all operations do (or don't) affect file timestamps correctly
#endif

43
src/fspp/fstest/FsppSymlinkTest.h
View File

@ -13,56 +13,33 @@ public:
void CreateSymlink(const std::string &source, const boost::filesystem::path &target) {
this->LoadDir("/")->createSymlink(source, target, 0, 0);
}
void Test_Create_AbsolutePath() {
CreateSymlink("mysymlink", "/my/symlink/target");
}
void Test_Create_RelativePath() {
CreateSymlink("mysymlink", "../target");
}
void Test_Read_AbsolutePath() {
CreateSymlink("mysymlink", "/my/symlink/target");
EXPECT_EQ("/my/symlink/target", this->LoadSymlink("/mysymlink")->target());
}
void Test_Read_RelativePath() {
CreateSymlink("mysymlink", "../target");
EXPECT_EQ("../target", this->LoadSymlink("/mysymlink")->target());
}
void Test_Delete() {
CreateSymlink("mysymlink", "/my/symlink/target");
std::cerr << "1" << std::endl;
EXPECT_NE(boost::none, this->device->Load("/mysymlink"));
std::cerr << "2" << std::endl;
this->LoadSymlink("/mysymlink")->remove();
std::cerr << "3" << std::endl;
EXPECT_EQ(boost::none, this->device->Load("/mysymlink"));
}
};
TYPED_TEST_CASE_P(FsppSymlinkTest);
TYPED_TEST_P(FsppSymlinkTest, Create_AbsolutePath) {
this->Test_Create_AbsolutePath();
this->CreateSymlink("mysymlink", "/my/symlink/target");
}
TYPED_TEST_P(FsppSymlinkTest, Create_RelativePath) {
this->Test_Create_RelativePath();
this->CreateSymlink("mysymlink", "../target");
}
TYPED_TEST_P(FsppSymlinkTest, Read_AbsolutePath) {
this->Test_Read_AbsolutePath();
this->CreateSymlink("mysymlink", "/my/symlink/target");
EXPECT_EQ("/my/symlink/target", this->LoadSymlink("/mysymlink")->target());
}
TYPED_TEST_P(FsppSymlinkTest, Read_RelativePath) {
this->Test_Read_RelativePath();
this->CreateSymlink("mysymlink", "../target");
EXPECT_EQ("../target", this->LoadSymlink("/mysymlink")->target());
}
TYPED_TEST_P(FsppSymlinkTest, Delete) {
this->Test_Delete();
this->CreateSymlink("mysymlink", "/my/symlink/target");
EXPECT_NE(boost::none, this->device->Load("/mysymlink"));
this->LoadSymlink("/mysymlink")->remove();
EXPECT_EQ(boost::none, this->device->Load("/mysymlink"));
}
REGISTER_TYPED_TEST_CASE_P(FsppSymlinkTest,

2
src/fspp/fstest/testutils/FileTest.h
View File

@ -16,6 +16,8 @@ public:
this->LoadDir("/")->createDir("mydir", this->MODE_PUBLIC, 0, 0);
this->LoadDir("/mydir")->createAndOpenFile("mynestedfile", this->MODE_PUBLIC, 0, 0);
file_nested = cpputils::to_unique_ptr(this->LoadFile("/mydir/mynestedfile"));
this->LoadDir("/")->createDir("mydir2", this->MODE_PUBLIC, 0, 0);
}
std::unique_ptr<fspp::File> file_root;
std::unique_ptr<fspp::File> file_nested;

2
src/fspp/fuse/Fuse.cpp
View File

@ -418,6 +418,8 @@ int Fuse::rename(const bf::path &from, const bf::path &to) {
LOG(DEBUG) << "rename(" << from << ", " << to << ")";
#endif
try {
ASSERT(from.is_absolute(), "from has to be an absolute path");
ASSERT(to.is_absolute(), "rename target has to be an absolute path. If this assert throws, we have to add code here that makes the path absolute.");
_fs->rename(from, to);
return 0;
} catch(const cpputils::AssertFailed &e) {