Prevent GUI from crashing when invalid values were written in numeric fields. Includes basic validation. #1709

xs/src/Config.cpp

@@ -33,19 +33,20 @@ ConfigBase::serialize(const t_config_option_key opt_key) {
     return opt->serialize();
 }
 
-void
+bool
 ConfigBase::set_deserialize(const t_config_option_key opt_key, std::string str) {
     if (this->def->count(opt_key) == 0) throw "Calling set_deserialize() on unknown option";
     ConfigOptionDef* optdef = &(*this->def)[opt_key];
     if (!optdef->shortcut.empty()) {
-        for (std::vector<t_config_option_key>::iterator it = optdef->shortcut.begin(); it != optdef->shortcut.end(); ++it)
-            this->set_deserialize(*it, str);
-        return;
+        for (std::vector<t_config_option_key>::iterator it = optdef->shortcut.begin(); it != optdef->shortcut.end(); ++it) {
+            if (!this->set_deserialize(*it, str)) return false;
+        }
+        return true;
     }
     
     ConfigOption* opt = this->option(opt_key, true);
     assert(opt != NULL);
-    opt->deserialize(str);
+    return opt->deserialize(str);
 }
 
 double
@@ -164,31 +165,37 @@ ConfigBase::get_at(t_config_option_key opt_key, size_t i) {
     }
 }
 
-void
+bool
 ConfigBase::set(t_config_option_key opt_key, SV* value) {
     ConfigOption* opt = this->option(opt_key, true);
     if (opt == NULL) CONFESS("Trying to set non-existing option");
     
     if (ConfigOptionFloat* optv = dynamic_cast<ConfigOptionFloat*>(opt)) {
+        if (!looks_like_number(value)) return false;
         optv->value = SvNV(value);
     } else if (ConfigOptionFloats* optv = dynamic_cast<ConfigOptionFloats*>(opt)) {
-        optv->values.clear();
+        std::vector<double> values;
         AV* av = (AV*)SvRV(value);
         const size_t len = av_len(av)+1;
         for (size_t i = 0; i < len; i++) {
             SV** elem = av_fetch(av, i, 0);
-            optv->values.push_back(SvNV(*elem));
+            if (!looks_like_number(*elem)) return false;
+            values.push_back(SvNV(*elem));
         }
+        optv->values = values;
     } else if (ConfigOptionInt* optv = dynamic_cast<ConfigOptionInt*>(opt)) {
+        if (!looks_like_number(value)) return false;
         optv->value = SvIV(value);
     } else if (ConfigOptionInts* optv = dynamic_cast<ConfigOptionInts*>(opt)) {
-        optv->values.clear();
+        std::vector<int> values;
         AV* av = (AV*)SvRV(value);
         const size_t len = av_len(av)+1;
         for (size_t i = 0; i < len; i++) {
             SV** elem = av_fetch(av, i, 0);
-            optv->values.push_back(SvIV(*elem));
+            if (!looks_like_number(*elem)) return false;
+            values.push_back(SvIV(*elem));
         }
+        optv->values = values;
     } else if (ConfigOptionString* optv = dynamic_cast<ConfigOptionString*>(opt)) {
         optv->value = std::string(SvPV_nolen(value), SvCUR(value));
     } else if (ConfigOptionStrings* optv = dynamic_cast<ConfigOptionStrings*>(opt)) {
@@ -200,17 +207,18 @@ ConfigBase::set(t_config_option_key opt_key, SV* value) {
             optv->values.push_back(std::string(SvPV_nolen(*elem), SvCUR(*elem)));
         }
     } else if (ConfigOptionPoint* optv = dynamic_cast<ConfigOptionPoint*>(opt)) {
-        optv->point.from_SV(value);
+        return optv->point.from_SV(value);
     } else if (ConfigOptionPoints* optv = dynamic_cast<ConfigOptionPoints*>(opt)) {
-        optv->values.clear();
+        std::vector<Pointf> values;
         AV* av = (AV*)SvRV(value);
         const size_t len = av_len(av)+1;
         for (size_t i = 0; i < len; i++) {
             SV** elem = av_fetch(av, i, 0);
             Pointf point;
-            point.from_SV(*elem);
-            optv->values.push_back(point);
+            if (!point.from_SV(*elem)) return false;
+            values.push_back(point);
         }
+        optv->values = values;
     } else if (ConfigOptionBool* optv = dynamic_cast<ConfigOptionBool*>(opt)) {
         optv->value = SvTRUE(value);
     } else if (ConfigOptionBools* optv = dynamic_cast<ConfigOptionBools*>(opt)) {
@@ -222,8 +230,9 @@ ConfigBase::set(t_config_option_key opt_key, SV* value) {
             optv->values.push_back(SvTRUE(*elem));
         }
     } else {
-        opt->deserialize( std::string(SvPV_nolen(value)) );
+        if (!opt->deserialize( std::string(SvPV_nolen(value)) )) return false;
     }
+    return true;
 }
 #endif
 
@@ -347,16 +356,17 @@ StaticConfig::option(const t_config_option_key opt_key) const
 }
 
 #ifdef SLIC3RXS
-void
+bool
 StaticConfig::set(t_config_option_key opt_key, SV* value) {
     ConfigOptionDef* optdef = &(*this->def)[opt_key];
     if (!optdef->shortcut.empty()) {
-        for (std::vector<t_config_option_key>::iterator it = optdef->shortcut.begin(); it != optdef->shortcut.end(); ++it)
-            this->set(*it, value);
-        return;
+        for (std::vector<t_config_option_key>::iterator it = optdef->shortcut.begin(); it != optdef->shortcut.end(); ++it) {
+            if (!this->set(*it, value)) return false;
+        }
+        return true;
     }
     
-    static_cast<ConfigBase*>(this)->set(opt_key, value);
+    return static_cast<ConfigBase*>(this)->set(opt_key, value);
 }
 #endif
 

xs/src/Config.hpp

@@ -21,7 +21,7 @@ class ConfigOption {
     public:
     virtual ~ConfigOption() {};
     virtual std::string serialize() const = 0;
-    virtual void deserialize(std::string str) = 0;
+    virtual bool deserialize(std::string str) = 0;
 };
 
 template <class T>
@@ -54,8 +54,9 @@ class ConfigOptionFloat : public ConfigOption
         return ss.str();
     };
     
-    void deserialize(std::string str) {
+    bool deserialize(std::string str) {
         this->value = ::atof(str.c_str());
+        return true;
     };
 };
 
@@ -72,13 +73,14 @@ class ConfigOptionFloats : public ConfigOption, public ConfigOptionVector<double
         return ss.str();
     };
     
-    void deserialize(std::string str) {
+    bool deserialize(std::string str) {
         this->values.clear();
         std::istringstream is(str);
         std::string item_str;
         while (std::getline(is, item_str, ',')) {
             this->values.push_back(::atof(item_str.c_str()));
         }
+        return true;
     };
 };
 
@@ -96,8 +98,9 @@ class ConfigOptionInt : public ConfigOption
         return ss.str();
     };
     
-    void deserialize(std::string str) {
+    bool deserialize(std::string str) {
         this->value = ::atoi(str.c_str());
+        return true;
     };
 };
 
@@ -114,13 +117,14 @@ class ConfigOptionInts : public ConfigOption, public ConfigOptionVector<int>
         return ss.str();
     };
     
-    void deserialize(std::string str) {
+    bool deserialize(std::string str) {
         this->values.clear();
         std::istringstream is(str);
         std::string item_str;
         while (std::getline(is, item_str, ',')) {
             this->values.push_back(::atoi(item_str.c_str()));
         }
+        return true;
     };
 };
 
@@ -145,7 +149,7 @@ class ConfigOptionString : public ConfigOption
         return str; 
     };
     
-    void deserialize(std::string str) {
+    bool deserialize(std::string str) {
         // s/\\n/\n/g
         size_t pos = 0;
         while ((pos = str.find("\\n", pos)) != std::string::npos) {
@@ -154,6 +158,7 @@ class ConfigOptionString : public ConfigOption
         }
         
         this->value = str;
+        return true;
     };
 };
 
@@ -171,13 +176,14 @@ class ConfigOptionStrings : public ConfigOption, public ConfigOptionVector<std::
         return ss.str();
     };
     
-    void deserialize(std::string str) {
+    bool deserialize(std::string str) {
         this->values.clear();
         std::istringstream is(str);
         std::string item_str;
         while (std::getline(is, item_str, ';')) {
             this->values.push_back(item_str);
         }
+        return true;
     };
 };
 
@@ -199,9 +205,10 @@ class ConfigOptionPercent : public ConfigOption
         return s;
     };
     
-    void deserialize(std::string str) {
+    bool deserialize(std::string str) {
         // don't try to parse the trailing % since it's optional
-        sscanf(str.c_str(), "%lf", &this->value);
+        int res = sscanf(str.c_str(), "%lf", &this->value);
+        return res == 1;
     };
 };
 
@@ -228,14 +235,16 @@ class ConfigOptionFloatOrPercent : public ConfigOption
         return s;
     };
     
-    void deserialize(std::string str) {
+    bool deserialize(std::string str) {
         if (str.find_first_of("%") != std::string::npos) {
-            sscanf(str.c_str(), "%lf%%", &this->value);
+            int res = sscanf(str.c_str(), "%lf%%", &this->value);
+            if (res == 0) return false;
             this->percent = true;
         } else {
             this->value = ::atof(str.c_str());
             this->percent = false;
         }
+        return true;
     };
 };
 
@@ -255,8 +264,9 @@ class ConfigOptionPoint : public ConfigOption
         return ss.str();
     };
     
-    void deserialize(std::string str) {
-        sscanf(str.c_str(), "%lf%*1[,x]%lf", &this->point.x, &this->point.y);
+    bool deserialize(std::string str) {
+        int res = sscanf(str.c_str(), "%lf%*1[,x]%lf", &this->point.x, &this->point.y);
+        return res == 2;
     };
 };
 
@@ -275,15 +285,17 @@ class ConfigOptionPoints : public ConfigOption, public ConfigOptionVector<Pointf
         return ss.str();
     };
     
-    void deserialize(std::string str) {
+    bool deserialize(std::string str) {
         this->values.clear();
         std::istringstream is(str);
         std::string point_str;
         while (std::getline(is, point_str, ',')) {
             Pointf point;
-            sscanf(point_str.c_str(), "%lfx%lf", &point.x, &point.y);
+            int res = sscanf(point_str.c_str(), "%lfx%lf", &point.x, &point.y);
+            if (res != 2) return false;
             this->values.push_back(point);
         }
+        return true;
     };
 };
 
@@ -299,8 +311,9 @@ class ConfigOptionBool : public ConfigOption
         return std::string(this->value ? "1" : "0");
     };
     
-    void deserialize(std::string str) {
+    bool deserialize(std::string str) {
         this->value = (str.compare("1") == 0);
+        return true;
     };
 };
 
@@ -317,13 +330,14 @@ class ConfigOptionBools : public ConfigOption, public ConfigOptionVector<bool>
         return ss.str();
     };
     
-    void deserialize(std::string str) {
+    bool deserialize(std::string str) {
         this->values.clear();
         std::istringstream is(str);
         std::string item_str;
         while (std::getline(is, item_str, ',')) {
             this->values.push_back(item_str.compare("1") == 0);
         }
+        return true;
     };
 };
 
@@ -345,10 +359,11 @@ class ConfigOptionEnum : public ConfigOption
         return "";
     };
 
-    void deserialize(std::string str) {
+    bool deserialize(std::string str) {
         t_config_enum_values enum_keys_map = ConfigOptionEnum<T>::get_enum_values();
-        assert(enum_keys_map.count(str) > 0);
+        if (enum_keys_map.count(str) == 0) return false;
         this->value = static_cast<T>(enum_keys_map[str]);
+        return true;
     };
 
     static t_config_enum_values get_enum_values();
@@ -371,9 +386,10 @@ class ConfigOptionEnumGeneric : public ConfigOption
         return "";
     };
 
-    void deserialize(std::string str) {
-        assert(this->keys_map->count(str) != 0);
+    bool deserialize(std::string str) {
+        if (this->keys_map->count(str) == 0) return false;
         this->value = (*this->keys_map)[str];
+        return true;
     };
 };
 
@@ -435,7 +451,7 @@ class ConfigBase
     virtual void keys(t_config_option_keys *keys) const = 0;
     void apply(const ConfigBase &other, bool ignore_nonexistent = false);
     std::string serialize(const t_config_option_key opt_key);
-    void set_deserialize(const t_config_option_key opt_key, std::string str);
+    bool set_deserialize(const t_config_option_key opt_key, std::string str);
     double get_abs_value(const t_config_option_key opt_key);
     double get_abs_value(const t_config_option_key opt_key, double ratio_over);
     
@@ -443,7 +459,7 @@ class ConfigBase
     SV* as_hash();
     SV* get(t_config_option_key opt_key);
     SV* get_at(t_config_option_key opt_key, size_t i);
-    void set(t_config_option_key opt_key, SV* value);
+    bool set(t_config_option_key opt_key, SV* value);
     #endif
 };
 
@@ -477,7 +493,7 @@ class StaticConfig : public ConfigBase
     const ConfigOption* option(const t_config_option_key opt_key) const;
     
     #ifdef SLIC3RXS
-    void set(t_config_option_key opt_key, SV* value);
+    bool set(t_config_option_key opt_key, SV* value);
     #endif
 };
 

xs/src/Point.cpp

@@ -201,12 +201,17 @@ Pointf::to_SV_pureperl() const {
     return newRV_noinc((SV*)av);
 }
 
-void
+bool
 Pointf::from_SV(SV* point_sv)
 {
     AV* point_av = (AV*)SvRV(point_sv);
-    this->x = SvNV(*av_fetch(point_av, 0, 0));
-    this->y = SvNV(*av_fetch(point_av, 1, 0));
+    SV* sv_x = *av_fetch(point_av, 0, 0);
+    SV* sv_y = *av_fetch(point_av, 1, 0);
+    if (!looks_like_number(sv_x) || !looks_like_number(sv_x)) return false;
+    
+    this->x = SvNV(sv_x);
+    this->y = SvNV(sv_y);
+    return true;
 }
 #endif
 

xs/src/Point.hpp

@@ -66,7 +66,7 @@ class Pointf
     void translate(double x, double y);
     
     #ifdef SLIC3RXS
-    void from_SV(SV* point_sv);
+    bool from_SV(SV* point_sv);
     SV* to_SV_pureperl() const;
     #endif
 };

xs/src/PrintConfig.hpp

@@ -228,6 +228,7 @@ class PrintConfigDef
         Options["fan_below_layer_time"].sidetext = "approximate seconds";
         Options["fan_below_layer_time"].cli = "fan-below-layer-time=i";
         Options["fan_below_layer_time"].width = 60;
+        Options["fan_below_layer_time"].min = 0;
         Options["fan_below_layer_time"].max = 1000;
 
         Options["filament_diameter"].type = coFloats;
@@ -235,6 +236,7 @@ class PrintConfigDef
         Options["filament_diameter"].tooltip = "Enter your filament diameter here. Good precision is required, so use a caliper and do multiple measurements along the filament, then compute the average.";
         Options["filament_diameter"].sidetext = "mm";
         Options["filament_diameter"].cli = "filament-diameter=f@";
+        Options["filament_diameter"].min = 0;
 
         Options["fill_angle"].type = coInt;
         Options["fill_angle"].label = "Fill angle";
@@ -242,6 +244,7 @@ class PrintConfigDef
         Options["fill_angle"].tooltip = "Default base angle for infill orientation. Cross-hatching will be applied to this. Bridges will be infilled using the best direction Slic3r can detect, so this setting does not affect them.";
         Options["fill_angle"].sidetext = "°";
         Options["fill_angle"].cli = "fill-angle=i";
+        Options["fill_angle"].min = 0;
         Options["fill_angle"].max = 359;
 
         Options["fill_density"].type = coPercent;