Better error validation for printer profiles
Data types loaded from disk were not properly ensured to match expected types and input validation also had deficits. Should fix #714
This commit is contained in:
parent
f5eef06e88
commit
f238ef40ec
5 changed files with 148 additions and 89 deletions
|
|
@ -98,6 +98,7 @@
|
|||
printer profiles
|
||||
* [#683](https://github.com/foosel/OctoPrint/issues/683) - Fixed heated bed option not being properly displayed in
|
||||
printer profiles
|
||||
* [#714](https://github.com/foosel/OctoPrint/issues/714) - Fixed type validation of printer profiles
|
||||
* Various fixes without tickets:
|
||||
* GCODE viewer now doesn't stumble over completely extrusionless GCODE files
|
||||
* Do not deliver the API key on settings API unless user has admin rights
|
||||
|
|
|
|||
|
|
@ -9,17 +9,28 @@ __copyright__ = "Copyright (C) 2014 The OctoPrint Project - Released under terms
|
|||
import os
|
||||
import copy
|
||||
import re
|
||||
import logging
|
||||
|
||||
from octoprint.settings import settings
|
||||
from octoprint.util import dict_merge, dict_clean
|
||||
from octoprint.util import dict_merge, dict_clean, dict_contains_keys
|
||||
|
||||
class SaveError(Exception):
|
||||
pass
|
||||
|
||||
class CouldNotOverwriteError(SaveError):
|
||||
pass
|
||||
|
||||
class InvalidProfileError(Exception):
|
||||
pass
|
||||
|
||||
class BedTypes(object):
|
||||
RECTANGULAR = "rectangular"
|
||||
CIRCULAR = "circular"
|
||||
|
||||
@classmethod
|
||||
def values(cls):
|
||||
return [getattr(cls, name) for name in cls.__dict__ if not name.startswith("__")]
|
||||
|
||||
class PrinterProfileManager(object):
|
||||
|
||||
default = dict(
|
||||
|
|
@ -51,8 +62,8 @@ class PrinterProfileManager(object):
|
|||
|
||||
def __init__(self):
|
||||
self._current = None
|
||||
|
||||
self._folder = settings().getBaseFolder("printerProfiles")
|
||||
self._logger = logging.getLogger(__name__)
|
||||
|
||||
def select(self, identifier):
|
||||
if identifier is None or not self.exists(identifier):
|
||||
|
|
@ -69,11 +80,14 @@ class PrinterProfileManager(object):
|
|||
return self._load_all()
|
||||
|
||||
def get(self, identifier):
|
||||
if identifier == "_default":
|
||||
return self._load_default()
|
||||
elif self.exists(identifier):
|
||||
return self._load_from_path(self._get_profile_path(identifier))
|
||||
else:
|
||||
try:
|
||||
if identifier == "_default":
|
||||
return self._load_default()
|
||||
elif self.exists(identifier):
|
||||
return self._load_from_path(self._get_profile_path(identifier))
|
||||
else:
|
||||
return None
|
||||
except InvalidProfileError:
|
||||
return None
|
||||
|
||||
def remove(self, identifier):
|
||||
|
|
@ -87,7 +101,7 @@ class PrinterProfileManager(object):
|
|||
elif "name" in profile:
|
||||
identifier = profile["name"]
|
||||
else:
|
||||
raise ValueError("profile must contain either id or name")
|
||||
raise InvalidProfileError("profile must contain either id or name")
|
||||
|
||||
identifier = self._sanitize(identifier)
|
||||
profile["id"] = identifier
|
||||
|
|
@ -95,6 +109,9 @@ class PrinterProfileManager(object):
|
|||
|
||||
if identifier == "_default":
|
||||
default_profile = dict_merge(self._load_default(), profile)
|
||||
if not self._ensure_valid_profile(default_profile):
|
||||
raise InvalidProfileError()
|
||||
|
||||
settings().set(["printerProfiles", "defaultProfile"], default_profile, defaults=dict(printerProfiles=dict(defaultProfile=self.__class__.default)))
|
||||
settings().save()
|
||||
else:
|
||||
|
|
@ -144,10 +161,13 @@ class PrinterProfileManager(object):
|
|||
all_identifiers = self._load_all_identifiers()
|
||||
results = dict()
|
||||
for identifier, path in all_identifiers.items():
|
||||
if identifier == "_default":
|
||||
profile = self._load_default()
|
||||
else:
|
||||
profile = self._load_from_path(path)
|
||||
try:
|
||||
if identifier == "_default":
|
||||
profile = self._load_default()
|
||||
else:
|
||||
profile = self._load_from_path(path)
|
||||
except InvalidProfileError:
|
||||
continue
|
||||
|
||||
if profile is None:
|
||||
continue
|
||||
|
|
@ -176,9 +196,17 @@ class PrinterProfileManager(object):
|
|||
import yaml
|
||||
with open(path) as f:
|
||||
profile = yaml.safe_load(f)
|
||||
profile = self._ensure_valid_profile(profile)
|
||||
if not profile:
|
||||
self._logger.warn("Invalid profile: %s" % path)
|
||||
raise InvalidProfileError()
|
||||
return profile
|
||||
|
||||
def _save_to_path(self, path, profile, allow_overwrite=False):
|
||||
validated_profile = self._ensure_valid_profile(profile)
|
||||
if not validated_profile:
|
||||
raise InvalidProfileError()
|
||||
|
||||
if os.path.exists(path) and not allow_overwrite:
|
||||
raise SaveError("Profile %s already exists and not allowed to overwrite" % profile["id"])
|
||||
|
||||
|
|
@ -197,8 +225,12 @@ class PrinterProfileManager(object):
|
|||
return False
|
||||
|
||||
def _load_default(self):
|
||||
default_profile = settings().get(["printerProfiles", "defaultProfile"])
|
||||
return dict_merge(copy.deepcopy(self.__class__.default), default_profile)
|
||||
default_overrides = settings().get(["printerProfiles", "defaultProfile"])
|
||||
profile = self._ensure_valid_profile(dict_merge(copy.deepcopy(self.__class__.default), default_overrides))
|
||||
if not profile:
|
||||
self._logger.warn("Invalid default profile after applying overrides")
|
||||
raise InvalidProfileError()
|
||||
return profile
|
||||
|
||||
def _get_profile_path(self, identifier):
|
||||
return os.path.join(self._folder, "%s.profile" % identifier)
|
||||
|
|
@ -216,3 +248,60 @@ class PrinterProfileManager(object):
|
|||
sanitized_name = sanitized_name.replace(" ", "_")
|
||||
return sanitized_name
|
||||
|
||||
def _ensure_valid_profile(self, profile):
|
||||
# ensure all keys are present
|
||||
if not dict_contains_keys(self.default, profile):
|
||||
return False
|
||||
|
||||
# conversion helper
|
||||
def convert_value(profile, path, converter):
|
||||
value = profile
|
||||
for part in path[:-1]:
|
||||
if not isinstance(value, dict) or not part in value:
|
||||
raise RuntimeError("%s is not contained in profile" % ".".join(path))
|
||||
value = value[part]
|
||||
|
||||
if not isinstance(value, dict) or not path[-1] in value:
|
||||
raise RuntimeError("%s is not contained in profile" % ".".join(path))
|
||||
|
||||
value[path[-1]] = converter(value[path[-1]])
|
||||
|
||||
# convert ints
|
||||
for path in (("extruder", "count"), ("axes", "x", "speed"), ("axes", "y", "speed"), ("axes", "z", "speed")):
|
||||
try:
|
||||
convert_value(profile, path, int)
|
||||
except:
|
||||
return False
|
||||
|
||||
# convert floats
|
||||
for path in (("volume", "width"), ("volume", "depth"), ("volume", "height"), ("extruder", "nozzleDiameter")):
|
||||
try:
|
||||
convert_value(profile, path, float)
|
||||
except:
|
||||
return False
|
||||
|
||||
# convert booleans
|
||||
for path in (("axes", "x", "inverted"), ("axes", "y", "inverted"), ("axes", "z", "inverted")):
|
||||
try:
|
||||
convert_value(profile, path, bool)
|
||||
except:
|
||||
return False
|
||||
|
||||
# validate form factor
|
||||
if not profile["volume"]["formFactor"] in BedTypes.values():
|
||||
return False
|
||||
|
||||
# validate offsets
|
||||
offsets = []
|
||||
for offset in profile["extruder"]["offsets"]:
|
||||
if not len(offset) == 2:
|
||||
return False
|
||||
x_offset, y_offset = offset
|
||||
try:
|
||||
offsets.append((float(x_offset), float(y_offset)))
|
||||
except:
|
||||
return False
|
||||
profile["extruder"]["offsets"] = offsets
|
||||
|
||||
return profile
|
||||
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ from octoprint.server.util.flask import restricted_access
|
|||
from octoprint.util import dict_merge
|
||||
|
||||
from octoprint.server import printerProfileManager
|
||||
from octoprint.printer.profile import InvalidProfileError, CouldNotOverwriteError, SaveError
|
||||
|
||||
|
||||
@api.route("/printerprofiles", methods=["GET"])
|
||||
|
|
@ -42,18 +43,26 @@ def printerProfilesAdd():
|
|||
del base_profile["id"]
|
||||
if "name" in base_profile:
|
||||
del base_profile["name"]
|
||||
profile = dict_merge(base_profile, json_data["profile"])
|
||||
if not _validate_profile(profile):
|
||||
return None, None, make_response("Profile is invalid, missing obligatory values", 400)
|
||||
|
||||
return _overwrite_profile(profile)
|
||||
profile = dict_merge(base_profile, json_data["profile"])
|
||||
try:
|
||||
saved_profile = printerProfileManager.save(profile, allow_overwrite=False)
|
||||
except InvalidProfileError:
|
||||
return make_response("Profile is invalid", 400)
|
||||
except CouldNotOverwriteError:
|
||||
return make_response("Profile already exists and overwriting was not allowed", 400)
|
||||
except Exception as e:
|
||||
return make_response("Could not save profile: %s" % e.message, 500)
|
||||
else:
|
||||
return jsonify(dict(profile=_convert_profile(saved_profile)))
|
||||
|
||||
@api.route("/printerprofiles/<string:identifier>", methods=["GET"])
|
||||
def printerProfilesGet(identifier):
|
||||
profile = printerProfileManager.get(identifier)
|
||||
if profile is None:
|
||||
make_response("Unknown profile: %s" % identifier, 404)
|
||||
return jsonify(_convert_profile(profile))
|
||||
return make_response("Unknown profile: %s" % identifier, 404)
|
||||
else:
|
||||
return jsonify(_convert_profile(profile))
|
||||
|
||||
@api.route("/printerprofiles/<string:identifier>", methods=["DELETE"])
|
||||
@restricted_access
|
||||
|
|
@ -84,15 +93,17 @@ def printerProfilesUpdate(identifier):
|
|||
del new_profile["default"]
|
||||
|
||||
new_profile["id"] = identifier
|
||||
if not _validate_profile(new_profile):
|
||||
make_response("Combined profile is invalid, missing obligatory values", 400)
|
||||
|
||||
try:
|
||||
saved_profile = printerProfileManager.save(new_profile, allow_overwrite=True, make_default=make_default)
|
||||
except InvalidProfileError:
|
||||
return make_response("Profile is invalid", 400)
|
||||
except CouldNotOverwriteError:
|
||||
return make_response("Profile already exists and overwriting was not allowed", 400)
|
||||
except Exception as e:
|
||||
make_response("Could not save profile: %s" % e.message)
|
||||
|
||||
return jsonify(dict(profile=_convert_profile(saved_profile)))
|
||||
return make_response("Could not save profile: %s" % e.message, 500)
|
||||
else:
|
||||
return jsonify(dict(profile=_convert_profile(saved_profile)))
|
||||
|
||||
def _convert_profiles(profiles):
|
||||
result = dict()
|
||||
|
|
@ -109,59 +120,3 @@ def _convert_profile(profile):
|
|||
converted["default"] = (profile["id"] == default)
|
||||
converted["current"] = (profile["id"] == current)
|
||||
return converted
|
||||
|
||||
def _validate_profile(profile):
|
||||
if not "name" in profile \
|
||||
and "volume" in profile \
|
||||
and "width" in profile["volume"] \
|
||||
and "depth" in profile["volume"] \
|
||||
and "height" in profile["volume"] \
|
||||
and "formFactor" in profile["volume"] \
|
||||
and "heatedBed" in profile \
|
||||
and "extruder" in profile \
|
||||
and "count" in profile["extruder"] \
|
||||
and "offsets" in profile["extruder"] \
|
||||
and len(profile["extruder"]["offsets"]) == profile["extruder"]["count"]:
|
||||
return False
|
||||
|
||||
for dimension in ("width", "depth", "height"):
|
||||
try:
|
||||
profile["volume"][dimension] = float(profile["volume"][dimension])
|
||||
except:
|
||||
return False
|
||||
|
||||
if not profile["volume"]["formFactor"] in ("rectangular", "circular"):
|
||||
return False
|
||||
|
||||
try:
|
||||
profile["heatedBed"] = bool(profile["heatedBed"])
|
||||
except:
|
||||
return False
|
||||
|
||||
try:
|
||||
profile["extruder"]["count"] = int(profile["extruder"]["count"])
|
||||
except:
|
||||
return False
|
||||
|
||||
converted_offsets = []
|
||||
for offset in profile["extruder"]["offsets"]:
|
||||
try:
|
||||
converted_offsets.append((float(offset[0]), float(offset[1])))
|
||||
except:
|
||||
return False
|
||||
profile["extruder"]["offsets"] = converted_offsets
|
||||
|
||||
return True
|
||||
|
||||
def _overwrite_profile(profile):
|
||||
if not "id" in profile and not "name" in profile:
|
||||
return None, None, make_response("Profile must contain either id or name")
|
||||
elif not "name" in profile:
|
||||
return None, None, make_response("Profile must contain a name")
|
||||
|
||||
try:
|
||||
saved_profile = printerProfileManager.save(profile, allow_overwrite=False)
|
||||
except Exception as e:
|
||||
return None, None, make_response("Could not save profile: %s" % e.message)
|
||||
|
||||
return jsonify(dict(profile=_convert_profile(saved_profile)))
|
||||
|
|
|
|||
|
|
@ -268,30 +268,30 @@ function PrinterProfilesViewModel() {
|
|||
color: self.editorColor(),
|
||||
model: self.editorModel(),
|
||||
volume: {
|
||||
width: self.editorVolumeWidth(),
|
||||
depth: self.editorVolumeDepth(),
|
||||
height: self.editorVolumeHeight(),
|
||||
width: parseFloat(self.editorVolumeWidth()),
|
||||
depth: parseFloat(self.editorVolumeDepth()),
|
||||
height: parseFloat(self.editorVolumeHeight()),
|
||||
formFactor: self.editorVolumeFormFactor()
|
||||
},
|
||||
heatedBed: self.editorHeatedBed(),
|
||||
extruder: {
|
||||
count: self.editorExtruders(),
|
||||
count: parseInt(self.editorExtruders()),
|
||||
offsets: [
|
||||
[0.0, 0.0]
|
||||
],
|
||||
nozzleDiameter: self.editorNozzleDiameter()
|
||||
nozzleDiameter: parseFloat(self.editorNozzleDiameter())
|
||||
},
|
||||
axes: {
|
||||
x: {
|
||||
speed: self.editorAxisXSpeed(),
|
||||
speed: parseInt(self.editorAxisXSpeed()),
|
||||
inverted: self.editorAxisXInverted()
|
||||
},
|
||||
y: {
|
||||
speed: self.editorAxisYSpeed(),
|
||||
speed: parseInt(self.editorAxisYSpeed()),
|
||||
inverted: self.editorAxisYInverted()
|
||||
},
|
||||
z: {
|
||||
speed: self.editorAxisZSpeed(),
|
||||
speed: parseInt(self.editorAxisZSpeed()),
|
||||
inverted: self.editorAxisZInverted()
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -269,6 +269,20 @@ def dict_clean(a, b):
|
|||
return result
|
||||
|
||||
|
||||
def dict_contains_keys(a, b):
|
||||
if not isinstance(a, dict) or not isinstance(b, dict):
|
||||
return False
|
||||
|
||||
for k, v in a.iteritems():
|
||||
if not k in b:
|
||||
return False
|
||||
elif isinstance(v, dict):
|
||||
if not dict_contains_keys(v, b[k]):
|
||||
return False
|
||||
|
||||
return True
|
||||
|
||||
|
||||
class Object(object):
|
||||
pass
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue