diff options
author | Felix Weilbach <felix.weilbach@nextcloud.com> | 2021-09-16 17:07:19 +0300 |
---|---|---|
committer | Matthieu Gallien (Rebase PR Action) <matthieu_gallien@yahoo.fr> | 2021-10-14 14:22:59 +0300 |
commit | df11424596a3694898c30fd8b48e9cb058e4e4b0 (patch) | |
tree | 24a114ca2abae5039b45691ff221eb3e085bcf73 | |
parent | af3021913be7c75607c5cce90a81ff1bedda3fb3 (diff) |
Trim trailing spaces before uploading files
Signed-off-by: Felix Weilbach <felix.weilbach@nextcloud.com>
-rw-r--r-- | src/libsync/discovery.cpp | 61 | ||||
-rw-r--r-- | src/libsync/discovery.h | 9 | ||||
-rw-r--r-- | src/libsync/discoveryphase.h | 1 | ||||
-rw-r--r-- | src/libsync/propagateupload.cpp | 14 | ||||
-rw-r--r-- | test/testlocaldiscovery.cpp | 65 |
5 files changed, 142 insertions, 8 deletions
diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 88434229f..ea6c92f7d 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -15,9 +15,12 @@ #include "discovery.h" #include "common/filesystembase.h" #include "common/syncjournaldb.h" +#include "filesystem.h" #include "syncfileitem.h" #include <QDebug> #include <algorithm> +#include <QEventLoop> +#include <QDir> #include <set> #include <QTextCodec> #include "vio/csync_vio_local.h" @@ -34,6 +37,50 @@ namespace OCC { Q_LOGGING_CATEGORY(lcDisco, "sync.discovery", QtInfoMsg) + +bool ProcessDirectoryJob::checkForInvalidFileName(const PathTuple &path, + const std::map<QString, Entries> &entries, Entries &entry) +{ + const auto originalFileName = entry.localEntry.name; + const auto newFileName = originalFileName.trimmed(); + + if (originalFileName == newFileName) { + return true; + } + + const auto entriesIter = entries.find(newFileName); + if (entriesIter != entries.end()) { + QString errorMessage; + const auto newFileNameEntry = entriesIter->second; + if (newFileNameEntry.serverEntry.isValid()) { + errorMessage = tr("File contains trailing spaces and coudn't be renamed, because a file with the same name already exists on the server."); + } + if (newFileNameEntry.localEntry.isValid()) { + errorMessage = tr("File contains trailing spaces and coudn't be renamed, because a file with the same name already exists locally."); + } + + if (!errorMessage.isEmpty()) { + auto item = SyncFileItemPtr::create(); + if (entry.localEntry.isDirectory) { + item->_type = CSyncEnums::ItemTypeDirectory; + } else { + item->_type = CSyncEnums::ItemTypeFile; + } + item->_file = path._target; + item->_originalFile = path._target; + item->_instruction = CSYNC_INSTRUCTION_ERROR; + item->_status = SyncFileItem::NormalError; + item->_errorString = errorMessage; + emit _discoveryData->itemDiscovered(item); + return false; + } + } + + entry.localEntry.renameName = newFileName; + + return true; +} + void ProcessDirectoryJob::start() { qCInfo(lcDisco) << "STARTING" << _currentFolder._server << _queryServer << _currentFolder._local << _queryLocal; @@ -73,12 +120,6 @@ void ProcessDirectoryJob::process() // However, if foo and foo.owncloud exists locally, there'll be "foo" // with local, db, server entries and "foo.owncloud" with only a local // entry. - struct Entries { - QString nameOverride; - SyncJournalFileRecord dbEntry; - RemoteInfo serverEntry; - LocalInfo localEntry; - }; std::map<QString, Entries> entries; for (auto &e : _serverNormalQueryEntries) { entries[e.name].serverEntry = std::move(e); @@ -136,8 +177,8 @@ void ProcessDirectoryJob::process() // // Iterate over entries and process them // - for (const auto &f : entries) { - const auto &e = f.second; + for (auto &f : entries) { + auto &e = f.second; PathTuple path; path = _currentFolder.addName(e.nameOverride.isEmpty() ? f.first : e.nameOverride); @@ -192,6 +233,9 @@ void ProcessDirectoryJob::process() processBlacklisted(path, e.localEntry, e.dbEntry); continue; } + if (!checkForInvalidFileName(path, entries, e)) { + continue; + } processFile(std::move(path), e.localEntry, e.serverEntry, e.dbEntry); } QTimer::singleShot(0, _discoveryData, &DiscoveryPhase::scheduleMoreJobs); @@ -345,6 +389,7 @@ void ProcessDirectoryJob::processFile(PathTuple path, item->_originalFile = path._original; item->_previousSize = dbEntry._fileSize; item->_previousModtime = dbEntry._modtime; + item->_renameTarget = localEntry.renameName; if (dbEntry._modtime == localEntry.modtime && dbEntry._type == ItemTypeVirtualFile && localEntry.type == ItemTypeFile) { item->_type = ItemTypeFile; diff --git a/src/libsync/discovery.h b/src/libsync/discovery.h index e66dc7609..661315916 100644 --- a/src/libsync/discovery.h +++ b/src/libsync/discovery.h @@ -104,6 +104,13 @@ public: SyncFileItemPtr _dirItem; private: + struct Entries + { + QString nameOverride; + SyncJournalFileRecord dbEntry; + RemoteInfo serverEntry; + LocalInfo localEntry; + }; /** Structure representing a path during discovery. A same path may have different value locally * or on the server in case of renames. @@ -143,6 +150,8 @@ private: } }; + bool checkForInvalidFileName(const PathTuple &path, const std::map<QString, Entries> &entries, Entries &entry); + /** Iterate over entries inside the directory (non-recursively). * * Called once _serverEntries and _localEntries are filled diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h index c1fbefa6b..ac599a797 100644 --- a/src/libsync/discoveryphase.h +++ b/src/libsync/discoveryphase.h @@ -70,6 +70,7 @@ struct LocalInfo { /** FileName of the entry (this does not contains any directory or path, just the plain name */ QString name; + QString renameName; time_t modtime = 0; int64_t size = 0; uint64_t inode = 0; diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index 3136603cd..cd9e9db1c 100644 --- a/src/libsync/propagateupload.cpp +++ b/src/libsync/propagateupload.cpp @@ -217,6 +217,20 @@ void PropagateUploadFileCommon::start() const auto slashPosition = path.lastIndexOf('/'); const auto parentPath = slashPosition >= 0 ? path.left(slashPosition) : QString(); + + if (!_item->_renameTarget.isEmpty() && _item->_file != _item->_renameTarget) { + // Try to rename the file + const auto originalFilePathAbsolute = propagator()->fullLocalPath(_item->_file); + const auto newFilePathAbsolute = propagator()->fullLocalPath(_item->_renameTarget); + const auto renameSuccess = QFile::rename(originalFilePathAbsolute, newFilePathAbsolute); + if (!renameSuccess) { + done(SyncFileItem::NormalError, "File contains trailing spaces and couldn't be renamed"); + return; + } + _item->_file = _item->_renameTarget; + _item->_modtime = FileSystem::getModTime(newFilePathAbsolute); + } + SyncJournalFileRecord parentRec; bool ok = propagator()->_journal->getFileRecord(parentPath, &parentRec); if (!ok) { diff --git a/test/testlocaldiscovery.cpp b/test/testlocaldiscovery.cpp index d1831a56d..193399332 100644 --- a/test/testlocaldiscovery.cpp +++ b/test/testlocaldiscovery.cpp @@ -204,6 +204,71 @@ private slots: QVERIFY(!fakeFolder.currentRemoteState().find("C/.foo")); QVERIFY(!fakeFolder.currentRemoteState().find("C/bar")); } + + void testCreateFileWithTrailingSpaces_localAndRemoteTrimmedDoNotExist_renameAndUploadFile() + { + FakeFolder fakeFolder { FileInfo::A12_B12_C12_S12() }; + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + const QString fileWithSpaces1(" foo"); + const QString fileWithSpaces2(" bar "); + const QString fileWithSpaces3("bla "); + + fakeFolder.localModifier().insert(fileWithSpaces1); + fakeFolder.localModifier().insert(fileWithSpaces2); + fakeFolder.localModifier().insert(fileWithSpaces3); + + QVERIFY(fakeFolder.syncOnce()); + + QVERIFY(fakeFolder.currentRemoteState().find(fileWithSpaces1.trimmed())); + QVERIFY(!fakeFolder.currentRemoteState().find(fileWithSpaces1)); + QVERIFY(fakeFolder.currentLocalState().find(fileWithSpaces1.trimmed())); + QVERIFY(!fakeFolder.currentLocalState().find(fileWithSpaces1)); + + QVERIFY(fakeFolder.currentRemoteState().find(fileWithSpaces2.trimmed())); + QVERIFY(!fakeFolder.currentRemoteState().find(fileWithSpaces2)); + QVERIFY(fakeFolder.currentLocalState().find(fileWithSpaces2.trimmed())); + QVERIFY(!fakeFolder.currentLocalState().find(fileWithSpaces2)); + + QVERIFY(fakeFolder.currentRemoteState().find(fileWithSpaces3.trimmed())); + QVERIFY(!fakeFolder.currentRemoteState().find(fileWithSpaces3)); + QVERIFY(fakeFolder.currentLocalState().find(fileWithSpaces3.trimmed())); + QVERIFY(!fakeFolder.currentLocalState().find(fileWithSpaces3)); + } + + void testCreateFileWithTrailingSpaces_localTrimmedDoesExist_dontRenameAndUploadFile() + { + FakeFolder fakeFolder { FileInfo::A12_B12_C12_S12() }; + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + const QString fileWithSpaces(" foo"); + const QString fileTrimmed("foo"); + + fakeFolder.localModifier().insert(fileTrimmed); + QVERIFY(fakeFolder.syncOnce()); + fakeFolder.localModifier().insert(fileWithSpaces); + QVERIFY(!fakeFolder.syncOnce()); + + QVERIFY(fakeFolder.currentRemoteState().find(fileTrimmed)); + QVERIFY(!fakeFolder.currentRemoteState().find(fileWithSpaces)); + QVERIFY(fakeFolder.currentLocalState().find(fileWithSpaces)); + QVERIFY(fakeFolder.currentLocalState().find(fileTrimmed)); + } + + void testCreateFileWithTrailingSpaces_localTrimmedAlsoCreated_dontRenameAndUploadFile() + { + FakeFolder fakeFolder { FileInfo::A12_B12_C12_S12() }; + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + const QString fileWithSpaces(" foo"); + const QString fileTrimmed("foo"); + + fakeFolder.localModifier().insert(fileTrimmed); + fakeFolder.localModifier().insert(fileWithSpaces); + QVERIFY(!fakeFolder.syncOnce()); + + QVERIFY(fakeFolder.currentRemoteState().find(fileTrimmed)); + QVERIFY(!fakeFolder.currentRemoteState().find(fileWithSpaces)); + QVERIFY(fakeFolder.currentLocalState().find(fileWithSpaces)); + QVERIFY(fakeFolder.currentLocalState().find(fileTrimmed)); + } }; QTEST_GUILESS_MAIN(TestLocalDiscovery) |