Merge pull request #24911 from krasko78/24908-PreventCorruptedFilesOn…

…Save

#24908: Validate saved files and alert user

src/framework/global/CMakeLists.txt

@@ -121,6 +121,9 @@ set(MODULE_SRC
     ${CMAKE_CURRENT_LIST_DIR}/io/dir.cpp
     ${CMAKE_CURRENT_LIST_DIR}/io/dir.h
 
+    ${CMAKE_CURRENT_LIST_DIR}/io/devtools/allzerosfilecorruptor.cpp
+    ${CMAKE_CURRENT_LIST_DIR}/io/devtools/allzerosfilecorruptor.h
+
     ${CMAKE_CURRENT_LIST_DIR}/serialization/xmlstreamreader.cpp
     ${CMAKE_CURRENT_LIST_DIR}/serialization/xmlstreamreader.h
     ${CMAKE_CURRENT_LIST_DIR}/serialization/xmlstreamwriter.cpp

src/framework/global/io/devtools/allzerosfilecorruptor.cpp

@@ -0,0 +1,37 @@
+/*
+ * SPDX-License-Identifier: GPL-3.0-only
+ * MuseScore-Studio-CLA-applies
+ *
+ * MuseScore Studio
+ * Music Composition & Notation
+ *
+ * Copyright (C) 2021 MuseScore Limited
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+#include "allzerosfilecorruptor.h"
+
+using namespace muse::io;
+
+AllZerosFileCorruptor::AllZerosFileCorruptor(const path_t& filePath)
+    : File(filePath)
+{
+}
+
+size_t AllZerosFileCorruptor::writeData(const uint8_t*, size_t len)
+{
+    // Ignore the actual data and write all zeros so as to corrupt the file.
+    uint8_t* corruptData = new uint8_t[len];
+    memset(corruptData, 0, len);
+    return File::writeData(corruptData, len);
+}

src/framework/global/io/devtools/allzerosfilecorruptor.h

@@ -0,0 +1,35 @@
+/*
+ * SPDX-License-Identifier: GPL-3.0-only
+ * MuseScore-Studio-CLA-applies
+ *
+ * MuseScore Studio
+ * Music Composition & Notation
+ *
+ * Copyright (C) 2021 MuseScore Limited
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+#pragma once
+
+#include "io/file.h"
+
+namespace muse::io {
+class AllZerosFileCorruptor : public File
+{
+public:
+    AllZerosFileCorruptor(const path_t& filePath);
+
+private:
+    size_t writeData(const uint8_t* data, size_t len) override;
+};
+}

src/project/internal/notationproject.cpp

@@ -28,6 +28,7 @@
 #include "global/io/buffer.h"
 #include "global/io/file.h"
 #include "global/io/ioretcodes.h"
+#include "global/io/devtools/allzerosfilecorruptor.h"
 
 #include "engraving/dom/undo.h"
 
@@ -500,7 +501,7 @@ Ret NotationProject::save(const muse::io::path_t& path, SaveMode saveMode)
             suffix = engraving::MSCX;
         }
 
-        return saveScore(path, suffix, false /*generateBackup*/, false /*createThumbnail*/);
+        return saveScore(path, suffix, false /*generateBackup*/, false /*createThumbnail*/, true /*isAutosave*/);
     }
 
     return make_ret(notation::Err::UnknownError);
@@ -546,18 +547,20 @@ Ret NotationProject::writeToDevice(QIODevice* device)
     return ret;
 }
 
-Ret NotationProject::saveScore(const muse::io::path_t& path, const std::string& fileSuffix, bool generateBackup, bool createThumbnail)
+Ret NotationProject::saveScore(const muse::io::path_t& path, const std::string& fileSuffix,
+                               bool generateBackup, bool createThumbnail, bool isAutosave)
 {
     if (!isMuseScoreFile(fileSuffix) && !fileSuffix.empty()) {
         return exportProject(path, fileSuffix);
     }
 
     MscIoMode ioMode = mscIoModeBySuffix(fileSuffix);
 
-    return doSave(path, ioMode, generateBackup, createThumbnail);
+    return doSave(path, ioMode, generateBackup, createThumbnail, isAutosave);
 }
 
-Ret NotationProject::doSave(const muse::io::path_t& path, engraving::MscIoMode ioMode, bool generateBackup, bool createThumbnail)
+Ret NotationProject::doSave(const muse::io::path_t& path, engraving::MscIoMode ioMode,
+                            bool generateBackup, bool createThumbnail, bool isAutosave)
 {
     TRACEFUNC;
 
@@ -592,10 +595,21 @@ Ret NotationProject::doSave(const muse::io::path_t& path, engraving::MscIoMode i
         IF_ASSERT_FAILED(params.mode != MscIoMode::Unknown) {
             return make_ret(Ret::Code::InternalError);
         }
+        if (ioMode == MscIoMode::Zip
+            && !isAutosave
+            && globalConfiguration()->devModeEnabled()
+            && savePath.contains(" - ALL_ZEROS_CORRUPTED.mscz")) {
+            // Create a corrupted file so devs/qa can simulate a saved corrupted file.
+            params.device = new AllZerosFileCorruptor(savePath);
+        }
 
         MscWriter msczWriter(params);
         Ret ret = writeProject(msczWriter, false /*onlySelection*/, createThumbnail);
         msczWriter.close();
+        if (params.device) {
+            delete params.device;
+            params.device = nullptr;
+        }
 
         if (!ret) {
             LOGE() << "failed write project to buffer: " << ret.toString();
@@ -641,10 +655,29 @@ Ret NotationProject::doSave(const muse::io::path_t& path, engraving::MscIoMode i
                 LOGW() << ret.toString();
             }
         } else {
-            Ret ret = fileSystem()->move(savePath, targetContainerPath, true);
+            Ret ret = muse::make_ok();
+
+            ret = fileSystem()->copy(savePath, targetContainerPath, true);
             if (!ret) {
                 return ret;
             }
+
+            if (!isAutosave) {
+                ret = checkSavedFileForCorruption(ioMode, targetContainerPath, targetMainFileName.toQString());
+                if (!ret) {
+                    if (ret.code() == (int)Err::CorruptionUponSavingError) {
+                        // Validate the temporary "saving" file too.
+                        Ret ret2 = checkSavedFileForCorruption(ioMode, savePath, targetMainFileName.toQString());
+                        if (!ret2) {
+                            return ret2;
+                        }
+                    }
+                    return ret;
+                }
+            }
+
+            // Remove the temp save file (not problematic if fails)
+            fileSystem()->remove(savePath);
         }
     }
 
@@ -784,6 +817,43 @@ Ret NotationProject::saveSelectionOnScore(const muse::io::path_t& path)
     return ret;
 }
 
+Ret NotationProject::checkSavedFileForCorruption(MscIoMode ioMode, const muse::io::path_t& path,
+                                                 const muse::io::path_t& scoreFileName)
+{
+    TRACEFUNC;
+
+    if (ioMode != MscIoMode::Zip) {
+        return muse::make_ok();
+    }
+
+    MscReader::Params params;
+    params.filePath = path;
+    params.mainFileName = scoreFileName.toString();
+    params.mode = MscIoMode::Zip;
+
+    MscReader msczReader(params);
+    Ret ret = msczReader.open();
+    if (!ret) {
+        return Ret(static_cast<int>(Err::CorruptionUponSavingError),
+                   muse::mtrc("project/save", "File “%1” could not be opened for validation. %2")
+                   .arg(path.toString(), String(ret.toString().c_str()))
+                   .toStdString());
+    }
+
+    // Try to extract the main score file.
+    ByteArray scoreFile = msczReader.readScoreFile();
+    msczReader.close();
+
+    if (scoreFile.empty()) {
+        return Ret(static_cast<int>(Err::CorruptionUponSavingError),
+                   muse::mtrc("project/save", "“%1” is corrupted or damaged.")
+                   .arg(path.toString())
+                   .toStdString());
+    }
+
+    return muse::make_ok();
+}
+
 Ret NotationProject::exportProject(const muse::io::path_t& path, const std::string& suffix)
 {
     TRACEFUNC;

src/project/internal/notationproject.h

@@ -39,6 +39,8 @@
 #include "projectaudiosettings.h"
 #include "iprojectmigrator.h"
 
+#include "global/iglobalconfiguration.h"
+
 namespace mu::engraving {
 class MscReader;
 class MscWriter;
@@ -54,6 +56,7 @@ class NotationProject : public INotationProject, public muse::Injectable, public
     muse::Inject<INotationReadersRegister> readers = { this };
     muse::Inject<INotationWritersRegister> writers = { this };
     muse::Inject<IProjectMigrator> migrator = { this };
+    muse::Inject<muse::IGlobalConfiguration> globalConfiguration = { this };
 
 public:
     NotationProject(const muse::modularity::ContextPtr& iocCtx)
@@ -109,12 +112,14 @@ class NotationProject : public INotationProject, public muse::Injectable, public
     muse::Ret doImport(const muse::io::path_t& path, const muse::io::path_t& stylePath, bool forceMode);
 
     muse::Ret saveScore(const muse::io::path_t& path, const std::string& fileSuffix, bool generateBackup = true,
-                        bool createThumbnail = true);
+                        bool createThumbnail = true, bool isAutosave = false);
     muse::Ret saveSelectionOnScore(const muse::io::path_t& path = muse::io::path_t());
     muse::Ret exportProject(const muse::io::path_t& path, const std::string& suffix);
-    muse::Ret doSave(const muse::io::path_t& path, engraving::MscIoMode ioMode, bool generateBackup = true, bool createThumbnail = true);
+    muse::Ret doSave(const muse::io::path_t& path, engraving::MscIoMode ioMode, bool generateBackup = true, bool createThumbnail = true,
+                     bool isAutosave = false);
     muse::Ret makeCurrentFileAsBackup();
     muse::Ret writeProject(engraving::MscWriter& msczWriter, bool onlySelection, bool createThumbnail = true);
+    muse::Ret checkSavedFileForCorruption(engraving::MscIoMode ioMode, const muse::io::path_t& path, const muse::io::path_t& scoreFileName);
 
     void listenIfNeedSaveChanges();
     void markAsSaved(const muse::io::path_t& path);

src/project/internal/projectactionscontroller.cpp

@@ -58,6 +58,9 @@ static const muse::Uri UPLOAD_PROGRESS_URI("musescore://project/upload/progress"
 static const QString MUSESCORE_URL_SCHEME("musescore");
 static const QString OPEN_SCORE_URL_HOSTNAME("open-score");
 
+static constexpr int RETRY_SAVE_BTN_ID = int(IInteractive::Button::CustomButton);
+static constexpr int SAVE_AS_BTN_ID    = RETRY_SAVE_BTN_ID + 1;
+
 void ProjectActionsController::init()
 {
     dispatcher()->reg(this, "file-new", this, &ProjectActionsController::newProject);
@@ -930,7 +933,23 @@ bool ProjectActionsController::saveProjectLocally(const muse::io::path_t& filePa
 
     if (!ret) {
         LOGE() << ret.toString();
-        warnScoreCouldnotBeSaved(ret);
+        if (ret.code() != (int)Err::CorruptionUponSavingError) {
+            warnScoreCouldnotBeSaved(ret);
+        } else {
+            switch (warnScoreHasBecomeCorruptedAfterSave(ret)) {
+            case RETRY_SAVE_BTN_ID:
+                async::Async::call(this, [this, filePath, saveMode]() {
+                    saveProjectLocally(filePath, saveMode);
+                });
+                break;
+
+            case SAVE_AS_BTN_ID:
+                async::Async::call(this, [this]() {
+                    saveProject(SaveMode::SaveAs);
+                });
+                break;
+            }
+        }
         return false;
     }
 
@@ -1577,6 +1596,40 @@ void ProjectActionsController::warnScoreCouldnotBeSaved(const std::string& error
     interactive()->warning(muse::trc("project/save", "Your score could not be saved"), errorText);
 }
 
+int ProjectActionsController::warnScoreHasBecomeCorruptedAfterSave(const Ret& ret)
+{
+    const QString errDetailsMessage = QString::fromStdString(ret.toString()).toHtmlEscaped();
+
+    const QString supportForumLink = String("<a href=\"%1\" style=\"text-decoration: none\">musescore.org</a>")
+                                     .arg(configuration()->supportForumUrl().toString());
+
+    const std::string title = muse::trc("project/save", "An error occurred while saving your score");
+
+    const std::string body = muse::qtrc("project/save",
+                                        "To preserve your score, try saving it again. "
+                                        "If this message still appears, please save your score as new copy. "
+                                        "You can also get help for this issue on %1.<br/><br/>"
+                                        "Error details (please cite when asking for support): %2")
+                             .arg(supportForumLink, errDetailsMessage)
+                             .toStdString();
+
+    IInteractive::ButtonDatas buttons;
+
+    IInteractive::ButtonData saveAsBtn(SAVE_AS_BTN_ID, muse::trc("project/save", "Save as…"));
+    saveAsBtn.role = IInteractive::ButtonRole::ContinueRole;
+    buttons.push_back(saveAsBtn);
+
+    IInteractive::ButtonData retryBtn(RETRY_SAVE_BTN_ID, muse::trc("project", "Try again"), true /*accent*/);
+    retryBtn.role = IInteractive::ButtonRole::ContinueRole;
+    buttons.push_back(retryBtn);
+
+    IInteractive::ButtonData cancelBtn = interactive()->buttonData(IInteractive::Button::Cancel);
+    buttons.push_back(cancelBtn);
+
+    return interactive()->error(title, IInteractive::Text(body, IInteractive::TextFormat::RichText),
+                                buttons, retryBtn.btn).button();
+}
+
 void ProjectActionsController::revertCorruptedScoreToLastSaved()
 {
     TRACEFUNC;

src/project/internal/projectactionscontroller.h

@@ -180,6 +180,7 @@ class ProjectActionsController : public IProjectFilesController, public muse::mi
 
     void warnScoreCouldnotBeSaved(const muse::Ret& ret);
     void warnScoreCouldnotBeSaved(const std::string& errorText);
+    int warnScoreHasBecomeCorruptedAfterSave(const muse::Ret& ret);
 
     void revertCorruptedScoreToLastSaved();
 

src/project/projecterrors.h

@@ -34,6 +34,7 @@ enum class Err {
     NoPartsError,
     CorruptionError,
     CorruptionUponOpenningError,
+    CorruptionUponSavingError,
 
     FileOpenError,
     InvalidCloudScoreId,