Skip to content

Commit

Permalink
Merge pull request #6965 from nextcloud/feature/server-forbidden-file…
Browse files Browse the repository at this point in the history
…names

Feature/server forbidden filenames
  • Loading branch information
mgallien authored Aug 21, 2024
2 parents a29bd8c + abb1f89 commit dfe9ada
Show file tree
Hide file tree
Showing 8 changed files with 173 additions and 10 deletions.
40 changes: 37 additions & 3 deletions src/gui/invalidfilenamedialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,12 @@ QString illegalCharacterListToString(const QVector<QChar> &illegalCharacters)

namespace OCC {

InvalidFilenameDialog::InvalidFilenameDialog(AccountPtr account, Folder *folder, QString filePath, FileLocation fileLocation, QWidget *parent)
InvalidFilenameDialog::InvalidFilenameDialog(AccountPtr account,
Folder *folder,
QString filePath,
FileLocation fileLocation,
InvalidMode invalidMode,
QWidget *parent)
: QDialog(parent)
, _ui(new Ui::InvalidFilenameDialog)
, _account(account)
Expand All @@ -89,8 +94,37 @@ InvalidFilenameDialog::InvalidFilenameDialog(AccountPtr account, Folder *folder,
_ui->descriptionLabel->setTextFormat(Qt::PlainText);
_ui->errorLabel->setTextFormat(Qt::PlainText);

_ui->descriptionLabel->setText(tr("The file \"%1\" could not be synced because the name contains characters which are not allowed on this system.").arg(_originalFileName));
_ui->explanationLabel->setText(tr("The following characters are not allowed on the system: * \" | & ? , ; : \\ / ~ < > leading/trailing spaces"));
switch (invalidMode) {
case InvalidMode::SystemInvalid:
_ui->descriptionLabel->setText(tr("The file \"%1\" could not be synced because the name contains characters which are not allowed on this system.").arg(_originalFileName));
_ui->explanationLabel->setText(tr("The following characters are not allowed on the system: * \" | & ? , ; : \\ / ~ < > leading/trailing spaces"));
break;
case InvalidMode::ServerInvalid:
_ui->descriptionLabel->setText(tr("The file \"%1\" could not be synced because the name contains characters which are not allowed on the server.").arg(_originalFileName));

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

auto explanations = QStringList();

if (!forbiddenCharacters.isEmpty()) {
explanations.append(tr("The following characters are not allowed: %1").arg(forbiddenCharacters.join(" ")));
}
if (!forbiddenBasenames.isEmpty()) {
explanations.append(tr("The following basenames are not allowed: %1").arg(forbiddenBasenames.join(" ")));
}
if (!forbiddenFilenames.isEmpty()) {
explanations.append(tr("The following filenames are not allowed: %1").arg(forbiddenFilenames.join(" ")));
}
if (!forbiddenExtensions.isEmpty()) {
explanations.append(tr("The following file extensions are not allowed: %1").arg(forbiddenExtensions.join(" ")));
}
_ui->explanationLabel->setText(explanations.join("\n"));
break;
}
_ui->filenameLineEdit->setText(filePathFileInfo.fileName());

connect(_ui->buttonBox, &QDialogButtonBox::accepted, this, &QDialog::accept);
Expand Down
11 changes: 10 additions & 1 deletion src/gui/invalidfilenamedialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,17 @@ class InvalidFilenameDialog : public QDialog
Default = 0,
NewLocalFile,
};
enum class InvalidMode {
SystemInvalid,
ServerInvalid
};

explicit InvalidFilenameDialog(AccountPtr account, Folder *folder, QString filePath, FileLocation fileLocation = FileLocation::Default, QWidget *parent = nullptr);
explicit InvalidFilenameDialog(AccountPtr account,
Folder *folder,
QString filePath,
FileLocation fileLocation = FileLocation::Default,
InvalidMode invalidMode = InvalidMode::SystemInvalid,
QWidget *parent = nullptr);

~InvalidFilenameDialog() override;

Expand Down
14 changes: 10 additions & 4 deletions src/gui/tray/activitylistmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -729,9 +729,12 @@ void ActivityListModel::slotTriggerDefaultAction(const int activityIndex)
const auto fileLocation = activity._syncFileItemStatus == SyncFileItem::FileNameInvalidOnServer
? InvalidFilenameDialog::FileLocation::NewLocalFile
: InvalidFilenameDialog::FileLocation::Default;
const auto invalidMode = activity._syncFileItemStatus == SyncFileItem::FileNameInvalidOnServer
? InvalidFilenameDialog::InvalidMode::ServerInvalid
: InvalidFilenameDialog::InvalidMode::SystemInvalid;

_currentInvalidFilenameDialog = new InvalidFilenameDialog(_accountState->account(), folder,
folderDir.filePath(activity._file), fileLocation);
folderDir.filePath(activity._file), fileLocation, invalidMode);
connect(_currentInvalidFilenameDialog, &InvalidFilenameDialog::accepted, folder, [folder]() {
folder->scheduleThisFolderSoon();
});
Expand Down Expand Up @@ -840,9 +843,12 @@ void ActivityListModel::slotTriggerAction(const int activityIndex, const int act
if (action._verb == "WEB") {
Utility::openBrowser(QUrl(action._link));
return;
} else if (action._verb == "FIX_CONFLICT_LOCALLY" &&
activity._type == Activity::SyncFileItemType &&
(activity._syncFileItemStatus == SyncFileItem::Conflict || activity._syncFileItemStatus == SyncFileItem::FileNameClash)) {
} else if (((action._verb == "FIX_CONFLICT_LOCALLY" || action._verb == "RENAME_LOCAL_FILE") &&
activity._type == Activity::SyncFileItemType &&
(activity._syncFileItemStatus == SyncFileItem::Conflict ||
activity._syncFileItemStatus == SyncFileItem::FileNameClash ||
activity._syncFileItemStatus == SyncFileItem::FileNameInvalid ||
activity._syncFileItemStatus == SyncFileItem::FileNameInvalidOnServer))) {
slotTriggerDefaultAction(activityIndex);
return;
} else if (action._verb == ActivityLink::WhitelistFolderVerb && !activity._file.isEmpty()) { // _folder == folder alias/name, _file == folder/file path
Expand Down
10 changes: 9 additions & 1 deletion src/gui/tray/usermodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,15 @@ void User::processCompletedSyncItem(const Folder *folder, const SyncFileItemPtr
_activityModel->addIgnoredFileToList(activity);
} else {
// add 'protocol error' to activity list
if (item->_status == SyncFileItem::Status::FileNameInvalid) {
if (item->_status == SyncFileItem::Status::FileNameInvalid || item->_status == SyncFileItem::Status::FileNameInvalidOnServer) {
ActivityLink buttonActivityLink;
buttonActivityLink._label = tr("Rename file");
buttonActivityLink._link = activity._link.toString();
buttonActivityLink._verb = "RENAME_LOCAL_FILE";
buttonActivityLink._primary = true;

activity._links = {buttonActivityLink};

showDesktopNotification(item->_file, activity._subject, activity._id);
} else if (item->_status == SyncFileItem::Conflict || item->_status == SyncFileItem::FileNameClash) {
ActivityLink buttonActivityLink;
Expand Down
20 changes: 20 additions & 0 deletions src/libsync/capabilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,26 @@ QStringList Capabilities::blacklistedFiles() const
return _capabilities["files"].toMap()["blacklisted_files"].toStringList();
}

QStringList Capabilities::forbiddenFilenames() const
{
return _capabilities["files"].toMap()["forbidden_filenames"].toStringList();
}

QStringList Capabilities::forbiddenFilenameCharacters() const
{
return _capabilities["files"].toMap()["forbidden_filename_characters"].toStringList();
}

QStringList Capabilities::forbiddenFilenameBasenames() const
{
return _capabilities["files"].toMap()["forbidden_filename_basenames"].toStringList();
}

QStringList Capabilities::forbiddenFilenameExtensions() const
{
return _capabilities["files"].toMap()["forbidden_filename_extensions"].toStringList();
}

/*-------------------------------------------------------------------------------------*/

// Direct Editing
Expand Down
5 changes: 5 additions & 0 deletions src/libsync/capabilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,11 @@ class OWNCLOUDSYNC_EXPORT Capabilities
*/
[[nodiscard]] QStringList blacklistedFiles() const;

[[nodiscard]] QStringList forbiddenFilenameCharacters() const;
[[nodiscard]] QStringList forbiddenFilenameBasenames() const;
[[nodiscard]] QStringList forbiddenFilenameExtensions() const;
[[nodiscard]] QStringList forbiddenFilenames() const;

/**
* Whether conflict files should remain local (default) or should be uploaded.
*/
Expand Down
44 changes: 43 additions & 1 deletion src/libsync/discovery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,37 @@ bool ProcessDirectoryJob::handleExcluded(const QString &path, const Entries &ent
}

const auto &localName = entries.localEntry.name;
const auto splitName = localName.split('.');
const auto &baseName = splitName.first();
const auto extension = splitName.size() > 1 ? splitName.last() : QString();
const auto accountCaps = _discoveryData->_account->capabilities();
const auto forbiddenFilenames = accountCaps.forbiddenFilenames();
const auto forbiddenBasenames = accountCaps.forbiddenFilenameBasenames();
const auto forbiddenExtensions = accountCaps.forbiddenFilenameExtensions();
const auto forbiddenChars = accountCaps.forbiddenFilenameCharacters();

const auto hasForbiddenFilename = forbiddenFilenames.contains(localName);
const auto hasForbiddenBasename = forbiddenBasenames.contains(baseName);
const auto hasForbiddenExtension = forbiddenExtensions.contains(extension);

auto forbiddenCharMatch = QString{};
const auto containsForbiddenCharacters =
std::any_of(forbiddenChars.cbegin(),
forbiddenChars.cend(),
[&localName, &forbiddenCharMatch](const QString &charPattern) {
if (localName.contains(charPattern)) {
forbiddenCharMatch = charPattern;
return true;
}
return false;
});

if (excluded == CSYNC_NOT_EXCLUDED && !localName.isEmpty()
&& _discoveryData->_serverBlacklistedFiles.contains(localName)) {
&& (_discoveryData->_serverBlacklistedFiles.contains(localName)
|| hasForbiddenFilename
|| hasForbiddenBasename
|| hasForbiddenExtension
|| containsForbiddenCharacters)) {
excluded = CSYNC_FILE_EXCLUDE_SERVER_BLACKLISTED;
isInvalidPattern = true;
}
Expand Down Expand Up @@ -401,6 +430,19 @@ bool ProcessDirectoryJob::handleExcluded(const QString &path, const Entries &ent
break;
case CSYNC_FILE_EXCLUDE_SERVER_BLACKLISTED:
item->_errorString = tr("The filename is blacklisted on the server.");
if (hasForbiddenFilename) {
item->_errorString += tr(" Reason: the entire filename is forbidden.");
}
if (hasForbiddenBasename) {
item->_errorString += tr(" Reason: the filename has a forbidden base name (filename start).");
}
if (hasForbiddenExtension) {
item->_errorString += tr(" Reason: the file has a forbidden extension (.%1).").arg(extension);
}
if (containsForbiddenCharacters) {
item->_errorString += tr(" Reason: the filename contains a forbidden character (%1).").arg(forbiddenCharMatch);
}
item->_status = SyncFileItem::FileNameInvalidOnServer;
break;
}
}
Expand Down
39 changes: 39 additions & 0 deletions test/testlocaldiscovery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,45 @@ private slots:
QVERIFY(!fakeFolder.currentRemoteState().find("C/bar"));
}

// Tests the behavior of forbidden filename detection
void testServerForbiddenFilenames()
{
FakeFolder fakeFolder { FileInfo::A12_B12_C12_S12() };
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());

fakeFolder.syncEngine().account()->setCapabilities({
{ "files", QVariantMap {
{ "forbidden_filenames", QVariantList { ".foo", "bar" } },
{ "forbidden_filename_characters", QVariantList { "_" } },
{ "forbidden_filename_basenames", QVariantList { "base" } },
{ "forbidden_filename_extensions", QVariantList { "ext" } }
} }
});

fakeFolder.localModifier().insert("C/.foo");
fakeFolder.localModifier().insert("C/bar");
fakeFolder.localModifier().insert("C/moo");
fakeFolder.localModifier().insert("C/.moo");
fakeFolder.localModifier().insert("C/potatopotato.txt");
fakeFolder.localModifier().insert("C/potato_potato.txt");
fakeFolder.localModifier().insert("C/basefilename.txt");
fakeFolder.localModifier().insert("C/base.txt");
fakeFolder.localModifier().insert("C/filename.txt");
fakeFolder.localModifier().insert("C/filename.ext");

QVERIFY(fakeFolder.syncOnce());
QVERIFY(fakeFolder.currentRemoteState().find("C/moo"));
QVERIFY(fakeFolder.currentRemoteState().find("C/.moo"));
QVERIFY(!fakeFolder.currentRemoteState().find("C/.foo"));
QVERIFY(!fakeFolder.currentRemoteState().find("C/bar"));
QVERIFY(fakeFolder.currentRemoteState().find("C/potatopotato.txt"));
QVERIFY(!fakeFolder.currentRemoteState().find("C/potato_potato.txt"));
QVERIFY(fakeFolder.currentRemoteState().find("C/basefilename.txt"));
QVERIFY(!fakeFolder.currentRemoteState().find("C/base.txt"));
QVERIFY(fakeFolder.currentRemoteState().find("C/filename.txt"));
QVERIFY(!fakeFolder.currentRemoteState().find("C/filename.ext"));
}

void testCreateFileWithTrailingSpaces_localAndRemoteTrimmedDoNotExist_renameAndUploadFile()
{
FakeFolder fakeFolder{FileInfo{}};
Expand Down

0 comments on commit dfe9ada

Please sign in to comment.