From b79b3c4efbae4e0c9ced578dd9f7058a773f488b Mon Sep 17 00:00:00 2001
From: Thorsten Hater <24411438+thorstenhater@users.noreply.github.com>
Date: Thu, 14 Apr 2022 09:00:20 +0200
Subject: [PATCH] Guard against 'malformed' ACC

Throw errors on unrecognised ACC mechanism formatting.
---
 data/lukasgd-segv-apr22.acc | 35 +++++++++++++++++++++++++++++++++++
 src/gui_state.cpp           | 18 ++++++++++++------
 src/mechanism.cpp           |  8 ++++++--
 3 files changed, 53 insertions(+), 8 deletions(-)
 create mode 100644 data/lukasgd-segv-apr22.acc

diff --git a/data/lukasgd-segv-apr22.acc b/data/lukasgd-segv-apr22.acc
new file mode 100644
index 0000000..86c64a9
--- /dev/null
+++ b/data/lukasgd-segv-apr22.acc
@@ -0,0 +1,35 @@
+(arbor-component
+  (meta-data
+    (version "0.1-dev"))
+  (cable-cell
+    (morphology
+      (branch 0 -1
+        (segment 0
+          (point -5.000000 0.000000 0.000000 5.000000)
+          (point 0.000000 0.000000 0.000000 5.000000)
+          1)
+        (segment 1
+          (point 0.000000 0.000000 0.000000 5.000000)
+          (point 5.000000 0.000000 0.000000 5.000000)
+          1)))
+    (label-dict
+      (region-def "all"
+        (all))
+      (region-def "apic"
+        (tag 4))
+      (region-def "axon"
+        (tag 2))
+      (region-def "dend"
+        (tag 3))
+      (region-def "soma"
+        (tag 1)))
+    (decor
+      (paint
+        (region "soma")
+        (membrane-capacitance 1.000000))
+      (paint
+        (region "soma")
+        (density
+          (mechanism "hh"
+            ("gkbar" 0.027125)
+            ("gnabar" 0.102993)))))))
diff --git a/src/gui_state.cpp b/src/gui_state.cpp
index baf60d7..1b63096 100644
--- a/src/gui_state.cpp
+++ b/src/gui_state.cpp
@@ -109,6 +109,9 @@ namespace {
       } catch (const arb::arbor_exception& e) {
         log_debug("Failed to load ACC: {}", e.what());
         loader_error = e.what();
+      } catch (const std::runtime_error& e) {
+        log_debug("Failed to load ACC: {}", e.what());
+        loader_error = e.what();
       }
 
       if (!loader_error.empty()) {
@@ -1105,16 +1108,19 @@ void gui_state::deserialize(const std::filesystem::path& fn) {
       state->ion_par_defs[{region, *ion}].Er = t.value;
     }
     void operator()(const arb::density& d) {
-      const auto& t = d.mech;
+      auto& t    = d.mech;
       auto id    = state->mechanisms.add(region);
       auto& data = state->mechanisms[id];
-      auto cat   = t.name();
       auto name  = t.name();
-      { auto sep = std::find(name.begin(), name.end(), ':'); name.erase(name.begin(), sep + 2); }
-      { auto sep = std::find(cat.begin(),  cat.end(),  ':'); cat.erase(sep, cat.end()); }
-      log_debug("Loading mech {} from cat {} ({})", name, cat, t.name());
+      auto pos   = name.find("::");
+      if (pos == std::string::npos
+       || pos == 0
+       || pos + 2 >= name.size()) log_error(fmt::format("Mechanism specifier '{}' is not in format 'cat::name'.", name));
+      auto cat   = name.substr(0, pos);
+      auto mech  = name.substr(pos+2);
+      log_debug("Loading mech {} from cat {} ({})", mech, cat, name);
       // TODO What about derived mechs?
-      make_mechanism(data, cat, name, t.values());
+      make_mechanism(data, cat, mech, t.values());
     }
   };
 
diff --git a/src/mechanism.cpp b/src/mechanism.cpp
index 51eeb25..361f57a 100644
--- a/src/mechanism.cpp
+++ b/src/mechanism.cpp
@@ -2,6 +2,7 @@
 
 #include <fmt/format.h>
 
+#include "utils.hpp"
 #include "gui.hpp"
 
 std::unordered_map<std::string, arb::mechanism_catalogue> catalogues = default_catalogues;
@@ -9,10 +10,13 @@ std::unordered_map<std::string, arb::mechanism_catalogue> catalogues = default_c
 void make_mechanism(mechanism_def& data,
                     const std::string& cat_name, const std::string& name,
                     const std::unordered_map<std::string, double>& values) {
+    if (name.empty()) log_error("Empty mechanism name. Selector must be 'cat::mech'.");
     data.name = name;
     data.cat  = cat_name;
-    auto cat = catalogues.at(cat_name);
-    auto info = cat[name];
+    if (!catalogues.contains(data.cat)) log_error(fmt::format("Unknown catalogue: {}", data.cat));
+    auto cat = catalogues[data.cat];
+    if (!cat.has(data.name)) log_error(fmt::format("Unknown mechanism {} in catalogue {}", data.name, data.cat));
+    auto info = cat[data.name];
     data.globals.clear();
     data.parameters.clear();
     data.states.clear();