Merge pull request #8760 from iFreilicht/fix-json-load-assertion-errors

Fix derivation load assertion errors

4 files changed, 76 insertions, 12 deletions

diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md
index df30ce83d..526b64fde 100644
--- a/doc/manual/src/release-notes/rl-next.md
+++ b/doc/manual/src/release-notes/rl-next.md
@@ -8,3 +8,5 @@
   These functions are useful for converting between flake references encoded as attribute sets and URLs.
 
 - [`builtins.toJSON`](@docroot@/language/builtins.md#builtins-parseFlakeRef) now prints [--show-trace](@docroot@/command-ref/conf-file.html#conf-show-trace) items for the path in which it finds an evaluation error.
+
+- Error messages regarding malformed input to [`derivation add`](@docroot@/command-ref/new-cli/nix3-derivation-add.md) are now clearer and more detailed.
diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc
index b831b2fe5..8a84bb1c5 100644
--- a/src/libstore/derivations.cc
+++ b/src/libstore/derivations.cc
@@ -993,6 +993,7 @@ DerivationOutput DerivationOutput::fromJSON(
     const ExperimentalFeatureSettings & xpSettings)
 {
     std::set<std::string_view> keys;
+    ensureType(_json, nlohmann::detail::value_t::object);
     auto json = (std::map<std::string, nlohmann::json>) _json;
 
     for (const auto & [key, _] : json)
@@ -1097,36 +1098,51 @@ Derivation Derivation::fromJSON(
     const Store & store,
     const nlohmann::json & json)
 {
+    using nlohmann::detail::value_t;
+
     Derivation res;
 
-    res.name = json["name"];
+    ensureType(json, value_t::object);
 
-    {
-        auto & outputsObj = json["outputs"];
+    res.name = ensureType(valueAt(json, "name"), value_t::string);
+
+    try {
+        auto & outputsObj = ensureType(valueAt(json, "outputs"), value_t::object);
         for (auto & [outputName, output] : outputsObj.items()) {
             res.outputs.insert_or_assign(
                 outputName,
                 DerivationOutput::fromJSON(store, res.name, outputName, output));
         }
+    } catch (Error & e) {
+        e.addTrace({}, "while reading key 'outputs'");
+        throw;
     }
 
-    {
-        auto & inputsList = json["inputSrcs"];
+    try {
+        auto & inputsList = ensureType(valueAt(json, "inputSrcs"), value_t::array);
         for (auto & input : inputsList)
             res.inputSrcs.insert(store.parseStorePath(static_cast<const std::string &>(input)));
+    } catch (Error & e) {
+        e.addTrace({}, "while reading key 'inputSrcs'");
+        throw;
     }
 
-    {
-        auto & inputDrvsObj = json["inputDrvs"];
-        for (auto & [inputDrvPath, inputOutputs] : inputDrvsObj.items())
+    try {
+        auto & inputDrvsObj = ensureType(valueAt(json, "inputDrvs"), value_t::object);
+        for (auto & [inputDrvPath, inputOutputs] : inputDrvsObj.items()) {
+            ensureType(inputOutputs, value_t::array);
             res.inputDrvs[store.parseStorePath(inputDrvPath)] =
                 static_cast<const StringSet &>(inputOutputs);
+        }
+    } catch (Error & e) {
+        e.addTrace({}, "while reading key 'inputDrvs'");
+        throw;
     }
 
-    res.platform = json["system"];
-    res.builder = json["builder"];
-    res.args = json["args"];
-    res.env = json["env"];
+    res.platform = ensureType(valueAt(json, "system"), value_t::string);
+    res.builder = ensureType(valueAt(json, "builder"), value_t::string);
+    res.args = ensureType(valueAt(json, "args"), value_t::array);
+    res.env = ensureType(valueAt(json, "env"), value_t::object);
 
     return res;
 }
diff --git a/src/libutil/json-utils.cc b/src/libutil/json-utils.cc
index d7220e71d..61cef743d 100644
--- a/src/libutil/json-utils.cc
+++ b/src/libutil/json-utils.cc
@@ -1,4 +1,5 @@
 #include "json-utils.hh"
+#include "error.hh"
 
 namespace nix {
 
@@ -16,4 +17,27 @@ nlohmann::json * get(nlohmann::json & map, const std::string & key)
     return &*i;
 }
 
+const nlohmann::json & valueAt(
+    const nlohmann::json & map,
+    const std::string & key)
+{
+    if (!map.contains(key))
+        throw Error("Expected JSON object to contain key '%s' but it doesn't", key);
+
+    return map[key];
+}
+
+const nlohmann::json & ensureType(
+    const nlohmann::json & value,
+    nlohmann::json::value_type expectedType
+    )
+{
+    if (value.type() != expectedType)
+        throw Error(
+            "Expected JSON value to be of type '%s' but it is of type '%s'",
+            nlohmann::json(expectedType).type_name(),
+            value.type_name());
+
+    return value;
+}
 }
diff --git a/src/libutil/json-utils.hh b/src/libutil/json-utils.hh
index 5e63c1af4..77c63595c 100644
--- a/src/libutil/json-utils.hh
+++ b/src/libutil/json-utils.hh
@@ -11,6 +11,28 @@ const nlohmann::json * get(const nlohmann::json & map, const std::string & key);
 nlohmann::json * get(nlohmann::json & map, const std::string & key);
 
 /**
+ * Get the value of a json object at a key safely, failing
+ * with a Nix Error if the key does not exist.
+ *
+ * Use instead of nlohmann::json::at() to avoid ugly exceptions.
+ *
+ * _Does not check whether `map` is an object_, use `ensureType` for that.
+ */
+const nlohmann::json & valueAt(
+    const nlohmann::json & map,
+    const std::string & key);
+
+/**
+ * Ensure the type of a json object is what you expect, failing
+ * with a Nix Error if it isn't.
+ *
+ * Use before type conversions and element access to avoid ugly exceptions.
+ */
+const nlohmann::json & ensureType(
+    const nlohmann::json & value,
+    nlohmann::json::value_type expectedType);
+
+/**
  * For `adl_serializer<std::optional<T>>` below, we need to track what
  * types are not already using `null`. Only for them can we use `null`
  * to represent `std::nullopt`.