Skip to content

Commit

Permalink
Merge pull request #7226 from nextcloud/backport/6871/stable-3.12
Browse files Browse the repository at this point in the history
[stable-3.12] Only check for leading/trailing space for files on Windows.
  • Loading branch information
mgallien authored Oct 1, 2024
2 parents 83cd330 + 1accf08 commit 93e9d89
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 4 deletions.
12 changes: 8 additions & 4 deletions src/libsync/discovery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,8 @@ bool ProcessDirectoryJob::handleExcluded(const QString &path, const Entries &ent

const auto fileName = path.mid(path.lastIndexOf('/') + 1);

if (excluded == CSYNC_NOT_EXCLUDED) {
const auto osDoesNotSupportsSpaces = Utility::isWindows();
if (excluded == CSYNC_NOT_EXCLUDED && osDoesNotSupportsSpaces) {
const auto endsWithSpace = fileName.endsWith(QLatin1Char(' '));
const auto startsWithSpace = fileName.startsWith(QLatin1Char(' '));
if (startsWithSpace && endsWithSpace) {
Expand All @@ -274,9 +275,12 @@ bool ProcessDirectoryJob::handleExcluded(const QString &path, const Entries &ent

// we don't need to trigger a warning if trailing/leading space file is already on the server or has already been synced down
// only if the OS supports trailing/leading spaces
const auto wasSyncedAlreadyAndOsSupportsSpaces = !Utility::isWindows() && (entries.serverEntry.isValid() || entries.dbEntry.isValid());
if ((excluded == CSYNC_FILE_EXCLUDE_LEADING_SPACE || excluded == CSYNC_FILE_EXCLUDE_TRAILING_SPACE || excluded == CSYNC_FILE_EXCLUDE_LEADING_AND_TRAILING_SPACE)
&& (wasSyncedAlreadyAndOsSupportsSpaces || _discoveryData->_leadingAndTrailingSpacesFilesAllowed.contains(_discoveryData->_localDir + path))) {
const auto wasSyncedAlready = entries.serverEntry.isValid() || entries.dbEntry.isValid();
const auto hasLeadingOrTrailingSpaces = excluded == CSYNC_FILE_EXCLUDE_LEADING_SPACE
|| excluded == CSYNC_FILE_EXCLUDE_TRAILING_SPACE
|| excluded == CSYNC_FILE_EXCLUDE_LEADING_AND_TRAILING_SPACE;
const auto leadingAndTrailingSpacesFilesAllowed = _discoveryData->_leadingAndTrailingSpacesFilesAllowed.contains(_discoveryData->_localDir + path);
if (hasLeadingOrTrailingSpaces && ((!osDoesNotSupportsSpaces && wasSyncedAlready) || leadingAndTrailingSpacesFilesAllowed)) {
excluded = CSYNC_NOT_EXCLUDED;
}

Expand Down
58 changes: 58 additions & 0 deletions test/testlocaldiscovery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -311,13 +311,23 @@ private slots:

QVERIFY(fakeFolder.syncOnce());

#if defined Q_OS_WINDOWS
QCOMPARE(completeSpy.findItem(fileWithSpaces1)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces2)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces3)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces4)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces5)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces6)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(QStringLiteral(" with spaces "))->_status, SyncFileItem::Status::FileNameInvalid);
#else
QCOMPARE(completeSpy.findItem(fileWithSpaces1)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces2)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces3)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces4)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces5)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces6)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(QStringLiteral(" with spaces "))->_status, SyncFileItem::Status::Success);
#endif

fakeFolder.syncEngine().addAcceptedInvalidFileName(fakeFolder.localPath() + fileWithSpaces1);
fakeFolder.syncEngine().addAcceptedInvalidFileName(fakeFolder.localPath() + fileWithSpaces2);
Expand All @@ -332,13 +342,15 @@ private slots:
fakeFolder.syncEngine().setLocalDiscoveryOptions(LocalDiscoveryStyle::DatabaseAndFilesystem, {QStringLiteral("foo"), QStringLiteral("bar"), QStringLiteral("bla"), QStringLiteral("A/foo"), QStringLiteral("A/bar"), QStringLiteral("A/bla")});
QVERIFY(fakeFolder.syncOnce());

#if defined Q_OS_WINDOWS
QCOMPARE(completeSpy.findItem(fileWithSpaces1)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces2)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces3)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces4)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces5)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces6)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(QStringLiteral(" with spaces "))->_status, SyncFileItem::Status::Success);
#endif
}

void testCreateFileWithTrailingSpaces_remoteDontGetRenamedAutomatically()
Expand Down Expand Up @@ -371,6 +383,44 @@ private slots:
}
}

void testCreateLocalPathsWithLeadingAndTrailingSpaces_syncOnSupportingOs()
{
FakeFolder fakeFolder{FileInfo()};
fakeFolder.localModifier().mkdir("A");
QVERIFY(fakeFolder.syncOnce());
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());

const QString fileWithSpaces1("A/ space");
const QString fileWithSpaces2("A/ space ");
const QString fileWithSpaces3("A/space ");
const QString folderWithSpaces1("A ");
const QString folderWithSpaces2(" B ");

fakeFolder.localModifier().insert(fileWithSpaces1);
fakeFolder.localModifier().insert(fileWithSpaces2);
fakeFolder.localModifier().insert(fileWithSpaces3);
fakeFolder.localModifier().mkdir(folderWithSpaces1);
fakeFolder.localModifier().mkdir(folderWithSpaces2);

ItemCompletedSpy completeSpy(fakeFolder);
completeSpy.clear();

QVERIFY(fakeFolder.syncOnce());

#if !defined Q_OS_WINDOWS
QCOMPARE(completeSpy.findItem(fileWithSpaces1)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces2)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces3)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(folderWithSpaces1)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(folderWithSpaces2)->_status, SyncFileItem::Status::Success);
QVERIFY(fakeFolder.remoteModifier().find(fileWithSpaces1));
QVERIFY(fakeFolder.remoteModifier().find(fileWithSpaces2));
QVERIFY(fakeFolder.remoteModifier().find(fileWithSpaces3));
QVERIFY(fakeFolder.remoteModifier().find(folderWithSpaces1));
QVERIFY(fakeFolder.remoteModifier().find(folderWithSpaces2));
#endif
}

void testCreateFileWithTrailingSpaces_remoteGetRenamedManually()
{
// On Windows we can't create files/folders with leading/trailing spaces locally. So, we have to fail those items. On other OSs - we just sync them down normally.
Expand Down Expand Up @@ -430,7 +480,15 @@ private slots:
QVERIFY(fakeFolder.syncOnce());

QVERIFY(fakeFolder.currentRemoteState().find(fileTrimmed));

#if defined Q_OS_WINDOWS
// no file with spaces on Windows
QVERIFY(!fakeFolder.currentRemoteState().find(fileWithSpaces));
#else
//Linux/Mac OS allows spaces
QVERIFY(fakeFolder.currentRemoteState().find(fileWithSpaces));
#endif

QVERIFY(fakeFolder.currentLocalState().find(fileWithSpaces));
QVERIFY(fakeFolder.currentLocalState().find(fileTrimmed));
}
Expand Down
18 changes: 18 additions & 0 deletions test/testsyncvirtualfiles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -817,12 +817,21 @@ private slots:

QVERIFY(fakeFolder.syncOnce());

#if defined Q_OS_WINDOWS
QCOMPARE(completeSpy.findItem(fileWithSpaces1)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces2)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces3)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces4)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces5)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces6)->_status, SyncFileItem::Status::FileNameInvalid);
#else
QCOMPARE(completeSpy.findItem(fileWithSpaces1)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces2)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces3)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces4)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces5)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces6)->_status, SyncFileItem::Status::Success);
#endif

fakeFolder.syncEngine().addAcceptedInvalidFileName(fakeFolder.localPath() + fileWithSpaces1);
fakeFolder.syncEngine().addAcceptedInvalidFileName(fakeFolder.localPath() + fileWithSpaces2);
Expand All @@ -835,12 +844,21 @@ private slots:

QVERIFY(fakeFolder.syncOnce());

#if defined Q_OS_WINDOWS
QCOMPARE(completeSpy.findItem(fileWithSpaces1)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces2)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces3)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces4)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces5)->_status, SyncFileItem::Status::Success);
QCOMPARE(completeSpy.findItem(fileWithSpaces6)->_status, SyncFileItem::Status::Success);
#else
QCOMPARE(completeSpy.findItem(fileWithSpaces1)->_status, SyncFileItem::Status::NoStatus);
QCOMPARE(completeSpy.findItem(fileWithSpaces2)->_status, SyncFileItem::Status::NoStatus);
QCOMPARE(completeSpy.findItem(fileWithSpaces3)->_status, SyncFileItem::Status::NoStatus);
QCOMPARE(completeSpy.findItem(fileWithSpaces4)->_status, SyncFileItem::Status::NoStatus);
QCOMPARE(completeSpy.findItem(fileWithSpaces5)->_status, SyncFileItem::Status::NoStatus);
QCOMPARE(completeSpy.findItem(fileWithSpaces6)->_status, SyncFileItem::Status::NoStatus);
#endif
}

void testCreateFileWithTrailingSpaces_remoteDontGetRenamedAutomatically()
Expand Down

0 comments on commit 93e9d89

Please sign in to comment.