From 572721e20d19e7c7e5d7d4ebad469d8fb6d8c0f8 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Fri, 6 Apr 2018 13:26:16 +0200 Subject: [PATCH 1/4] Catch ContainerFormatError when deserialising containers Only the deserialize() functions themselves may pass the ContainerFormatError on, because their callers will have to handle those errors anyway. Contributes to issue CURA-5045. --- cura/Settings/ContainerManager.py | 12 +++++----- cura/Settings/CuraContainerRegistry.py | 10 ++++---- plugins/3MFReader/ThreeMFWorkspaceReader.py | 23 ++++++++++++++----- .../CuraProfileReader/CuraProfileReader.py | 8 +++++-- .../GCodeProfileReader/GCodeProfileReader.py | 6 ++++- plugins/PostProcessingPlugin/Script.py | 12 ++++++---- 6 files changed, 48 insertions(+), 23 deletions(-) diff --git a/cura/Settings/ContainerManager.py b/cura/Settings/ContainerManager.py index f0eb2303f2..0dc26207df 100644 --- a/cura/Settings/ContainerManager.py +++ b/cura/Settings/ContainerManager.py @@ -1,4 +1,4 @@ -# Copyright (c) 2017 Ultimaker B.V. +# Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. import os.path @@ -22,12 +22,10 @@ from UM.Settings.DefinitionContainer import DefinitionContainer from UM.Settings.InstanceContainer import InstanceContainer from UM.MimeTypeDatabase import MimeTypeNotFoundError +from UM.Settings.ContainerFormatError import ContainerFormatError from UM.Settings.ContainerRegistry import ContainerRegistry - -from UM.i18n import i18nCatalog - from cura.Settings.ExtruderManager import ExtruderManager -from cura.Settings.ExtruderStack import ExtruderStack +from UM.i18n import i18nCatalog catalog = i18nCatalog("cura") @@ -289,7 +287,9 @@ class ContainerManager(QObject): with open(file_url, "rt", encoding = "utf-8") as f: container.deserialize(f.read()) except PermissionError: - return {"status": "error", "message": "Permission denied when trying to read the file"} + return {"status": "error", "message": "Permission denied when trying to read the file."} + except ContainerFormatError: + return {"status": "error", "Message": "The material file appears to be corrupt."} except Exception as ex: return {"status": "error", "message": str(ex)} diff --git a/cura/Settings/CuraContainerRegistry.py b/cura/Settings/CuraContainerRegistry.py index 6600512672..bbd45cdb96 100644 --- a/cura/Settings/CuraContainerRegistry.py +++ b/cura/Settings/CuraContainerRegistry.py @@ -1,4 +1,4 @@ -# Copyright (c) 2017 Ultimaker B.V. +# Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. import os @@ -11,6 +11,7 @@ from typing import Optional from PyQt5.QtWidgets import QMessageBox from UM.Decorators import override +from UM.Settings.ContainerFormatError import ContainerFormatError from UM.Settings.ContainerRegistry import ContainerRegistry from UM.Settings.ContainerStack import ContainerStack from UM.Settings.InstanceContainer import InstanceContainer @@ -25,7 +26,6 @@ from UM.Resources import Resources from . import ExtruderStack from . import GlobalStack -from .ExtruderManager import ExtruderManager from cura.CuraApplication import CuraApplication from cura.Machines.QualityManager import getMachineDefinitionIDForQualitySearch @@ -420,7 +420,6 @@ class CuraContainerRegistry(ContainerRegistry): Logger.log("d", "Converting ContainerStack {stack} to {type}", stack = container.getId(), type = container_type) - new_stack = None if container_type == "extruder_train": new_stack = ExtruderStack.ExtruderStack(container.getId()) else: @@ -706,7 +705,10 @@ class CuraContainerRegistry(ContainerRegistry): instance_container = InstanceContainer(container_id) with open(file_path, "r", encoding = "utf-8") as f: serialized = f.read() - instance_container.deserialize(serialized, file_path) + try: + instance_container.deserialize(serialized, file_path) + except ContainerFormatError: + continue self.addContainer(instance_container) break diff --git a/plugins/3MFReader/ThreeMFWorkspaceReader.py b/plugins/3MFReader/ThreeMFWorkspaceReader.py index 847f71c6aa..7e86cec6a9 100755 --- a/plugins/3MFReader/ThreeMFWorkspaceReader.py +++ b/plugins/3MFReader/ThreeMFWorkspaceReader.py @@ -12,9 +12,11 @@ import xml.etree.ElementTree as ET from UM.Workspace.WorkspaceReader import WorkspaceReader from UM.Application import Application +from UM.ConfigurationErrorMessage import ConfigurationErrorMessage from UM.Logger import Logger from UM.i18n import i18nCatalog from UM.Signal import postponeSignals, CompressTechnique +from UM.Settings.ContainerFormatError import ContainerFormatError from UM.Settings.ContainerStack import ContainerStack from UM.Settings.DefinitionContainer import DefinitionContainer from UM.Settings.InstanceContainer import InstanceContainer @@ -332,7 +334,10 @@ class ThreeMFWorkspaceReader(WorkspaceReader): containers_found_dict["quality_changes"] = True # Check if there really is a conflict by comparing the values instance_container = InstanceContainer(container_id) - instance_container.deserialize(serialized, file_name = instance_container_file_name) + try: + instance_container.deserialize(serialized, file_name = instance_container_file_name) + except ContainerFormatError: + continue if quality_changes[0] != instance_container: quality_changes_conflict = True elif container_type == "quality": @@ -639,8 +644,11 @@ class ThreeMFWorkspaceReader(WorkspaceReader): definitions = self._container_registry.findDefinitionContainersMetadata(id = container_id) if not definitions: definition_container = DefinitionContainer(container_id) - definition_container.deserialize(archive.open(definition_container_file).read().decode("utf-8"), - file_name = definition_container_file) + try: + definition_container.deserialize(archive.open(definition_container_file).read().decode("utf-8"), + file_name = definition_container_file) + except ContainerFormatError: + continue #Skip this definition container file. Pretend there is none. self._container_registry.addContainer(definition_container) Job.yieldThread() @@ -679,8 +687,11 @@ class ThreeMFWorkspaceReader(WorkspaceReader): if to_deserialize_material: material_container = xml_material_profile(container_id) - material_container.deserialize(archive.open(material_container_file).read().decode("utf-8"), - file_name = container_id + "." + self._material_container_suffix) + try: + material_container.deserialize(archive.open(material_container_file).read().decode("utf-8"), + file_name = container_id + "." + self._material_container_suffix) + except ContainerFormatError: + continue #Pretend that this material didn't exist. if need_new_name: new_name = ContainerRegistry.getInstance().uniqueName(material_container.getName()) material_container.setName(new_name) @@ -704,7 +715,7 @@ class ThreeMFWorkspaceReader(WorkspaceReader): # To solve this, we schedule _updateActiveMachine() for later so it will have the latest data. self._updateActiveMachine(global_stack) - # Load all the nodes / meshdata of the workspace + # Load all the nodes / mesh data of the workspace nodes = self._3mf_mesh_reader.read(file_name) if nodes is None: nodes = [] diff --git a/plugins/CuraProfileReader/CuraProfileReader.py b/plugins/CuraProfileReader/CuraProfileReader.py index 69b2187541..8630e885a5 100644 --- a/plugins/CuraProfileReader/CuraProfileReader.py +++ b/plugins/CuraProfileReader/CuraProfileReader.py @@ -1,9 +1,10 @@ -# Copyright (c) 2016 Ultimaker B.V. +# Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. import configparser from UM.PluginRegistry import PluginRegistry from UM.Logger import Logger +from UM.Settings.ContainerFormatError import ContainerFormatError from UM.Settings.InstanceContainer import InstanceContainer # The new profile to make. from cura.ProfileReader import ProfileReader @@ -77,7 +78,10 @@ class CuraProfileReader(ProfileReader): profile.addMetaDataEntry("type", "quality_changes") try: profile.deserialize(serialized) - except Exception as e: # Parsing error. This is not a (valid) Cura profile then. + except ContainerFormatError as e: + Logger.log("e", "Error in the format of a container: %s", str(e)) + return None + except Exception as e: Logger.log("e", "Error while trying to parse profile: %s", str(e)) return None return profile diff --git a/plugins/GCodeProfileReader/GCodeProfileReader.py b/plugins/GCodeProfileReader/GCodeProfileReader.py index d6bda85a48..3ffbb9d910 100644 --- a/plugins/GCodeProfileReader/GCodeProfileReader.py +++ b/plugins/GCodeProfileReader/GCodeProfileReader.py @@ -1,9 +1,10 @@ -# Copyright (c) 2015 Ultimaker B.V. +# Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. import re #Regular expressions for parsing escape characters in the settings. import json +from UM.Settings.ContainerFormatError import ContainerFormatError from UM.Settings.InstanceContainer import InstanceContainer from UM.Logger import Logger from UM.i18n import i18nCatalog @@ -113,6 +114,9 @@ def readQualityProfileFromString(profile_string): profile = InstanceContainer("") try: profile.deserialize(profile_string) + except ContainerFormatError as e: + Logger.log("e", "Corrupt profile in this g-code file: %s", str(e)) + return None except Exception as e: # Not a valid g-code file. Logger.log("e", "Unable to serialise the profile: %s", str(e)) return None diff --git a/plugins/PostProcessingPlugin/Script.py b/plugins/PostProcessingPlugin/Script.py index 7f419cd422..d844705f1c 100644 --- a/plugins/PostProcessingPlugin/Script.py +++ b/plugins/PostProcessingPlugin/Script.py @@ -1,12 +1,12 @@ # Copyright (c) 2015 Jaime van Kessel -# Copyright (c) 2017 Ultimaker B.V. +# Copyright (c) 2018 Ultimaker B.V. # The PostProcessingPlugin is released under the terms of the AGPLv3 or higher. -from UM.Logger import Logger from UM.Signal import Signal, signalemitter from UM.i18n import i18nCatalog # Setting stuff import from UM.Application import Application +from UM.Settings.ContainerFormatError import ContainerFormatError from UM.Settings.ContainerStack import ContainerStack from UM.Settings.InstanceContainer import InstanceContainer from UM.Settings.DefinitionContainer import DefinitionContainer @@ -39,8 +39,12 @@ class Script: self._definition = definitions[0] else: self._definition = DefinitionContainer(setting_data["key"]) - self._definition.deserialize(json.dumps(setting_data)) - ContainerRegistry.getInstance().addContainer(self._definition) + try: + self._definition.deserialize(json.dumps(setting_data)) + ContainerRegistry.getInstance().addContainer(self._definition) + except ContainerFormatError: + self._definition = None + return self._stack.addContainer(self._definition) self._instance = InstanceContainer(container_id="ScriptInstanceContainer") self._instance.setDefinition(self._definition.getId()) From 4615c756a7561b1a484ceebcefb4d47e6e826609 Mon Sep 17 00:00:00 2001 From: Lipu Fei Date: Mon, 9 Apr 2018 14:51:03 +0200 Subject: [PATCH 2/4] More clear error handling for container deserialization CURA-5045 - If a container cannot be deserialized in project loading, it should fail right on the spot because even if it continues, it still won't work. - In other places, at least log deserialization errors if any of them show up. --- cura/Settings/CuraContainerRegistry.py | 1 + plugins/3MFReader/ThreeMFWorkspaceReader.py | 15 +++++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/cura/Settings/CuraContainerRegistry.py b/cura/Settings/CuraContainerRegistry.py index bbd45cdb96..b0ab99ee35 100644 --- a/cura/Settings/CuraContainerRegistry.py +++ b/cura/Settings/CuraContainerRegistry.py @@ -708,6 +708,7 @@ class CuraContainerRegistry(ContainerRegistry): try: instance_container.deserialize(serialized, file_path) except ContainerFormatError: + Logger.logException("e", "Unable to deserialize InstanceContainer %s", file_path) continue self.addContainer(instance_container) break diff --git a/plugins/3MFReader/ThreeMFWorkspaceReader.py b/plugins/3MFReader/ThreeMFWorkspaceReader.py index 7e86cec6a9..d037d8324c 100755 --- a/plugins/3MFReader/ThreeMFWorkspaceReader.py +++ b/plugins/3MFReader/ThreeMFWorkspaceReader.py @@ -12,7 +12,6 @@ import xml.etree.ElementTree as ET from UM.Workspace.WorkspaceReader import WorkspaceReader from UM.Application import Application -from UM.ConfigurationErrorMessage import ConfigurationErrorMessage from UM.Logger import Logger from UM.i18n import i18nCatalog from UM.Signal import postponeSignals, CompressTechnique @@ -337,7 +336,9 @@ class ThreeMFWorkspaceReader(WorkspaceReader): try: instance_container.deserialize(serialized, file_name = instance_container_file_name) except ContainerFormatError: - continue + Logger.logException("e", "Failed to deserialize InstanceContainer %s from project file %s", + instance_container_file_name, file_name) + return ThreeMFWorkspaceReader.PreReadResult.failed if quality_changes[0] != instance_container: quality_changes_conflict = True elif container_type == "quality": @@ -648,7 +649,11 @@ class ThreeMFWorkspaceReader(WorkspaceReader): definition_container.deserialize(archive.open(definition_container_file).read().decode("utf-8"), file_name = definition_container_file) except ContainerFormatError: - continue #Skip this definition container file. Pretend there is none. + # We cannot just skip the definition file because everything else later will just break if the + # machine definition cannot be found. + Logger.logException("e", "Failed to deserialize definition file %s in project file %s", + definition_container_file, file_name) + raise self._container_registry.addContainer(definition_container) Job.yieldThread() @@ -691,7 +696,9 @@ class ThreeMFWorkspaceReader(WorkspaceReader): material_container.deserialize(archive.open(material_container_file).read().decode("utf-8"), file_name = container_id + "." + self._material_container_suffix) except ContainerFormatError: - continue #Pretend that this material didn't exist. + Logger.logException("e", "Failed to deserialize material file %s in project file %s", + material_container_file, file_name) + raise if need_new_name: new_name = ContainerRegistry.getInstance().uniqueName(material_container.getName()) material_container.setName(new_name) From e96c50a582f6d1558b6db194ec39f100963641d5 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Thu, 12 Apr 2018 16:54:21 +0200 Subject: [PATCH 3/4] Fall back to FDMPrinter if definition can't be loaded Some of the settings will be different. But at least it won't crash. Contributes to issue CURA-5045. --- plugins/3MFReader/ThreeMFWorkspaceReader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/3MFReader/ThreeMFWorkspaceReader.py b/plugins/3MFReader/ThreeMFWorkspaceReader.py index d037d8324c..73db48f1a2 100755 --- a/plugins/3MFReader/ThreeMFWorkspaceReader.py +++ b/plugins/3MFReader/ThreeMFWorkspaceReader.py @@ -653,7 +653,7 @@ class ThreeMFWorkspaceReader(WorkspaceReader): # machine definition cannot be found. Logger.logException("e", "Failed to deserialize definition file %s in project file %s", definition_container_file, file_name) - raise + definition_container = self._container_registry.findDefinitionContainers(id = "fdmprinter")[0] #Fall back to defaults. self._container_registry.addContainer(definition_container) Job.yieldThread() From 1769a697926cb1b5ec6489fcc8e349275b8c4e81 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Thu, 12 Apr 2018 16:56:05 +0200 Subject: [PATCH 4/4] Don't crash Cura when material file is wrongly formatted That was the whole point of this change. Instead degrade gracefully. It won't load the material file but give an error message saying that the material is corrupt. Then it won't be able to load the stack as well because the material doesn't exist, and give an error about the stack as well. Contributes to issue CURA-5045. --- plugins/3MFReader/ThreeMFWorkspaceReader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/3MFReader/ThreeMFWorkspaceReader.py b/plugins/3MFReader/ThreeMFWorkspaceReader.py index 73db48f1a2..9b27220cfd 100755 --- a/plugins/3MFReader/ThreeMFWorkspaceReader.py +++ b/plugins/3MFReader/ThreeMFWorkspaceReader.py @@ -698,7 +698,7 @@ class ThreeMFWorkspaceReader(WorkspaceReader): except ContainerFormatError: Logger.logException("e", "Failed to deserialize material file %s in project file %s", material_container_file, file_name) - raise + continue if need_new_name: new_name = ContainerRegistry.getInstance().uniqueName(material_container.getName()) material_container.setName(new_name)