diff --git a/src/common/filesystembase.cpp b/src/common/filesystembase.cpp index b345e099483..f561d624e7e 100644 --- a/src/common/filesystembase.cpp +++ b/src/common/filesystembase.cpp @@ -26,6 +26,8 @@ #include #include +#include + #include #include @@ -49,6 +51,16 @@ namespace OCC { Q_LOGGING_CATEGORY(lcFileSystem, "sync.filesystem", QtInfoMsg) +QByteArray FileSystem::encodeFileName(const QString &fileName) +{ + return fileName.toLocal8Bit(); +} + +QString FileSystem::decodeFileName(const char *localFileName) +{ + return QString::fromLocal8Bit(localFileName); +} + QString FileSystem::longWinPath(const QString &inpath) { #ifndef Q_OS_WIN @@ -197,28 +209,15 @@ bool FileSystem::uncheckedRenameReplace(const QString &originFileName, QString *errorString) { Q_ASSERT(errorString); + #ifndef Q_OS_WIN - bool success; - QFile orig(originFileName); - // We want a rename that also overwrites. QFile::rename does not overwrite. - // Qt 5.1 has QSaveFile::renameOverwrite we could use. - // ### FIXME - success = true; - bool destExists = fileExists(destinationFileName); - if (destExists && !QFile::remove(destinationFileName)) { - *errorString = orig.errorString(); - qCWarning(lcFileSystem) << "Target file could not be removed."; - success = false; - } - if (success) { - success = orig.rename(destinationFileName); - } - if (!success) { - *errorString = orig.errorString(); + std::error_code err; + std::filesystem::rename(originFileName.toStdString(), destinationFileName.toStdString(), err); + if (err) { + *errorString = QString::fromStdString(err.message()); qCWarning(lcFileSystem) << "Renaming temp file to final failed: " << *errorString; return false; } - #else //Q_OS_WIN // You can not overwrite a read-only file on windows. @@ -351,6 +350,22 @@ bool FileSystem::fileExists(const QString &filename, const QFileInfo &fileInfo) return re; } +bool FileSystem::mkpath(const QString &parent, const QString &newDir) +{ +#ifdef Q_OS_WIN + return QDir(parent).mkpath(newDir); +#else // POSIX + std::error_code err; + QString fullPath = parent; + if (!fullPath.endsWith(u'/')) { + fullPath += u'/'; + } + fullPath += newDir; + std::filesystem::create_directories(fullPath.toStdString(), err); + return err.value() == 0; +#endif +} + QString FileSystem::fileSystemForPath(const QString &path) { QString p = path; diff --git a/src/common/filesystembase.h b/src/common/filesystembase.h index 6506b98a8c0..76eaeb4b995 100644 --- a/src/common/filesystembase.h +++ b/src/common/filesystembase.h @@ -47,6 +47,9 @@ OCSYNC_EXPORT Q_DECLARE_LOGGING_CATEGORY(lcFileSystem) namespace FileSystem { OCSYNC_EXPORT Q_NAMESPACE; + QByteArray OCSYNC_EXPORT encodeFileName(const QString &fileName); + QString OCSYNC_EXPORT decodeFileName(const char *localFileName); + /** * List of characters not allowd in filenames on Windows */ @@ -105,6 +108,8 @@ namespace FileSystem { */ bool OCSYNC_EXPORT fileExists(const QString &filename, const QFileInfo & = QFileInfo()); + bool OCSYNC_EXPORT mkpath(const QString &parent, const QString &newDir); + /** * @brief Rename the file \a originFileName to \a destinationFileName. * diff --git a/src/csync/std/c_time.cpp b/src/csync/std/c_time.cpp index 4677f1bcb33..439964b58da 100644 --- a/src/csync/std/c_time.cpp +++ b/src/csync/std/c_time.cpp @@ -23,11 +23,9 @@ #include "common/filesystembase.h" -#include - #ifdef HAVE_UTIMES int c_utimes(const QString &uri, const struct timeval *times) { - int ret = utimes(QFile::encodeName(uri).constData(), times); + int ret = utimes(uri.toLocal8Bit().constData(), times); return ret; } #else // HAVE_UTIMES diff --git a/src/csync/vio/csync_vio_local_unix.cpp b/src/csync/vio/csync_vio_local_unix.cpp index 39a06ceebd3..e866cded635 100644 --- a/src/csync/vio/csync_vio_local_unix.cpp +++ b/src/csync/vio/csync_vio_local_unix.cpp @@ -28,11 +28,11 @@ #include "csync.h" -#include "vio/csync_vio_local.h" +#include "common/filesystembase.h" #include "common/vfs.h" +#include "vio/csync_vio_local.h" #include -#include Q_LOGGING_CATEGORY(lcCSyncVIOLocal, "sync.csync.vio_local", QtInfoMsg) @@ -48,7 +48,7 @@ struct csync_vio_handle_t { csync_vio_handle_t *csync_vio_local_opendir(const QString &name) { std::unique_ptr handle(new csync_vio_handle_t{}); - auto dirname = QFile::encodeName(name); + auto dirname = OCC::FileSystem::encodeFileName(name); handle->dh = opendir(dirname.constData()); if (!handle->dh) { @@ -77,7 +77,7 @@ std::unique_ptr csync_vio_local_readdir(csync_vio_handle_t *h } while (qstrcmp(dirent->d_name, ".") == 0 || qstrcmp(dirent->d_name, "..") == 0); file_stat.reset(new csync_file_stat_t); - file_stat->path = QFile::decodeName(dirent->d_name); + file_stat->path = OCC::FileSystem::decodeFileName(dirent->d_name); /* Check for availability of d_type, see manpage. */ #if defined(_DIRENT_HAVE_D_TYPE) || defined(__APPLE__) @@ -120,8 +120,8 @@ int csync_vio_local_stat(const QString &uri, csync_file_stat_t *buf) { struct stat sb; - if (lstat(QFile::encodeName(uri).constData(), &sb) < 0) { - return -1; + if (lstat(OCC::FileSystem::encodeFileName(uri).constData(), &sb) < 0) { + return -1; } switch (sb.st_mode & S_IFMT) { diff --git a/src/gui/socketapi/socketapi.cpp b/src/gui/socketapi/socketapi.cpp index b2911aa43a0..20a5a2f343b 100644 --- a/src/gui/socketapi/socketapi.cpp +++ b/src/gui/socketapi/socketapi.cpp @@ -235,9 +235,7 @@ void SocketApi::slotReadSocket() static auto invalidListener = QSharedPointer::create(nullptr); const auto listener = _listeners.value(socket, invalidListener); while (socket->canReadLine()) { - // Make sure to normalize the input from the socket to - // make sure that the path will match, especially on OS X. - QString line = QString::fromUtf8(socket->readLine()).normalized(QString::NormalizationForm_C); + QString line = QString::fromUtf8(socket->readLine()); // Note: do NOT use QString::trimmed() here! That will also remove any trailing spaces (which _are_ part of the filename)! line.chop(1); // remove the '\n' diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 4f77705789d..0cb8d810497 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -178,8 +178,8 @@ void PropagateLocalMkdir::start() done(SyncFileItem::NormalError, tr("Can not create local folder %1 because of a local file name clash with %2").arg(newDirStr, QDir::toNativeSeparators(clash.get()))); return; } - QDir localDir(propagator()->localPath()); - if (!localDir.mkpath(_item->_file)) { + + if (!FileSystem::mkpath(propagator()->localPath(), _item->_file)) { done(SyncFileItem::NormalError, tr("could not create folder %1").arg(newDirStr)); return; } diff --git a/test/testfilesystem.cpp b/test/testfilesystem.cpp index 79ae6968d60..e2fb5da7659 100644 --- a/test/testfilesystem.cpp +++ b/test/testfilesystem.cpp @@ -137,6 +137,74 @@ private Q_SLOTS: QVERIFY(tmp.remove()); } + + void testMkdir_data() + { + QTest::addColumn("name"); + + const unsigned char a_umlaut_composed_bytes[] = {0xc3, 0xa4, 0x00}; + const QString a_umlaut_composed = QString::fromUtf8(reinterpret_cast(a_umlaut_composed_bytes)); + const QString a_umlaut_decomposed = a_umlaut_composed.normalized(QString::NormalizationForm_D); + + QTest::newRow("a-umlaut composed") << a_umlaut_composed; + QTest::newRow("a-umlaut decomposed") << a_umlaut_decomposed; + } + + // This is not a full test, it is meant to verify that no nomalization changes are done. + // The implementation of `OCC::FileSystem::mkpath` relies on either Qt (Windows) or + // `std::filesystem` (POSIX), which should be covered by tests of their own. + void testMkdir() + { + QFETCH(QString, name); + + auto tmp = OCC::TestUtils::createTempDir(); + QVERIFY(OCC::FileSystem::mkpath(tmp.path(), name)); + csync_file_stat_t buf; + auto dh = csync_vio_local_opendir(tmp.path()); + QVERIFY(dh); + while (auto fs = csync_vio_local_readdir(dh, nullptr)) { + QCOMPARE(fs->path, name); + QCOMPARE(fs->type, ItemTypeDirectory); + } + csync_vio_local_closedir(dh); + } + + void testRename_data() + { + QTest::addColumn("name"); + + const unsigned char a_umlaut_composed_bytes[] = {0xc3, 0xa4, 0x00}; + const QString a_umlaut_composed = QString::fromUtf8(reinterpret_cast(a_umlaut_composed_bytes)); + const QString a_umlaut_decomposed = a_umlaut_composed.normalized(QString::NormalizationForm_D); + + QTest::newRow("a-umlaut composed") << a_umlaut_composed; + QTest::newRow("a-umlaut decomposed") << a_umlaut_decomposed; + } + + // This is not a full test, it is meant to verify that no nomalization changes are done. + void testRename() + { + QFETCH(QString, name); + + auto tmp = OCC::TestUtils::createTempDir(); + QFile f(tmp.path() + u"/abc"); + QVERIFY(f.open(QFile::WriteOnly)); + QByteArray data("abc"); + QCOMPARE(f.write(data), data.size()); + f.close(); + + QString err; + QVERIFY(OCC::FileSystem::uncheckedRenameReplace(f.fileName(), tmp.path() + u'/' + name, &err)); + + csync_file_stat_t buf; + auto dh = csync_vio_local_opendir(tmp.path()); + QVERIFY(dh); + while (auto fs = csync_vio_local_readdir(dh, nullptr)) { + QCOMPARE(fs->path, name); + QCOMPARE(fs->type, ItemTypeFile); + } + csync_vio_local_closedir(dh); + } }; QTEST_GUILESS_MAIN(TestFileSystem) diff --git a/test/testlocaldiscovery.cpp b/test/testlocaldiscovery.cpp index 58e0b32878a..d71bc4f4bc3 100644 --- a/test/testlocaldiscovery.cpp +++ b/test/testlocaldiscovery.cpp @@ -238,6 +238,83 @@ private Q_SLOTS: QVERIFY(!fakeFolder.currentRemoteState().find(QStringLiteral("C/.foo"))); QVERIFY(!fakeFolder.currentRemoteState().find(QStringLiteral("C/bar"))); } + + void testNameNormalization_data() + { + QTest::addColumn("correct"); + QTest::addColumn("incorrect"); + + const unsigned char a_umlaut_composed_bytes[] = {0xc3, 0xa4, 0x00}; + const QString a_umlaut_composed = QString::fromUtf8(reinterpret_cast(a_umlaut_composed_bytes)); + const QString a_umlaut_decomposed = a_umlaut_composed.normalized(QString::NormalizationForm_D); + + QTest::newRow("a_umlaut decomposed") << a_umlaut_decomposed << a_umlaut_composed; + QTest::newRow("a_umlaut composed") << a_umlaut_composed << a_umlaut_decomposed; + } + + // Test that when a file/directory name on the remote is encoded in NFC, the local name is encoded + // in the same way, and that a subsequent sync does not change anything. And the same for NFD. + void testNameNormalization() + { + QFETCH_GLOBAL(Vfs::Mode, vfsMode); + QFETCH_GLOBAL(bool, filesAreDehydrated); + + QFETCH(QString, correct); + QFETCH(QString, incorrect); + + // Create an empty remote folder + FakeFolder fakeFolder({FileInfo{}}, vfsMode, filesAreDehydrated); + OperationCounter counter(fakeFolder); + + // Create a file with an a-umlout in the "correct" normalization: + fakeFolder.remoteModifier().mkdir(QStringLiteral("P")); + fakeFolder.remoteModifier().mkdir(QStringLiteral("P/A")); + fakeFolder.remoteModifier().insert(QStringLiteral("P/A/") + correct); + + // Same for a directory, holding a "normal" file: + fakeFolder.remoteModifier().mkdir(QStringLiteral("P/B") + correct); + fakeFolder.remoteModifier().insert(QStringLiteral("P/B") + correct + QStringLiteral("/b")); + + LocalDiscoveryTracker tracker; + connect(&fakeFolder.syncEngine(), &SyncEngine::itemCompleted, &tracker, &LocalDiscoveryTracker::slotItemCompleted); + connect(&fakeFolder.syncEngine(), &SyncEngine::finished, &tracker, &LocalDiscoveryTracker::slotSyncFinished); + + // First sync: discover that there are files/directories on the server that are not yet synced to the local end + QVERIFY(fakeFolder.applyLocalModificationsAndSync()); + + // Check that locally we have the file and the directory with the correct names: + { + auto localState = fakeFolder.currentLocalState(); + QVERIFY(localState.find(QStringLiteral("P/A/") + correct) != nullptr); // check if the file exists + QVERIFY(localState.find(QStringLiteral("P/B") + correct + QStringLiteral("/b")) != nullptr); // check if the file exists + } + + counter.reset(); + + qDebug() << "*** MARK"; // Log marker to check if a PUT/DELETE shows up in the second sync + + // Force a full local discovery on the next sync, which forces a walk of the (local) file system, reading back names (and file sizes/mtimes/etc.)... + fakeFolder.syncEngine().setLocalDiscoveryOptions(LocalDiscoveryStyle::DatabaseAndFilesystem, {QStringLiteral("P")}); + tracker.startSyncFullDiscovery(); + + // ... and start the second sync: + QVERIFY(fakeFolder.applyLocalModificationsAndSync()); + + // If the normalization of the file/directory name did not change, no rename/move/etc. should have been detected, so check that the client didn't issue + // any of these operations: + QCOMPARE(counter.nDELETE, 0); + QCOMPARE(counter.nMOVE, 0); + QCOMPARE(counter.nPUT, 0); + + // Check that the remote names are unchanged, and that no "incorrect" names have been introduced: + FileInfo &remoteState = fakeFolder.currentRemoteState(); + QVERIFY(remoteState.find(QStringLiteral("P/A/") + correct) != nullptr); // check if the file still exists in the original normalization + QVERIFY(remoteState.find(QStringLiteral("P/A/") + incorrect) == nullptr); // there should NOT be a file with another normalization + QVERIFY(remoteState.find(QStringLiteral("P/B") + correct + QStringLiteral("/b")) + != nullptr); // check if the directory still exists in the original normalization + QVERIFY(remoteState.find(QStringLiteral("P/B") + incorrect + QStringLiteral("/b")) + == nullptr); // there should NOT be a directory with another normalization + } }; QTEST_GUILESS_MAIN(TestLocalDiscovery) diff --git a/test/testutils/syncenginetestutils.cpp b/test/testutils/syncenginetestutils.cpp index 10bf18890b4..4d1a9a2fbc3 100644 --- a/test/testutils/syncenginetestutils.cpp +++ b/test/testutils/syncenginetestutils.cpp @@ -15,6 +15,7 @@ #include "libsync/syncresult.h" #include +#include using namespace std::chrono_literals; using namespace std::chrono; @@ -1099,34 +1100,41 @@ void FakeFolder::toDisk(QDir &dir, const FileInfo &templateFi) void FakeFolder::fromDisk(QDir &dir, FileInfo &templateFi) { - const auto infoList = dir.entryInfoList(QDir::AllEntries | QDir::NoDotAndDotDot); - for (const auto &diskChild : infoList) { - if (diskChild.isHidden() || diskChild.fileName().startsWith(QStringLiteral(".sync_"))) { - // Skip system files, sqlite db files, sync log, etc. + auto dh = csync_vio_local_opendir(dir.absolutePath()); + if (!dh) { + return; + } + while (true) { + auto dirent = csync_vio_local_readdir(dh, nullptr); + if (!dirent) + break; + if (dirent->type == ItemTypeSkip) + continue; + if (dirent->is_hidden || dirent->path.startsWith(QStringLiteral(".sync_"))) continue; - } - if (diskChild.isDir()) { + QString absolutePathItem = dir.absolutePath() + QDir::separator() + dirent->path; + if (dirent->type == ItemTypeDirectory) { QDir subDir = dir; - subDir.cd(diskChild.fileName()); - FileInfo &subFi = templateFi.children[diskChild.fileName()] = FileInfo { diskChild.fileName() }; - subFi.setLastModified(diskChild.lastModified()); + subDir.cd(dirent->path); + FileInfo &subFi = templateFi.children[dirent->path] = FileInfo{dirent->path}; + subFi.setLastModified(QDateTime::fromSecsSinceEpoch(dirent->modtime, QTimeZone::utc())); fromDisk(subDir, subFi); } else { - FileInfo fi(diskChild.fileName()); + FileInfo fi(dirent->path); fi.isDir = false; - fi.fileSize = diskChild.size(); - fi.isDehydratedPlaceholder = isDehydratedPlaceholder(diskChild.absoluteFilePath()); - fi.setLastModified(diskChild.lastModified()); + fi.fileSize = dirent->size; + fi.isDehydratedPlaceholder = isDehydratedPlaceholder(absolutePathItem); + fi.setLastModified(QDateTime::fromSecsSinceEpoch(dirent->modtime, QTimeZone::utc())); if (fi.isDehydratedPlaceholder) { fi.contentChar = '\0'; fi.contentSize = 0; } else { - QFile f { diskChild.filePath() }; + QFile f{absolutePathItem}; OC_ENFORCE(f.open(QFile::ReadOnly)); auto content = f.read(1); if (content.size() == 0) { - qWarning() << "Empty file at:" << diskChild.filePath(); + qWarning() << "Empty file at:" << dirent->path; fi.contentChar = FileInfo::DefaultContentChar; } else { fi.contentChar = content.at(0); @@ -1137,6 +1145,7 @@ void FakeFolder::fromDisk(QDir &dir, FileInfo &templateFi) templateFi.children.insert(fi.name, fi); } } + csync_vio_local_closedir(dh); } FileInfo &findOrCreateDirs(FileInfo &base, const PathComponents &components) diff --git a/test/testutils/syncenginetestutils.h b/test/testutils/syncenginetestutils.h index 5144b9249e1..4ea218bcb0d 100644 --- a/test/testutils/syncenginetestutils.h +++ b/test/testutils/syncenginetestutils.h @@ -710,6 +710,11 @@ inline char *printDbData(const FileInfo &fi) return QTest::toString(QStringLiteral("FileInfo with %1 files(%2)").arg(files.size()).arg(files.join(QStringLiteral(", ")))); } +/** + * @brief Utility class that count the number of GET/PUT/MOVE/DELETE operations during a sync. + * + * This can be used for subsequent syncs, but the counters need to be reset. + */ struct OperationCounter { int nGET = 0; @@ -717,7 +722,7 @@ struct OperationCounter int nMOVE = 0; int nDELETE = 0; - OperationCounter() {}; + OperationCounter() { } OperationCounter(const OperationCounter &) = delete; OperationCounter(OperationCounter &&) = delete; void operator=(OperationCounter const &) = delete;