Fix rename when overwriting an existing file in the same directory

src/cryfs/filesystem/CryNode.cpp

@@ -62,14 +62,16 @@ void CryNode::rename(const bf::path &to) {
   if (old == boost::none) {
     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
+  fsblobstore::DirEntry oldEntry = *old; // Copying this and not only keeping the reference is necessary, because the operations below (i.e. RenameChild()) might make a reference invalid.
-  if (targetDir->key() != (*_parent)->key() || oldName != to.filename().native()) {
+  auto onOverwritten = [this] (const blockstore::Key &key) {
-    auto onOverwritten = [this] (const blockstore::Key &key) {
       device()->RemoveBlob(key);
-    };
+  };
-    targetDir->AddOrOverwriteChild(to.filename().native(), old->key(), old->type(), old->mode(), old->uid(), old->gid(),
+  if (targetDir->key() == (*_parent)->key()) {
-                        old->lastAccessTime(), old->lastModificationTime(), onOverwritten);
+    targetDir->RenameChild(oldEntry.key(), to.filename().native(), onOverwritten);
-    (*_parent)->RemoveChild(oldName);
+  } else {
+    targetDir->AddOrOverwriteChild(to.filename().native(), oldEntry.key(), oldEntry.type(), oldEntry.mode(), oldEntry.uid(), oldEntry.gid(),
+                                   oldEntry.lastAccessTime(), oldEntry.lastModificationTime(), onOverwritten);
+    (*_parent)->RemoveChild(oldEntry.name());
   }
 }
 

src/cryfs/filesystem/cachingfsblobstore/DirBlobRef.h

@@ -48,6 +48,10 @@ public:
         return _base->AddOrOverwriteChild(name, blobKey, type, mode, uid, gid, lastAccessTime, lastModificationTime, onOverwritten);
     }
 
+    void RenameChild(const blockstore::Key &key, const std::string &newName, std::function<void (const blockstore::Key &key)> onOverwritten) {
+        return _base->RenameChild(key, newName, onOverwritten);
+    }
+
     void statChild(const blockstore::Key &key, struct ::stat *result) const {
         return _base->statChild(key, result);
     }

src/cryfs/filesystem/fsblobstore/DirBlob.cpp

@@ -89,11 +89,16 @@ void DirBlob::AddOrOverwriteChild(const std::string &name, const Key &blobKey, f
                                   mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime,
                                   std::function<void (const blockstore::Key &key)> onOverwritten) {
   std::unique_lock<std::mutex> lock(_mutex);
-
   _entries.addOrOverwrite(name, blobKey, entryType, mode, uid, gid, lastAccessTime, lastModificationTime, onOverwritten);
   _changed = true;
 }
 
+void DirBlob::RenameChild(const blockstore::Key &key, const std::string &newName, std::function<void (const blockstore::Key &key)> onOverwritten) {
+  std::unique_lock<std::mutex> lock(_mutex);
+  _entries.rename(key, newName, onOverwritten);
+  _changed = true;
+}
+
 boost::optional<const DirEntry&> DirBlob::GetChild(const string &name) const {
   std::unique_lock<std::mutex> lock(_mutex);
   return _entries.get(name);

src/cryfs/filesystem/fsblobstore/DirBlob.h

@@ -47,6 +47,8 @@ namespace cryfs {
                           mode_t mode, uid_t uid, gid_t gid, timespec lastAccessTime, timespec lastModificationTime,
                           std::function<void (const blockstore::Key &key)> onOverwritten);
 
+            void RenameChild(const blockstore::Key &key, const std::string &newName, std::function<void (const blockstore::Key &key)> onOverwritten);
+
             void RemoveChild(const std::string &name);
 
             void RemoveChild(const blockstore::Key &key);

src/cryfs/filesystem/fsblobstore/utils/DirEntryList.cpp

@@ -19,9 +19,10 @@ DirEntryList::DirEntryList() : _entries() {
 Data DirEntryList::serialize() const {
     Data serialized(_serializedSize());
     unsigned int offset = 0;
-    for (const auto &entry : _entries) {
+    for (auto iter = _entries.begin(); iter != _entries.end(); ++iter) {
-        entry.serialize(static_cast<uint8_t*>(serialized.dataOffset(offset)));
+        ASSERT(iter == _entries.begin() || std::less<Key>()((iter-1)->key(), iter->key()), "Invariant hurt: Directory entries should be ordered by key and not have duplicate keys.");
-        offset += entry.serializedSize();
+        iter->serialize(static_cast<uint8_t*>(serialized.dataOffset(offset)));
+        offset += iter->serializedSize();
     }
     return serialized;
 }
@@ -39,6 +40,7 @@ void DirEntryList::deserializeFrom(const void *data, uint64_t size) {
     const char *pos = static_cast<const char*>(data);
     while (pos < static_cast<const char*>(data) + size) {
         pos = DirEntry::deserializeAndAddToVector(pos, &_entries);
+        ASSERT(_entries.size() == 1 || std::less<Key>()(_entries[_entries.size()-2].key(), _entries[_entries.size()-1].key()), "Invariant hurt: Directory entries should be ordered by key and not have duplicate keys.");
     }
 }
 
@@ -72,6 +74,22 @@ void DirEntryList::addOrOverwrite(const string &name, const Key &blobKey, fspp::
     }
 }
 
+void DirEntryList::rename(const blockstore::Key &key, const std::string &name, std::function<void (const blockstore::Key &key)> onOverwritten) {
+    auto foundSameName = _findByName(name);
+    if (foundSameName != _entries.end()) {
+        onOverwritten(foundSameName->key());
+        _entries.erase(foundSameName);
+    }
+
+    ASSERT(_findByName(name) == _entries.end(), "There is still an entry with this name. That means there was a duplicate.");
+
+    auto elementToRename = _findByKey(key);
+    std::string oldName = elementToRename->name();
+    ASSERT(elementToRename != _entries.end(), "Didn't find element to rename");
+    elementToRename->setName(name);
+    ASSERT(_findByName(oldName) == _entries.end(), "There is an entry with the old name left. That means there was a duplicate.");
+}
+
 void DirEntryList::_overwrite(vector<DirEntry>::iterator 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) {

src/cryfs/filesystem/fsblobstore/utils/DirEntryList.h

@@ -26,6 +26,7 @@ namespace cryfs {
             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,
                      std::function<void (const blockstore::Key &key)> onOverwritten);
+            void rename(const blockstore::Key &key, const std::string &name, std::function<void (const blockstore::Key &key)> onOverwritten);
             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);

src/cryfs/filesystem/parallelaccessfsblobstore/DirBlobRef.h

@@ -44,6 +44,10 @@ public:
         return _base->AddOrOverwriteChild(name, blobKey, type, mode, uid, gid, lastAccessTime, lastModificationTime, onOverwritten);
     }
 
+    void RenameChild(const blockstore::Key &key, const std::string &newName, std::function<void (const blockstore::Key &key)> onOverwritten) {
+        return _base->RenameChild(key, newName, onOverwritten);
+    }
+
     void statChild(const blockstore::Key &key, struct ::stat *result) const {
         return _base->statChild(key, result);
     }