Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/automate windows file name compatibility #7850

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
+200 −92
Open

Feature/automate windows file name compatibility #7850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
42d53a9
enforce trailing and leading spaces for new files
mgallien Jan 14, 2025
70552fa
automatic removing of invalid spaces in file names
mgallien Jan 23, 2025
ae2eeeb
try to detect if naming rules are enabled on server before auto rename
mgallien Feb 12, 2025
1056071
basically try to enforce the windows compatibility as guess from server
mgallien Feb 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 54 additions & 11 deletions src/libsync/discovery.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/libsync/discovery.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/libsync/discovery.cpp 

File src/libsync/discovery.cpp does not conform to Custom style guidelines. (lines 293, 300, 301, 303, 308, 309, 310, 343, 2281, 2289, 2306)
* Copyright (C) by Olivier Goffart <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -272,8 +272,7 @@

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

const auto osDoesNotSupportsSpaces = Utility::isWindows();
if (excluded == CSYNC_NOT_EXCLUDED && osDoesNotSupportsSpaces) {
if (excluded == CSYNC_NOT_EXCLUDED) {
const auto endsWithSpace = fileName.endsWith(QLatin1Char(' '));
const auto startsWithSpace = fileName.startsWith(QLatin1Char(' '));
if (startsWithSpace && endsWithSpace) {
Expand All @@ -291,20 +290,25 @@
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)) {
const auto leadingAndTrailingSpacesFilesAllowed = !_discoveryData->_shouldEnforceWindowsFileNameCompatibility || _discoveryData->_leadingAndTrailingSpacesFilesAllowed.contains(_discoveryData->_localDir + path);
if (hasLeadingOrTrailingSpaces && (wasSyncedAlready || leadingAndTrailingSpacesFilesAllowed)) {
excluded = CSYNC_NOT_EXCLUDED;
}

// FIXME: move to ExcludedFiles 's regexp ?
bool isInvalidPattern = false;
if (excluded == CSYNC_NOT_EXCLUDED && !_discoveryData->_invalidFilenameRx.pattern().isEmpty()) {
if (path.contains(_discoveryData->_invalidFilenameRx)) {
excluded = CSYNC_FILE_EXCLUDE_INVALID_CHAR;
isInvalidPattern = true;
}
if (excluded == CSYNC_NOT_EXCLUDED
&& !wasSyncedAlready
&& !_discoveryData->_invalidFilenameRx.pattern().isEmpty()
&& path.contains(_discoveryData->_invalidFilenameRx)) {

excluded = CSYNC_FILE_EXCLUDE_INVALID_CHAR;
isInvalidPattern = true;
}
if (excluded == CSYNC_NOT_EXCLUDED && _discoveryData->_ignoreHiddenFiles && isHidden) {
if (excluded == CSYNC_NOT_EXCLUDED
&& !entries.dbEntry.isValid()
&& _discoveryData->_ignoreHiddenFiles && isHidden) {

excluded = CSYNC_FILE_EXCLUDE_HIDDEN;
}

Expand Down Expand Up @@ -335,7 +339,8 @@
});

if (excluded == CSYNC_NOT_EXCLUDED && !localName.isEmpty()
&& (_discoveryData->_serverBlacklistedFiles.contains(localName)
&& !wasSyncedAlready
&&(_discoveryData->_serverBlacklistedFiles.contains(localName)
|| hasForbiddenFilename
|| hasForbiddenBasename
|| hasForbiddenExtension
Expand Down Expand Up @@ -415,14 +420,17 @@
case CSYNC_FILE_EXCLUDE_TRAILING_SPACE:
item->_errorString = tr("Filename contains trailing spaces.");
item->_status = SyncFileItem::FileNameInvalid;
maybeRenameForWindowsCompatibility(_discoveryData->_localDir + item->_file, excluded);
break;
case CSYNC_FILE_EXCLUDE_LEADING_SPACE:
item->_errorString = tr("Filename contains leading spaces.");
item->_status = SyncFileItem::FileNameInvalid;
maybeRenameForWindowsCompatibility(_discoveryData->_localDir + item->_file, excluded);
break;
case CSYNC_FILE_EXCLUDE_LEADING_AND_TRAILING_SPACE:
item->_errorString = tr("Filename contains leading and trailing spaces.");
item->_status = SyncFileItem::FileNameInvalid;
maybeRenameForWindowsCompatibility(_discoveryData->_localDir + item->_file, excluded);
break;
case CSYNC_FILE_EXCLUDE_LONG_FILENAME:
item->_errorString = tr("Filename is too long.");
Expand Down Expand Up @@ -462,6 +470,7 @@
}
item->_errorString = reasonString.isEmpty() ? errorString : QStringLiteral("%1 %2").arg(errorString, reasonString);
item->_status = SyncFileItem::FileNameInvalidOnServer;
maybeRenameForWindowsCompatibility(_discoveryData->_localDir + item->_file, excluded);
break;
}
}
Expand Down Expand Up @@ -2269,4 +2278,38 @@
}
}

void ProcessDirectoryJob::maybeRenameForWindowsCompatibility(const QString &absoluteFileName,
CSYNC_EXCLUDE_TYPE excludeReason)
{
if (!_discoveryData->_shouldEnforceWindowsFileNameCompatibility) {
return;
}

const auto fileInfo = QFileInfo{absoluteFileName};
switch (excludeReason)
{
case CSYNC_NOT_EXCLUDED:
case CSYNC_FILE_EXCLUDE_CASE_CLASH_CONFLICT:
case CSYNC_FILE_EXCLUDE_AND_REMOVE:
case CSYNC_FILE_EXCLUDE_CANNOT_ENCODE:
case CSYNC_FILE_EXCLUDE_INVALID_CHAR:
case CSYNC_FILE_EXCLUDE_SERVER_BLACKLISTED:
case CSYNC_FILE_EXCLUDE_HIDDEN:
case CSYNC_FILE_SILENTLY_EXCLUDED:
case CSYNC_FILE_EXCLUDE_STAT_FAILED:
case CSYNC_FILE_EXCLUDE_LONG_FILENAME:
case CSYNC_FILE_EXCLUDE_LIST:
case CSYNC_FILE_EXCLUDE_CONFLICT:
break;
case CSYNC_FILE_EXCLUDE_LEADING_AND_TRAILING_SPACE:
case CSYNC_FILE_EXCLUDE_LEADING_SPACE:
case CSYNC_FILE_EXCLUDE_TRAILING_SPACE:
{
const auto renameTarget = QString{fileInfo.absolutePath() + QStringLiteral("/") + fileInfo.fileName().trimmed()};
FileSystem::rename(absoluteFileName, renameTarget);
break;
}
}
}

}
4 changes: 4 additions & 0 deletions src/libsync/discovery.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/libsync/discovery.h

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/libsync/discovery.h 

File src/libsync/discovery.h does not conform to Custom style guidelines. (lines 259)
* Copyright (C) by Olivier Goffart <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand All @@ -14,7 +14,8 @@

#pragma once

#include <QObject>

Check failure on line 17 in src/libsync/discovery.h

View workflow job for this annotation

GitHub Actions / build

src/libsync/discovery.h:17:10 [clang-diagnostic-error] 

'QObject' file not found
#include "csync_exclude.h"
#include "discoveryphase.h"
#include "syncfileitem.h"
#include "common/asserts.h"
Expand Down Expand Up @@ -255,6 +256,9 @@
*/
void setupDbPinStateActions(SyncJournalFileRecord &record);

void maybeRenameForWindowsCompatibility(const QString &absoluteFileName,
CSYNC_EXCLUDE_TYPE excludeReason);

qint64 _lastSyncTimestamp = 0;

QueryMode _queryServer = QueryMode::NormalQuery;
Expand Down
6 changes: 6 additions & 0 deletions src/libsync/discoveryphase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

#pragma once

#include <QObject>

Check failure on line 17 in src/libsync/discoveryphase.h

View workflow job for this annotation

GitHub Actions / build

src/libsync/discoveryphase.h:17:10 [clang-diagnostic-error] 

'QObject' file not found
#include <QElapsedTimer>
#include <QStringList>
#include <csync.h>
Expand Down Expand Up @@ -326,6 +326,7 @@
QRegularExpression _invalidFilenameRx; // FIXME: maybe move in ExcludedFiles
QStringList _serverBlacklistedFiles; // The blacklist from the capabilities
QStringList _leadingAndTrailingSpacesFilesAllowed;
bool _shouldEnforceWindowsFileNameCompatibility = false;
bool _ignoreHiddenFiles = false;
std::function<bool(const QString &)> _shouldDiscoverLocaly;

Expand All @@ -342,6 +343,11 @@

QStringList _listExclusiveFiles;

QStringList _forbiddenFilenames;
QStringList _forbiddenBasenames;
QStringList _forbiddenExtensions;
QStringList _forbiddenChars;

bool _hasUploadErrorItems = false;
bool _hasDownloadRemovedItems = false;

Expand Down
23 changes: 23 additions & 0 deletions src/libsync/syncengine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/libsync/syncengine.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/libsync/syncengine.cpp 

File src/libsync/syncengine.cpp does not conform to Custom style guidelines. (lines 684, 685, 686)
* Copyright (C) by Duncan Mac-Vicar P. <[email protected]>
* Copyright (C) by Klaas Freitag <[email protected]>
*
Expand Down Expand Up @@ -671,6 +671,24 @@
return;
}

const auto accountCaps = _account->capabilities();
const auto forbiddenFilenames = accountCaps.forbiddenFilenames();
const auto forbiddenBasenames = accountCaps.forbiddenFilenameBasenames();
const auto forbiddenExtensions = accountCaps.forbiddenFilenameExtensions();
const auto forbiddenChars = accountCaps.forbiddenFilenameCharacters();

_discoveryPhase->_forbiddenFilenames = forbiddenFilenames;
_discoveryPhase->_forbiddenBasenames = forbiddenBasenames;
_discoveryPhase->_forbiddenExtensions = forbiddenExtensions;
_discoveryPhase->_forbiddenChars = forbiddenChars;
if (!forbiddenFilenames.isEmpty() &&
!forbiddenBasenames.isEmpty() &&
!forbiddenExtensions.isEmpty() &&
!forbiddenChars.isEmpty()) {
_shouldEnforceWindowsFileNameCompatibility = true;
_discoveryPhase->_shouldEnforceWindowsFileNameCompatibility = _shouldEnforceWindowsFileNameCompatibility;
}

// Check for invalid character in old server version
QString invalidFilenamePattern = _account->capabilities().invalidFilenameRegex();
if (invalidFilenamePattern.isNull()
Expand Down Expand Up @@ -1208,6 +1226,11 @@
_leadingAndTrailingSpacesFilesAllowed.append(filePath);
}

void SyncEngine::setLocalDiscoveryEnforceWindowsFileNameCompatibility(bool value)
{
_shouldEnforceWindowsFileNameCompatibility = value;
}

bool SyncEngine::wasFileTouched(const QString &fn) const
{
// Start from the end (most recent) and look for our path. Check the time just in case.
Expand Down
2 changes: 2 additions & 0 deletions src/libsync/syncengine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

#pragma once

#include <cstdint>

Check failure on line 18 in src/libsync/syncengine.h

View workflow job for this annotation

GitHub Actions / build

src/libsync/syncengine.h:18:10 [clang-diagnostic-error] 

'cstdint' file not found

#include <QMutex>
#include <QThread>
Expand Down Expand Up @@ -159,6 +159,7 @@
*/
void setLocalDiscoveryOptions(OCC::LocalDiscoveryStyle style, std::set<QString> paths = {});
void addAcceptedInvalidFileName(const QString& filePath);
void setLocalDiscoveryEnforceWindowsFileNameCompatibility(bool value);

signals:
// During update, before reconcile
Expand Down Expand Up @@ -409,6 +410,7 @@
std::set<QString> _localDiscoveryPaths;

QStringList _leadingAndTrailingSpacesFilesAllowed;
bool _shouldEnforceWindowsFileNameCompatibility = false;

// Hash of files we have scheduled for later sync runs, along with a
// pointer to the timer which will trigger the sync run for it.
Expand Down
51 changes: 17 additions & 34 deletions test/testlocaldiscovery.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
/*

Check notice on line 1 in test/testlocaldiscovery.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on test/testlocaldiscovery.cpp 

File test/testlocaldiscovery.cpp does not conform to Custom style guidelines. (lines 448, 452, 518, 522)
* This software is in the public domain, furnished "as is", without technical
* support, and with no warranty, express or implied, as to its usefulness for
* any purpose.
*
*/

#include <QtTest>

Check failure on line 8 in test/testlocaldiscovery.cpp

View workflow job for this annotation

GitHub Actions / build

test/testlocaldiscovery.cpp:8:10 [clang-diagnostic-error] 

'QtTest' file not found
#include "syncenginetestutils.h"
#include <syncengine.h>
#include <localdiscoverytracker.h>
Expand Down Expand Up @@ -394,23 +394,13 @@

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(extraFileNameWithSpaces)->_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(extraFileNameWithSpaces)->_status, SyncFileItem::Status::Success);
#endif

fakeFolder.syncEngine().addAcceptedInvalidFileName(fakeFolder.localPath() + fileWithSpaces1);
fakeFolder.syncEngine().addAcceptedInvalidFileName(fakeFolder.localPath() + fileWithSpaces2);
Expand Down Expand Up @@ -455,15 +445,15 @@

QVERIFY(fakeFolder.syncOnce());

if (Utility::isWindows()) {
#if defined Q_OS_WINDOWS
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 {
#else
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
}

void testCreateLocalPathsWithLeadingAndTrailingSpaces_syncOnSupportingOs()
Expand All @@ -490,18 +480,16 @@

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
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(folderWithSpaces1)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(folderWithSpaces2)->_status, SyncFileItem::Status::FileNameInvalid);
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));
}

void testCreateFileWithTrailingSpaces_remoteGetRenamedManually()
Expand All @@ -525,17 +513,17 @@
ItemCompletedSpy completeSpy(fakeFolder);
completeSpy.clear();

QVERIFY(fakeFolder.syncOnce());
QVERIFY(fakeFolder.syncOnce());

if (Utility::isWindows()) {
#if defined Q_OS_WINDOWS
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 {
#else
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.remoteModifier().rename(fileWithSpaces4, fileWithoutSpaces4);
fakeFolder.remoteModifier().rename(fileWithSpaces5, fileWithoutSpaces5);
Expand Down Expand Up @@ -564,13 +552,8 @@

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