Improved error handling when importing configuration from a G-code.

lib/Slic3r/Config.pm

@@ -100,7 +100,7 @@ sub merge {
 sub load {
     my $class = shift;
     my ($file) = @_;
-    
+
     if ($file =~ /\.gcode$/i || $file =~ /\.g$/i) {
         my $config = $class->new;        
         $config->_load_from_gcode($file);

lib/Slic3r/GUI/MainFrame.pm

@@ -548,12 +548,13 @@ sub load_config_file {
         $file = Slic3r::decode_path($dlg->GetPaths);
         $dlg->Destroy;
     }
+    for my $tab (values %{$self->{options_tabs}}) {
+        # Dont proceed further if the config file cannot be loaded.
+        return undef if ! $tab->load_config_file($file);
+    }
     $Slic3r::GUI::Settings->{recent}{config_directory} = dirname($file);
     wxTheApp->save_settings;
     $last_config = $file;
-    for my $tab (values %{$self->{options_tabs}}) {
-        $tab->load_config_file($file);
-    }
 }
 
 sub export_configbundle {

lib/Slic3r/GUI/Tab.pm

@@ -455,17 +455,21 @@ sub load_config_file {
     my $i = first { $self->{presets}[$_]{file} eq $file && $self->{presets}[$_]{external} } 1..$#{$self->{presets}};
     if (!$i) {
         my $preset_name = basename($file);  # keep the .ini suffix
-        push @{$self->{presets}}, Slic3r::GUI::Tab::Preset->new(
+        my $preset_new = Slic3r::GUI::Tab::Preset->new(
             file        => $file,
             name        => $preset_name,
             external    => 1,
         );
+        # Try to load the config file before it is entered into the list. If the loading fails, an undef is returned.
+        return undef if ! defined $preset_new->config;
+        push @{$self->{presets}}, $preset_new;
         $self->{presets_choice}->Append($preset_name);
         $i = $#{$self->{presets}};
     }
     $self->{presets_choice}->SetSelection($i - $self->{default_suppressed});
     $self->on_select_preset;
     $self->_on_presets_changed;
+    return 1;
 }
 
 sub load_config {
@@ -1710,13 +1714,15 @@ sub on_preset_loaded {
 
 sub load_config_file {
     my $self = shift;
-    $self->SUPER::load_config_file(@_);
-    
-    Slic3r::GUI::warning_catcher($self)->(
-        "Your configuration was imported. However, Slic3r is currently only able to import settings "
-        . "for the first defined filament. We recommend you don't use exported configuration files "
-        . "for multi-extruder setups and rely on the built-in preset management system instead.")
-        if @{ $self->{config}->nozzle_diameter } > 1;
+    if ($self->SUPER::load_config_file(@_)) {
+        Slic3r::GUI::warning_catcher($self)->(
+            "Your configuration was imported. However, Slic3r is currently only able to import settings "
+            . "for the first defined filament. We recommend you don't use exported configuration files "
+            . "for multi-extruder setups and rely on the built-in preset management system instead.")
+            if @{ $self->{config}->nozzle_diameter } > 1;
+        return 1;
+    }
+    return undef;
 }
 
 package Slic3r::GUI::Tab::Page;
@@ -1862,7 +1868,11 @@ sub config {
         
         # apply preset values on top of defaults
         my $config = Slic3r::Config->new_from_defaults(@$keys);
-        my $external_config = Slic3r::Config->load($self->file);
+        my $external_config = eval { Slic3r::Config->load($self->file); };
+        if ($@) {
+            Slic3r::GUI::show_error(undef, $@);
+            return undef;
+        }
         $config->set($_, $external_config->get($_))
             for grep $external_config->has($_), @$keys;
 

xs/src/libslic3r/Config.cpp

@@ -312,7 +312,14 @@ void ConfigBase::load(const std::string &file)
 void ConfigBase::load_from_gcode(const std::string &file)
 {
     // 1) Read a 64k block from the end of the G-code.
-    boost::nowide::ifstream ifs(file);
+	boost::nowide::ifstream ifs(file);
+	{
+		const char slic3r_gcode_header[] = "; generated by Slic3r ";
+		std::string firstline;
+		std::getline(ifs, firstline);
+		if (strncmp(slic3r_gcode_header, firstline.c_str(), strlen(slic3r_gcode_header)) != 0)
+			throw std::exception("Not a Slic3r generated g-code.");
+	}
     ifs.seekg(0, ifs.end);
 	auto file_length = ifs.tellg();
 	auto data_length = std::min<std::fstream::streampos>(65535, file_length);
@@ -325,6 +332,7 @@ void ConfigBase::load_from_gcode(const std::string &file)
     char *data_start = data.data();
     // boost::nowide::ifstream seems to cook the text data somehow, so less then the 64k of characters may be retrieved.
 	char *end = data_start + strlen(data.data());
+    size_t num_key_value_pairs = 0;
     for (;;) {
         // Extract next line.
         for (-- end; end > data_start && (*end == '\r' || *end == '\n'); -- end);
@@ -362,11 +370,17 @@ void ConfigBase::load_from_gcode(const std::string &file)
             break;
         try {
             this->set_deserialize(key, value);
+            ++ num_key_value_pairs;
         } catch (UnknownOptionException & /* e */) {
             // ignore
         }
         end = start;
     }
+    if (num_key_value_pairs < 90) {
+        char msg[80];
+        sprintf(msg, "Suspiciously low number of configuration values extracted: %d", num_key_value_pairs);
+        throw std::exception(msg);
+    }
 }
 
 void ConfigBase::save(const std::string &file) const

xs/xsp/Config.xsp

@@ -39,7 +39,14 @@
     %name{setenv} void setenv_();
     double min_object_distance();
     %name{_load} void load(std::string file);
-    %name{_load_from_gcode} void load_from_gcode(std::string file);
+    %name{_load_from_gcode} void load_from_gcode(std::string input_file)
+        %code%{
+            try {
+                THIS->load_from_gcode(input_file);
+            } catch (std::exception& e) {
+                croak("Error exracting configuration from a g-code %s:\n%s\n", input_file.c_str(), e.what());
+            }
+        %};
     %name{_save} void save(std::string file);
 };