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.

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

[stable-3.9] Bugfix/unsupported filename on server #5812

Merged
merged 2 commits into from
Jun 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 20 additions & 9 deletions src/gui/invalidfilenamedialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,13 @@ QString illegalCharacterListToString(const QVector<QChar> &illegalCharacters)

namespace OCC {

InvalidFilenameDialog::InvalidFilenameDialog(AccountPtr account, Folder *folder, QString filePath, QWidget *parent)
InvalidFilenameDialog::InvalidFilenameDialog(AccountPtr account, Folder *folder, QString filePath, FileLocation fileLocation, QWidget *parent)
: QDialog(parent)
, _ui(new Ui::InvalidFilenameDialog)
, _account(account)
, _folder(folder)
, _filePath(std::move(filePath))
, _fileLocation(fileLocation)
{
Q_ASSERT(_account);
Q_ASSERT(_folder);
Expand Down Expand Up @@ -103,7 +104,12 @@ InvalidFilenameDialog::InvalidFilenameDialog(AccountPtr account, Folder *folder,
connect(_ui->filenameLineEdit, &QLineEdit::textChanged, this,
&InvalidFilenameDialog::onFilenameLineEditTextChanged);

checkIfAllowedToRename();
if (_fileLocation == FileLocation::NewLocalFile) {
allowRenaming();
_ui->errorLabel->setText({});
} else {
checkIfAllowedToRename();
}
}

InvalidFilenameDialog::~InvalidFilenameDialog() = default;
Expand Down Expand Up @@ -136,13 +142,7 @@ void InvalidFilenameDialog::onCheckIfAllowedToRenameComplete(const QVariantMap &
}
}

_ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(true);
_ui->filenameLineEdit->setEnabled(true);
_ui->filenameLineEdit->selectAll();

const auto filePathFileInfo = QFileInfo(_filePath);
const auto fileName = filePathFileInfo.fileName();
processLeadingOrTrailingSpacesError(fileName);
allowRenaming();
}

bool InvalidFilenameDialog::processLeadingOrTrailingSpacesError(const QString &fileName)
Expand Down Expand Up @@ -184,6 +184,17 @@ void InvalidFilenameDialog::onPropfindPermissionError(QNetworkReply *reply)
onCheckIfAllowedToRenameComplete({}, reply);
}

void InvalidFilenameDialog::allowRenaming()
{
_ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(true);
_ui->filenameLineEdit->setEnabled(true);
_ui->filenameLineEdit->selectAll();

const auto filePathFileInfo = QFileInfo(_filePath);
const auto fileName = filePathFileInfo.fileName();
processLeadingOrTrailingSpacesError(fileName);
}

void InvalidFilenameDialog::useInvalidName()
{
emit acceptedInvalidName(_filePath);
Expand Down
9 changes: 8 additions & 1 deletion src/gui/invalidfilenamedialog.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/gui/invalidfilenamedialog.h

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/gui/invalidfilenamedialog.h

File src/gui/invalidfilenamedialog.h (lines 43): Code does not conform to Custom style guidelines.
* Copyright (C) by Felix Weilbach <felix.weilbach@nextcloud.com>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -35,7 +35,12 @@
Q_OBJECT

public:
explicit InvalidFilenameDialog(AccountPtr account, Folder *folder, QString filePath, QWidget *parent = nullptr);
enum class FileLocation {
Default = 0,
NewLocalFile,
};

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

~InvalidFilenameDialog() override;

Expand All @@ -53,6 +58,7 @@
QString _relativeFilePath;
QString _originalFileName;
QString _newFilename;
FileLocation _fileLocation = FileLocation::Default;

void onFilenameLineEditTextChanged(const QString &text);
void onMoveJobFinished();
Expand All @@ -65,6 +71,7 @@
bool processLeadingOrTrailingSpacesError(const QString &fileName);
void onPropfindPermissionSuccess(const QVariantMap &values);
void onPropfindPermissionError(QNetworkReply *reply = nullptr);
void allowRenaming();
private slots:
void useInvalidName();
};
Expand Down
13 changes: 10 additions & 3 deletions src/gui/tray/activitylistmodel.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/gui/tray/activitylistmodel.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/gui/tray/activitylistmodel.cpp

File src/gui/tray/activitylistmodel.cpp (lines 233, 352, 661, 669, 670): Code does not conform to Custom style guidelines.
* Copyright (C) by Klaas Freitag <freitag@owncloud.com>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -230,6 +230,7 @@
|| a._syncFileItemStatus == SyncFileItem::Restoration
|| a._syncFileItemStatus == SyncFileItem::FileLocked
|| a._syncFileItemStatus == SyncFileItem::FileNameInvalid
|| a._syncFileItemStatus == SyncFileItem::FileNameInvalidOnServer
|| a._syncFileItemStatus == SyncFileItem::FileNameClash) {
colorIconPath.append("state-warning.svg");
return colorIconPath;
Expand Down Expand Up @@ -348,7 +349,8 @@
return !a._links.isEmpty() &&
a._syncFileItemStatus != SyncFileItem::FileNameClash &&
a._syncFileItemStatus != SyncFileItem::Conflict &&
a._syncFileItemStatus != SyncFileItem::FileNameInvalid;
a._syncFileItemStatus != SyncFileItem::FileNameInvalid &&
a._syncFileItemStatus != SyncFileItem::FileNameInvalidOnServer;
case IsCurrentUserFileActivityRole:
return a._isCurrentUserFileActivity;
case ThumbnailRole: {
Expand Down Expand Up @@ -656,15 +658,20 @@
} else if (activity._syncFileItemStatus == SyncFileItem::FileNameClash) {
triggerCaseClashAction(activity);
return;
} else if (activity._syncFileItemStatus == SyncFileItem::FileNameInvalid) {
} else if (activity._syncFileItemStatus == SyncFileItem::FileNameInvalid
|| activity._syncFileItemStatus == SyncFileItem::FileNameInvalidOnServer) {
if (!_currentInvalidFilenameDialog.isNull()) {
_currentInvalidFilenameDialog->close();
}

auto folder = FolderMan::instance()->folder(activity._folder);
const auto folderDir = QDir(folder->path());
const auto fileLocation = activity._syncFileItemStatus == SyncFileItem::FileNameInvalidOnServer
? InvalidFilenameDialog::FileLocation::NewLocalFile
: InvalidFilenameDialog::FileLocation::Default;

_currentInvalidFilenameDialog = new InvalidFilenameDialog(_accountState->account(), folder,
folderDir.filePath(activity._file));
folderDir.filePath(activity._file), fileLocation);
connect(_currentInvalidFilenameDialog, &InvalidFilenameDialog::accepted, folder, [folder]() {
folder->scheduleThisFolderSoon();
});
Expand Down
22 changes: 22 additions & 0 deletions src/libsync/abstractnetworkjob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,11 @@ QString AbstractNetworkJob::errorStringParsingBody(QByteArray *body)
return base;
}

QString AbstractNetworkJob::errorStringParsingBodyException(const QByteArray &body) const
{
return extractException(body);
}

AbstractNetworkJob::~AbstractNetworkJob()
{
setReply(nullptr);
Expand Down Expand Up @@ -423,6 +428,23 @@ QString extractErrorMessage(const QByteArray &errorResponse)
return exception;
}

QString extractException(const QByteArray &errorResponse)
{
QXmlStreamReader reader(errorResponse);
reader.readNextStartElement();
if (reader.name() != QLatin1String("error")) {
return {};
}

while (!reader.atEnd() && !reader.hasError()) {
reader.readNextStartElement();
if (reader.name() == QLatin1String("exception")) {
return reader.readElementText();
}
}
return {};
}

QString errorMessage(const QString &baseError, const QByteArray &body)
{
QString msg = baseError;
Expand Down
22 changes: 22 additions & 0 deletions src/libsync/abstractnetworkjob.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,18 @@ class OWNCLOUDSYNC_EXPORT AbstractNetworkJob : public QObject
*/
QString errorStringParsingBody(QByteArray *body = nullptr);

/** Like errorString, but also checking the reply body for information.
*
* Specifically, sometimes xml bodies have extra error information.
* This function reads the body of the reply and parses out the
* error information, if possible.
*
* \a body is optinally filled with the reply body.
*
* Warning: Needs to call reply()->readAll().
*/
[[nodiscard]] QString errorStringParsingBodyException(const QByteArray &body) const;

/** Make a new request */
void retry();

Expand Down Expand Up @@ -233,6 +245,16 @@ class NetworkJobTimeoutPauser
*/
QString OWNCLOUDSYNC_EXPORT extractErrorMessage(const QByteArray &errorResponse);


/** Gets the SabreDAV-style exception from an error response.
*
* This assumes the response is XML with a 'exception' tag that has a
* 'exception' tag that contains the data to extract.
*
* Returns a null string if no message was found.
*/
[[nodiscard]] QString OWNCLOUDSYNC_EXPORT extractException(const QByteArray &errorResponse);

/** Builds a error message based on the error and the reply body. */
QString OWNCLOUDSYNC_EXPORT errorMessage(const QString &baseError, const QByteArray &body);

Expand Down
1 change: 1 addition & 0 deletions src/libsync/bulkpropagatorjob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,7 @@ void BulkPropagatorJob::handleJobDoneErrors(SyncFileItemPtr item,
case SyncFileItem::FileIgnored:
case SyncFileItem::FileLocked:
case SyncFileItem::FileNameInvalid:
case SyncFileItem::FileNameInvalidOnServer:
case SyncFileItem::FileNameClash:
case SyncFileItem::NoStatus:
case SyncFileItem::NormalError:
Expand Down
2 changes: 2 additions & 0 deletions src/libsync/owncloudpropagator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ void PropagateItemJob::done(const SyncFileItem::Status statusArg, const QString
case SyncFileItem::BlacklistedError:
case SyncFileItem::FileLocked:
case SyncFileItem::FileNameInvalid:
case SyncFileItem::FileNameInvalidOnServer:
case SyncFileItem::FileNameClash:
// nothing
break;
Expand Down Expand Up @@ -1507,6 +1508,7 @@ void PropagateRootDirectory::slotSubJobsFinished(SyncFileItem::Status status)
case SyncFileItem::FileLocked:
case SyncFileItem::Restoration:
case SyncFileItem::FileNameInvalid:
case SyncFileItem::FileNameInvalidOnServer:
case SyncFileItem::DetailError:
case SyncFileItem::Success:
break;
Expand Down
1 change: 1 addition & 0 deletions src/libsync/progressdispatcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ bool Progress::isWarningKind(SyncFileItem::Status kind)
|| kind == SyncFileItem::Conflict || kind == SyncFileItem::Restoration
|| kind == SyncFileItem::DetailError || kind == SyncFileItem::BlacklistedError
|| kind == SyncFileItem::FileLocked || kind == SyncFileItem::FileNameInvalid
|| kind == SyncFileItem::FileNameInvalidOnServer
|| kind == SyncFileItem::FileNameClash;
}

Expand Down
7 changes: 7 additions & 0 deletions src/libsync/propagateupload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,13 @@ void PropagateUploadFileCommon::commonErrorHandling(AbstractNetworkJob *job)
status = SyncFileItem::DetailError;
errorString = tr("Upload of %1 exceeds the quota for the folder").arg(Utility::octetsToString(_fileToUpload._size));
emit propagator()->insufficientRemoteStorage();
} else if (_item->_httpErrorCode == 400) {
const auto exception = job->errorStringParsingBodyException(replyContent);

if (exception.endsWith(QStringLiteral("\\InvalidPath"))) {
errorString = tr("Unable to upload an item with invalid characters");
status = SyncFileItem::FileNameInvalidOnServer;
}
}

abortWithError(status, errorString);
Expand Down
5 changes: 5 additions & 0 deletions src/libsync/syncfileitem.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ class OWNCLOUDSYNC_EXPORT SyncFileItem
*/
FileNameInvalid,

/**
* The filename contains invalid characters and can not be uploaded to the server
*/
FileNameInvalidOnServer,

/**
* There is a file name clash (e.g. attempting to download test.txt when TEST.TXT already exists
* on a platform where the filesystem is case-insensitive
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/syncresult.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

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

View workflow job for this annotation

GitHub Actions / build

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

File src/libsync/syncresult.cpp (lines 144): Code does not conform to Custom style guidelines.
* Copyright (C) by Duncan Mac-Vicar P. <duncan@kde.org>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -141,7 +141,7 @@
if (!_firstItemError) {
_firstItemError = item;
}
} else if (item->_status == SyncFileItem::Conflict || item->_status == SyncFileItem::FileNameInvalid || item->_status == SyncFileItem::FileNameClash) {
} else if (item->_status == SyncFileItem::Conflict || item->_status == SyncFileItem::FileNameInvalid || item->_status == SyncFileItem::FileNameInvalidOnServer || item->_status == SyncFileItem::FileNameClash) {
if (item->_instruction == CSYNC_INSTRUCTION_CONFLICT) {
_numNewConflictItems++;
if (!_firstNewConflictItem) {
Expand Down