Merge pull request #2781 from nextcloud/cherry_pick_2020_w53

Cherry pick 2020 w53

src/common/filesystembase.cpp

@@ -372,6 +372,7 @@ bool FileSystem::remove(const QString &fileName, QString *errorString)
 
 bool FileSystem::moveToTrash(const QString &fileName, QString *errorString)
 {
+    // TODO: Qt 5.15 bool QFile::moveToTrash()
 #if defined Q_OS_UNIX && !defined Q_OS_MAC
     QString trashPath, trashFilePath, trashInfoPath;
     QString xdgDataHome = QFile::decodeName(qgetenv("XDG_DATA_HOME"));
@@ -497,33 +498,26 @@ bool FileSystem::isJunction(const QString &filename)
 #endif
 }
 
-QString FileSystem::pathtoUNC(const QString &str)
+#ifdef Q_OS_WIN
+QString FileSystem::pathtoUNC(const QString &_str)
 {
-    int len = 0;
-    QString longStr;
-
-    len = str.length();
-    longStr.reserve(len + 4);
-
-    // prepend \\?\ and convert '/' => '\' to support long names
-    if (str[0] == QLatin1Char('/') || str[0] == QLatin1Char('\\')) {
-        // Don't prepend if already UNC
-        if (!(len > 1 && (str[1] == QLatin1Char('/') || str[1] == QLatin1Char('\\')))) {
-            longStr.append(QStringLiteral("\\\\?"));
-        }
-    } else {
-        longStr.append(QStringLiteral("\\\\?\\")); // prepend string by this four magic chars.
+    if (_str.isEmpty()) {
+        return _str;
     }
-    longStr += str;
+    const QString str = QDir::toNativeSeparators(_str);
+    const QLatin1Char sep('\\');
 
-    /* replace all occurences of / with the windows native \ */
+    // we already have a unc path
+    if (str.startsWith(sep + sep)) {
+        return str;
+    }
+    // prepend \\?\ and to support long names
 
-    for (auto &c : longStr) {
-        if (c == QLatin1Char('/')) {
-            c = QLatin1Char('\\');
-        }
+    if (str.at(0) == sep) {
+        return QStringLiteral(R"(\\?)") + str;
     }
-    return longStr;
+    return QStringLiteral(R"(\\?\)") + str;
 }
+#endif
 
 } // namespace OCC

src/common/filesystembase.h

@@ -128,6 +128,19 @@ namespace FileSystem {
      * Returns the file system used at the given path.
      */
     QString fileSystemForPath(const QString &path);
+
+    /*
+     * This function takes a path and converts it to a UNC representation of the
+     * string. That means that it prepends a \\?\ (unless already UNC) and converts
+     * all slashes to backslashes.
+     *
+     * Note the following:
+     *  - The string must be absolute.
+     *  - it needs to contain a drive character to be a valid UNC
+     *  - A conversion is only done if the path len is larger than 245. Otherwise
+     *    the windows API functions work with the normal "unixoid" representation too.
+     */
+    QString OCSYNC_EXPORT pathtoUNC(const QString &str);
 #endif
 
     /**
@@ -144,19 +157,6 @@ namespace FileSystem {
      * Returns whether the file is a junction (windows only)
      */
     bool OCSYNC_EXPORT isJunction(const QString &filename);
-
-    /*
-     * This function takes a path and converts it to a UNC representation of the
-     * string. That means that it prepends a \\?\ (unless already UNC) and converts
-     * all slashes to backslashes.
-     *
-     * Note the following:
-     *  - The string must be absolute.
-     *  - it needs to contain a drive character to be a valid UNC
-     *  - A conversion is only done if the path len is larger than 245. Otherwise
-     *    the windows API functions work with the normal "unixoid" representation too.
-     */
-    QString OCSYNC_EXPORT pathtoUNC(const QString &str);
 }
 
 /** @} */

src/common/utility_win.cpp

@@ -94,7 +94,7 @@ void setLaunchOnStartup_private(const QString &appName, const QString &guiName,
     QString runPath = QLatin1String(runPathC);
     QSettings settings(runPath, QSettings::NativeFormat);
     if (enable) {
-        settings.setValue(appName, QCoreApplication::applicationFilePath().replace(QLatin1Char('/'), QLatin1Char('\\')));
+        settings.setValue(appName, QDir::toNativeSeparators(QCoreApplication::applicationFilePath()));
     } else {
         settings.remove(appName);
     }

src/common/vfs.h

@@ -158,24 +158,24 @@ class OCSYNC_EXPORT Vfs : public QObject
      * If the remote metadata changes, the local placeholder's metadata should possibly
      * change as well.
      */
-    virtual Result<void, QString> updateMetadata(const QString &filePath, time_t modtime, qint64 size, const QByteArray &fileId) = 0;
+    virtual Q_REQUIRED_RESULT Result<void, QString> updateMetadata(const QString &filePath, time_t modtime, qint64 size, const QByteArray &fileId) = 0;
 
     /// Create a new dehydrated placeholder. Called from PropagateDownload.
-    virtual Result<void, QString> createPlaceholder(const SyncFileItem &item) = 0;
+    virtual Q_REQUIRED_RESULT Result<void, QString> createPlaceholder(const SyncFileItem &item) = 0;
 
     /** Convert a hydrated placeholder to a dehydrated one. Called from PropagateDownlaod.
      *
      * This is different from delete+create because preserving some file metadata
      * (like pin states) may be essential for some vfs plugins.
      */
-    virtual Result<void, QString> dehydratePlaceholder(const SyncFileItem &item) = 0;
+    virtual Q_REQUIRED_RESULT Result<void, QString> dehydratePlaceholder(const SyncFileItem &item) = 0;
 
     /** Discovery hook: even unchanged files may need UPDATE_METADATA.
      *
      * For instance cfapi vfs wants local hydrated non-placeholder files to
      * become hydrated placeholder files.
      */
-    virtual bool needsMetadataUpdate(const SyncFileItem &item) = 0;
+    virtual Q_REQUIRED_RESULT bool needsMetadataUpdate(const SyncFileItem &item) = 0;
 
     /** Convert a new file to a hydrated placeholder.
      *
@@ -190,13 +190,13 @@ class OCSYNC_EXPORT Vfs : public QObject
      * new placeholder shall supersede, for rename-replace actions with new downloads,
      * for example.
      */
-    virtual void convertToPlaceholder(
+    virtual Q_REQUIRED_RESULT Result<void, QString> convertToPlaceholder(
         const QString &filename,
         const SyncFileItem &item,
         const QString &replacesFile = QString()) = 0;
 
     /// Determine whether the file at the given absolute path is a dehydrated placeholder.
-    virtual bool isDehydratedPlaceholder(const QString &filePath) = 0;
+    virtual Q_REQUIRED_RESULT bool isDehydratedPlaceholder(const QString &filePath) = 0;
 
     /** Similar to isDehydratedPlaceholder() but used from sync discovery.
      *
@@ -205,7 +205,7 @@ class OCSYNC_EXPORT Vfs : public QObject
      *
      * Returning true means that type was fully determined.
      */
-    virtual bool statTypeVirtualFile(csync_file_stat_t *stat, void *stat_data) = 0;
+    virtual Q_REQUIRED_RESULT bool statTypeVirtualFile(csync_file_stat_t *stat, void *stat_data) = 0;
 
     /** Sets the pin state for the item at a path.
      *
@@ -216,7 +216,7 @@ class OCSYNC_EXPORT Vfs : public QObject
      *
      * folderPath is relative to the sync folder. Can be "" for root folder.
      */
-    virtual bool setPinState(const QString &folderPath, PinState state) = 0;
+    virtual Q_REQUIRED_RESULT bool setPinState(const QString &folderPath, PinState state) = 0;
 
     /** Returns the pin state of an item at a path.
      *
@@ -227,7 +227,7 @@ class OCSYNC_EXPORT Vfs : public QObject
      *
      * Returns none on retrieval error.
      */
-    virtual Optional<PinState> pinState(const QString &folderPath) = 0;
+    virtual Q_REQUIRED_RESULT Optional<PinState> pinState(const QString &folderPath) = 0;
 
     /** Returns availability status of an item at a path.
      *
@@ -236,7 +236,7 @@ class OCSYNC_EXPORT Vfs : public QObject
      *
      * folderPath is relative to the sync folder. Can be "" for root folder.
      */
-    virtual AvailabilityResult availability(const QString &folderPath) = 0;
+    virtual Q_REQUIRED_RESULT AvailabilityResult availability(const QString &folderPath) = 0;
 
 public slots:
     /** Update in-sync state based on SyncFileStatusTracker signal.
@@ -296,7 +296,7 @@ class OCSYNC_EXPORT VfsOff : public Vfs
     Result<void, QString> updateMetadata(const QString &, time_t, qint64, const QByteArray &) override { return {}; }
     Result<void, QString> createPlaceholder(const SyncFileItem &) override { return {}; }
     Result<void, QString> dehydratePlaceholder(const SyncFileItem &) override { return {}; }
-    void convertToPlaceholder(const QString &, const SyncFileItem &, const QString &) override {}
+    Result<void, QString> convertToPlaceholder(const QString &, const SyncFileItem &, const QString &) override { return {}; }
 
     bool needsMetadataUpdate(const SyncFileItem &) override { return false; }
     bool isDehydratedPlaceholder(const QString &) override { return false; }

src/gui/folder.cpp

@@ -1260,6 +1260,7 @@ void Folder::slotAboutToRemoveAllFiles(SyncFileItem::Direction dir, std::functio
         }
         setSyncPaused(oldPaused);
     });
+    connect(this, &Folder::destroyed, msgBox, &QMessageBox::deleteLater);
     msgBox->open();
 }
 

src/gui/generalsettings.cpp

@@ -161,7 +161,10 @@ GeneralSettings::GeneralSettings(QWidget *parent)
         _ui->autostartCheckBox->setDisabled(true);
         _ui->autostartCheckBox->setToolTip(tr("You cannot disable autostart because system-wide autostart is enabled."));
     } else {
-        _ui->autostartCheckBox->setChecked(Utility::hasLaunchOnStartup(Theme::instance()->appName()));
+        const bool hasAutoStart = Utility::hasLaunchOnStartup(Theme::instance()->appName());
+        // make sure the binary location is correctly set
+        slotToggleLaunchOnStartup(hasAutoStart);
+        _ui->autostartCheckBox->setChecked(hasAutoStart);
         connect(_ui->autostartCheckBox, &QAbstractButton::toggled, this, &GeneralSettings::slotToggleLaunchOnStartup);
     }
 

src/libsync/accessmanager.cpp

@@ -90,9 +90,8 @@ QNetworkReply *AccessManager::createRequest(QNetworkAccessManager::Operation op,
     }
 #endif
 
-    HttpLogger::logRequest(newRequest, op, outgoingData);
     const auto reply = QNetworkAccessManager::createRequest(op, newRequest, outgoingData);
-    HttpLogger::logReplyOnFinished(reply);
+    HttpLogger::logRequest(reply, op, outgoingData);
     return reply;
 }
 

src/libsync/filesystem.cpp

@@ -42,22 +42,17 @@ bool FileSystem::fileEquals(const QString &fn1, const QString &fn2)
     }
 
     const int BufferSize = 16 * 1024;
-    char buffer1[BufferSize];
-    char buffer2[BufferSize];
-    do {
-        int r = f1.read(buffer1, BufferSize);
-        if (f2.read(buffer2, BufferSize) != r) {
-            // this should normally not happen: the files are supposed to have the same size.
+    QByteArray buffer1(BufferSize, 0);
+    QByteArray buffer2(BufferSize, 0);
+    // the files have the same size, compare all of it
+    while(!f1.atEnd()){
+        f1.read(buffer1.data(), BufferSize);
+        f2.read(buffer2.data(), BufferSize);
+        if (buffer1 != buffer2) {
             return false;
         }
-        if (r <= 0) {
-            return true;
-        }
-        if (memcmp(buffer1, buffer2, r) != 0) {
-            return false;
-        }
-    } while (true);
-    return false;
+    };
+    return true;
 }
 
 time_t FileSystem::getModTime(const QString &filename)
@@ -130,7 +125,7 @@ qint64 FileSystem::getSize(const QString &filename)
 {
 #ifdef Q_OS_WIN
     if (isLnkFile(filename)) {
-        // Use csync to get the file size. Qt seems unable to get at it.
+        // Qt handles .lnk as symlink... https://doc.qt.io/qt-5/qfileinfo.html#details
         return getSizeWithCsync(filename);
     }
 #endif

src/libsync/httplogger.cpp

@@ -53,7 +53,8 @@ void logHttp(const QByteArray &verb, const QString &url, const QByteArray &id, c
     for (const auto &it : header) {
         stream << it.first << ": ";
         if (it.first == "Authorization") {
-            stream << "[redacted]";
+            stream << (it.second.startsWith("Bearer ") ? "Bearer" : "Basic");
+            stream << " [redacted]";
         } else {
             stream << it.second;
         }
@@ -85,24 +86,9 @@ void logHttp(const QByteArray &verb, const QString &url, const QByteArray &id, c
 
 namespace OCC {
 
-
-void HttpLogger::logReplyOnFinished(const QNetworkReply *reply)
-{
-    if (!lcNetworkHttp().isInfoEnabled()) {
-        return;
-    }
-    QObject::connect(reply, &QNetworkReply::finished, reply, [reply] {
-        logHttp(requestVerb(*reply),
-            reply->url().toString(),
-            reply->request().rawHeader(XRequestId()),
-            reply->header(QNetworkRequest::ContentTypeHeader).toString(),
-            reply->rawHeaderPairs(),
-            const_cast<QNetworkReply *>(reply));
-    });
-}
-
-void HttpLogger::logRequest(const QNetworkRequest &request, QNetworkAccessManager::Operation operation, QIODevice *device)
+void HttpLogger::logRequest(QNetworkReply *reply, QNetworkAccessManager::Operation operation, QIODevice *device)
 {
+    const auto request = reply->request();
     if (!lcNetworkHttp().isInfoEnabled()) {
         return;
     }
@@ -118,6 +104,15 @@ void HttpLogger::logRequest(const QNetworkRequest &request, QNetworkAccessManage
         request.header(QNetworkRequest::ContentTypeHeader).toString(),
         header,
         device);
+
+    QObject::connect(reply, &QNetworkReply::finished, reply, [reply] {
+        logHttp(requestVerb(*reply),
+            reply->url().toString(),
+            reply->request().rawHeader(XRequestId()),
+            reply->header(QNetworkRequest::ContentTypeHeader).toString(),
+            reply->rawHeaderPairs(),
+            reply);
+    });
 }
 
 QByteArray HttpLogger::requestVerb(QNetworkAccessManager::Operation operation, const QNetworkRequest &request)

src/libsync/httplogger.h

@@ -20,8 +20,7 @@
 
 namespace OCC {
 namespace HttpLogger {
-    void OWNCLOUDSYNC_EXPORT logReplyOnFinished(const QNetworkReply *reply);
-    void OWNCLOUDSYNC_EXPORT logRequest(const QNetworkRequest &request, QNetworkAccessManager::Operation operation, QIODevice *device);
+    void OWNCLOUDSYNC_EXPORT logRequest(QNetworkReply *reply, QNetworkAccessManager::Operation operation, QIODevice *device);
 
     /**
     * Helper to construct the HTTP verb used in the request

src/libsync/owncloudpropagator.cpp

@@ -737,7 +737,9 @@ QString OwncloudPropagator::adjustRenamedPath(const QString &original) const
 bool OwncloudPropagator::updateMetadata(const SyncFileItem &item, const QString &localFolderPath, SyncJournalDb &journal, Vfs &vfs)
 {
     QString fsPath = localFolderPath + item.destination();
-    vfs.convertToPlaceholder(fsPath, item);
+    if (!vfs.convertToPlaceholder(fsPath, item)) {
+        return false;
+    }
     auto record = item.toSyncJournalFileRecordWithInode(fsPath);
     return journal.setFileRecord(record);
 }

src/libsync/propagatedownload.cpp

@@ -964,7 +964,11 @@ void PropagateDownloadFile::downloadFinished()
         preserveGroupOwnership(_tmpFile.fileName(), existingFile);
 
         // Make the file a hydrated placeholder if possible
-        propagator()->syncOptions()._vfs->convertToPlaceholder(_tmpFile.fileName(), *_item, fn);
+        const auto result = propagator()->syncOptions()._vfs->convertToPlaceholder(_tmpFile.fileName(), *_item, fn);
+        if (!result) {
+            done(SyncFileItem::NormalError, result.error());
+            return;
+        }
     }
 
     // Apply the remote permissions

src/libsync/syncengine.cpp

@@ -346,7 +346,12 @@ void OCC::SyncEngine::slotItemDiscovered(const OCC::SyncFileItemPtr &item)
 
             // Ensure it's a placeholder file on disk
             if (item->_type == ItemTypeFile) {
-                _syncOptions._vfs->convertToPlaceholder(filePath, *item);
+                const auto result = _syncOptions._vfs->convertToPlaceholder(filePath, *item);
+                if (!result) {
+                    item->_instruction = CSYNC_INSTRUCTION_ERROR;
+                    item->_errorString = tr("Could not update file : %1").arg(result.error());
+                    return;
+                }
             }
 
             // Update on-disk virtual file metadata
@@ -732,10 +737,11 @@ void SyncEngine::slotDiscoveryFinished()
         }
 
         QPointer<QObject> guard = new QObject();
-        auto callback = [this, finish, guard](bool cancel) -> void {
+        QPointer<QObject> self = this;
+        auto callback = [this, self, finish, guard](bool cancel) -> void {
             // use a guard to ensure its only called once...
-            if (!guard)
-            {
+            // qpointer to self to ensure we still exist
+            if (!guard || !self) {
                 return;
             }
             guard->deleteLater();