Correctly handle errors for unsupported filenames on the server. Allow user to rename...

Signed-off-by: allexzander <blackslayer4@gmail.com>

diff --git a/src/gui/invalidfilenamedialog.cpp b/src/gui/invalidfilenamedialog.cpp
index 366596c61c547d3179da45d3ec35e82f739206ef..235f29650e9dde1fabbe228d1ea51c5d4f65ecba 100644 (file)
--- a/src/gui/invalidfilenamedialog.cpp
+++ b/src/gui/invalidfilenamedialog.cpp
@@ -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);
@@ -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;
@@ -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)
@@ -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);

diff --git a/src/gui/invalidfilenamedialog.h b/src/gui/invalidfilenamedialog.h
index f55b76de38025cdda5372b818e7108c0dfbc9d4d..f2552e5fc10800c96e968c84d5ac0e036657f9aa 100644 (file)
--- a/src/gui/invalidfilenamedialog.h
+++ b/src/gui/invalidfilenamedialog.h
@@ -35,7 +35,12 @@ class InvalidFilenameDialog : public QDialog
     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;
 
@@ -53,6 +58,7 @@ private:
     QString _relativeFilePath;
     QString _originalFileName;
     QString _newFilename;
+    FileLocation _fileLocation = FileLocation::Default;
 
     void onFilenameLineEditTextChanged(const QString &text);
     void onMoveJobFinished();
@@ -65,6 +71,7 @@ private:
     bool processLeadingOrTrailingSpacesError(const QString &fileName);
     void onPropfindPermissionSuccess(const QVariantMap &values);
     void onPropfindPermissionError(QNetworkReply *reply = nullptr);
+    void allowRenaming();
 private slots:
     void useInvalidName();
 };

diff --git a/src/gui/tray/activitylistmodel.cpp b/src/gui/tray/activitylistmodel.cpp
index 8e5f3e45b71708a862be088aa6f31e6956407ddc..6dddc66124c038a4f4c5ed3716e72107a1f12a97 100644 (file)
--- a/src/gui/tray/activitylistmodel.cpp
+++ b/src/gui/tray/activitylistmodel.cpp
@@ -230,6 +230,7 @@ QVariant ActivityListModel::data(const QModelIndex &index, int role) const
                 || 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;
@@ -348,7 +349,8 @@ QVariant ActivityListModel::data(const QModelIndex &index, int role) const
         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: {
@@ -656,15 +658,20 @@ void ActivityListModel::slotTriggerDefaultAction(const int activityIndex)
     } 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();
         });

diff --git a/src/libsync/abstractnetworkjob.cpp b/src/libsync/abstractnetworkjob.cpp
index cb31cd6f4adaf88c6b50f545383596556e41039f..369ae89eb5560de2e66f59988f704b980a2057b5 100644 (file)
--- a/src/libsync/abstractnetworkjob.cpp
+++ b/src/libsync/abstractnetworkjob.cpp
@@ -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);
@@ -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;

diff --git a/src/libsync/abstractnetworkjob.h b/src/libsync/abstractnetworkjob.h
index 2e2a45b22fe86c1e24fe5163cd24e8c109c2347e..6af3041c5d17c9dacaf0945660588f197738b8e8 100644 (file)
--- a/src/libsync/abstractnetworkjob.h
+++ b/src/libsync/abstractnetworkjob.h
@@ -91,6 +91,18 @@ public:
      */
     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();
 
@@ -233,6 +245,16 @@ private:
  */
 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);
 

diff --git a/src/libsync/bulkpropagatorjob.cpp b/src/libsync/bulkpropagatorjob.cpp
index 698962e5a1eb8f32a7b946897baa1ef213aee4f0..0ec04fa963593956ff3e950358e3e111ef5abc12 100644 (file)
--- a/src/libsync/bulkpropagatorjob.cpp
+++ b/src/libsync/bulkpropagatorjob.cpp
@@ -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:

diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp
index 31efa1af2e3c505277af3562cf835b8454f2adc0..b07a514be7ef28dc962832640d4bd436da97e343 100644 (file)
--- a/src/libsync/owncloudpropagator.cpp
+++ b/src/libsync/owncloudpropagator.cpp
@@ -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;

diff --git a/src/libsync/progressdispatcher.cpp b/src/libsync/progressdispatcher.cpp
index abc820fb5fbf88b610653b04d1aa3bbe9307a6b3..88c06296737b6657aa4d51ef8baaabd892a03346 100644 (file)
--- a/src/libsync/progressdispatcher.cpp
+++ b/src/libsync/progressdispatcher.cpp
@@ -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;
 }
 

diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp
index 76e164b6e30f7b94b92dda9c777ad5e1c76927fc..69330dce46021d094612f13c5567a85cd5278b33 100644 (file)
--- a/src/libsync/propagateupload.cpp
+++ b/src/libsync/propagateupload.cpp
@@ -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);

diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h
index b03d6b53b02cdd5a0e83b8f54455402565092a38..eb263f3424bfcf2cb1bdbdcc5db89c9155ecaee8 100644 (file)
--- a/src/libsync/syncfileitem.h
+++ b/src/libsync/syncfileitem.h
@@ -72,6 +72,11 @@ public:
          */
         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

diff --git a/src/libsync/syncresult.cpp b/src/libsync/syncresult.cpp
index 3dede8661363170543a7351f5b18fb13ecd087d3..114010476242d85628deff6c81e0851537317fb2 100644 (file)
--- a/src/libsync/syncresult.cpp
+++ b/src/libsync/syncresult.cpp
@@ -141,7 +141,7 @@ void SyncResult::processCompletedItem(const SyncFileItemPtr &item)
         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) {