Merge pull request #3778 from nextcloud/feature/trim-trailing-spaces

Trim trailing spaces before uploading files
This commit is contained in:
Matthieu Gallien 2021-10-14 16:00:15 +02:00 committed by GitHub
commit f2811ea027
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 142 additions and 8 deletions

View File

@ -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;

View File

@ -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

View File

@ -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;

View File

@ -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) {

View File

@ -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)