Convert VFS file to placeholder again if needed

Note that Windows may convert a placeholder file to a regular file when it is replaced by another file, even if the old and new file (inode, modified time, file size) are identical. WIN32_FIND_DATA::dwReserved0 is only initialised in case of FILE_ATTRIBUTE_REPARSE_POINT is set. This field is not a bit set. Check corrected to only include all cloud reparse points and not other special reparse points. Signed-off-by: Dries Mys <[email protected]>
nextcloud · Jul 15, 2023 · 5856376 · 5856376
1 parent 36832f4
commit 5856376
Show file tree

Hide file tree

Showing 9 changed files with 59 additions and 4 deletions.
diff --git a/src/csync/csync.h b/src/csync/csync.h
@@ -207,6 +207,7 @@ struct OCSYNC_EXPORT csync_file_stat_s {
   bool has_ignored_files BITFIELD(1); // Specify that a directory, or child directory contains ignored files.
   bool is_hidden BITFIELD(1); // Not saved in the DB, only used during discovery for local files.
   bool isE2eEncrypted BITFIELD(1);
+  bool is_metadata_missing BITFIELD(1); // Indicates the file has missing metadata, f.ex. the file is not a placeholder in case of vfs.
 
   QByteArray path;
   QByteArray rename_path;
@@ -233,6 +234,7 @@ struct OCSYNC_EXPORT csync_file_stat_s {
     , has_ignored_files(false)
     , is_hidden(false)
     , isE2eEncrypted(false)
+    , is_metadata_missing(false)
   { }
 };
 

diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp
@@ -601,7 +601,8 @@ void Folder::slotWatchedPathChanged(const QString &path, ChangeReason reason)
                     spurious = false;
                 if (*pinState == PinState::OnlineOnly && record.isFile())
                     spurious = false;
-            }
+            } else
+                spurious = false;
         }
         if (spurious) {
             qCInfo(lcFolder) << "Ignoring spurious notification for file" << relativePath;

diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
@@ -473,7 +473,8 @@ void ProcessDirectoryJob::processFile(PathTuple path,
                               << " | type: " << dbEntry._type << "/" << localEntry.type << "/" << (serverEntry.isDirectory ? ItemTypeDirectory : ItemTypeFile)
                               << " | e2ee: " << dbEntry.isE2eEncrypted() << "/" << serverEntry.isE2eEncrypted()
                               << " | e2eeMangledName: " << dbEntry.e2eMangledName() << "/" << serverEntry.e2eMangledName
-                              << " | file lock: " << localFileIsLocked << "//" << serverFileIsLocked;
+                              << " | file lock: " << localFileIsLocked << "//" << serverFileIsLocked
+                              << " | metadata missing: /" << localEntry.isMetadataMissing << '/';
 
     if (localEntry.isValid()
         && !serverEntry.isValid()
@@ -1073,6 +1074,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
                 item->_type = ItemTypeVirtualFileDehydration;
             } else if (!serverModified
                 && (dbEntry._inode != localEntry.inode
+                    || localEntry.isMetadataMissing
                     || _discoveryData->_syncOptions._vfs->needsMetadataUpdate(*item))) {
                 item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA;
                 item->_direction = SyncFileItem::Down;

diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp
@@ -324,6 +324,7 @@ void DiscoverySingleLocalDirectoryJob::run() {
         i.isHidden = dirent->is_hidden;
         i.isSymLink = dirent->type == ItemTypeSoftLink;
         i.isVirtualFile = dirent->type == ItemTypeVirtualFile || dirent->type == ItemTypeVirtualFileDownload;
+        i.isMetadataMissing = dirent->is_metadata_missing;
         i.type = dirent->type;
         results.push_back(i);
     }

diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h
@@ -101,6 +101,7 @@ struct LocalInfo
     bool isHidden = false;
     bool isVirtualFile = false;
     bool isSymLink = false;
+    bool isMetadataMissing = false;
     [[nodiscard]] bool isValid() const { return !name.isNull(); }
 };
 

diff --git a/src/libsync/vfs/cfapi/cfapiwrapper.cpp b/src/libsync/vfs/cfapi/cfapiwrapper.cpp
@@ -846,3 +846,14 @@ OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::co
         return stateResult;
     }
 }
+
+OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::revertPlaceholder(const QString &path)
+{
+    const qint64 result = CfRevertPlaceholder(handleForPath(path).get(), CF_REVERT_FLAG_NONE, nullptr);
+    if (result != S_OK) {
+        qCWarning(lcCfApiWrapper) << "Couldn't revert placeholder for" << path << ":" << QString::fromWCharArray(_com_error(result).ErrorMessage());
+        return {"Couldn't revert placeholder"};
+    }
+
+    return OCC::Vfs::ConvertToPlaceholderResult::Ok;
+}
diff --git a/src/libsync/vfs/cfapi/cfapiwrapper.h b/src/libsync/vfs/cfapi/cfapiwrapper.h
@@ -97,6 +97,7 @@ NEXTCLOUD_CFAPI_EXPORT Result<void, QString> createPlaceholderInfo(const QString
 NEXTCLOUD_CFAPI_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> updatePlaceholderInfo(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath = QString());
 NEXTCLOUD_CFAPI_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> convertToPlaceholder(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath);
 NEXTCLOUD_CFAPI_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> dehydratePlaceholder(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId);
+NEXTCLOUD_CFAPI_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> revertPlaceholder(const QString &path);
 
 }
 

diff --git a/src/libsync/vfs/cfapi/vfs_cfapi.cpp b/src/libsync/vfs/cfapi/vfs_cfapi.cpp
@@ -259,17 +259,19 @@ bool VfsCfApi::statTypeVirtualFile(csync_file_stat_t *stat, void *statData)
     const auto isPinned = (ffd->dwFileAttributes & FILE_ATTRIBUTE_PINNED) != 0;
     const auto isUnpinned = (ffd->dwFileAttributes & FILE_ATTRIBUTE_UNPINNED) != 0;
     const auto hasReparsePoint = (ffd->dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0;
-    const auto hasCloudTag = (ffd->dwReserved0 & IO_REPARSE_TAG_CLOUD) != 0;
+    const auto hasCloudTag = hasReparsePoint && (ffd->dwReserved0 & ~IO_REPARSE_TAG_CLOUD_MASK) == (IO_REPARSE_TAG_CLOUD & ~IO_REPARSE_TAG_CLOUD_MASK);
 
     const auto isWindowsShortcut = !isDirectory && FileSystem::isLnkFile(stat->path);
 
     const auto isExcludeFile = !isDirectory && FileSystem::isExcludeFile(stat->path);
 
+    stat->is_metadata_missing = !hasCloudTag;
+
     // It's a dir with a reparse point due to the placeholder info (hence the cloud tag)
     // if we don't remove the reparse point flag the discovery will end up thinking
     // it is a file... let's prevent it
     if (isDirectory) {
-        if (hasReparsePoint && hasCloudTag) {
+        if (hasCloudTag) {
             ffd->dwFileAttributes &= ~FILE_ATTRIBUTE_REPARSE_POINT;
         }
         return false;

diff --git a/test/testsynccfapi.cpp b/test/testsynccfapi.cpp
@@ -120,6 +120,40 @@ private slots:
         QTest::newRow("skip local discovery") << false;
     }
 
+    void testUnexpectedNonPlaceholder()
+    {
+        FakeFolder fakeFolder{FileInfo{}};
+        auto vfs = setupVfs(fakeFolder);
+        ItemCompletedSpy completeSpy(fakeFolder);
+
+        // Create a new local (non-placeholder) file
+        fakeFolder.localModifier().insert("file0");
+        fakeFolder.localModifier().insert("file1");
+        QVERIFY(!vfs->pinState("file0").isValid());
+        QVERIFY(!vfs->pinState("file1").isValid());
+
+        // Sync the files: files should be converted to placeholder files
+        QVERIFY(fakeFolder.syncOnce());
+        QVERIFY(vfs->pinState("file0").isValid());
+        QVERIFY(vfs->pinState("file1").isValid());
+
+        // Sync again to ensure items are fully synced, otherwise test may succeed due to those pending changes.
+        QVERIFY(fakeFolder.syncOnce());
+        completeSpy.clear();
+        QVERIFY(fakeFolder.syncOnce());
+        QVERIFY(completeSpy.isEmpty());
+
+        // Convert to regular file (may occur when file is replaced by another one)
+        QVERIFY(cfapi::revertPlaceholder(fakeFolder.localPath() + "file1"));
+        QVERIFY(vfs->pinState("file0").isValid());
+        QVERIFY(!vfs->pinState("file1").isValid());
+
+        // Sync again: file should be correctly converted to placeholders
+        QVERIFY(fakeFolder.syncOnce());
+        QVERIFY(vfs->pinState("file0").isValid());
+        QVERIFY(vfs->pinState("file1").isValid());
+    }
+
     void testVirtualFileLifecycle()
     {
         QFETCH(bool, doLocalDiscovery);