#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);
+
+protected:
+    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,30 @@ 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.code() == (int)Err::CorruptionUponSavingError) {
+                            ret2.setData("savingFileIsCorrupt", true);
+                            return ret2;
+                        }
+                    }
+                    return ret;
+                }
+            }
+
+            // Remove the temp save file (not problematic if fails)
+            fileSystem()->remove(savePath);
         }
     }
 
@@ -779,6 +813,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", "File “%1” is corrupt 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

@@ -930,7 +930,11 @@ bool ProjectActionsController::saveProjectLocally(const muse::io::path_t& filePa
 
     if (!ret) {
         LOGE() << ret.toString();
-        warnScoreCouldnotBeSaved(ret);
+        if (ret.code() == (int)Err::CorruptionUponSavingError) {
+            warnScoreCorruptAfterSave(ret);
+        } else {
+            warnScoreCouldnotBeSaved(ret);
+        }
         return false;
     }
 
@@ -1577,6 +1581,28 @@ void ProjectActionsController::warnScoreCouldnotBeSaved(const std::string& error
     interactive()->warning(muse::trc("project/save", "Your score could not be saved"), errorText);
 }
 
+void ProjectActionsController::warnScoreCorruptAfterSave(const Ret& ret)
+{
+    std::string errMessage = ret.toString();
+
+    std::any savingFileIsCorrupt = ret.data("savingFileIsCorrupt");
+    std::string savingFileIsCorruptMessage = savingFileIsCorrupt.has_value() && std::any_cast<bool>(savingFileIsCorrupt)
+                                             ? ""
+                                             : " " + muse::trc("project/save", "or from the _saving file"); // The Linux build doesn't like trailing and leading whitespace in translations
+
+    std::string message = muse::qtrc("project/save",
+                                     "An error occurred while saving your file and sadly it has become corrupt. "
+                                     "This may be a one-off error. Please retry saving the file in order not to lose your latest changes.\n\n"
+                                     "If it keeps happening, try saving the file to a different location or to MuseScore.com. "
+                                     "If all else fails, restore the file from the most recent backup created by MuseScore Studio%1.\n\n"
+                                     "If you need help, go to the “Support and bug reports” forum at https://musescore.org/en/forum\n\n"
+                                     "Error: %2")
+                          .arg(savingFileIsCorruptMessage.c_str(), errMessage.c_str())
+                          .toStdString();
+
+    interactive()->warning(muse::trc("project/save", "Your score could not be saved"), message);
+}
+
 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);
+    void warnScoreCorruptAfterSave(const muse::Ret& ret);
 
     void revertCorruptedScoreToLastSaved();
 

src/project/projecterrors.h

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