Improve checked json casting (#10087)

This introduces new utility functions to get elements from JSON — in an ergonomic way and with nice error messages if the expected type does not match.

Co-authored-by: John Ericson <[email protected]>

Author: haenoe

.gitignore

@@ -49,6 +49,9 @@ perl/Makefile.config
 /src/libexpr/tests
 /tests/unit/libexpr/libnixexpr-tests
 
+# /src/libfetchers
+/tests/unit/libfetchers/libnixfetchers-tests
+
 # /src/libstore/
 *.gen.*
 /src/libstore/tests

Makefile

@@ -34,6 +34,7 @@ makefiles += \
   tests/unit/libutil-support/local.mk \
   tests/unit/libstore/local.mk \
   tests/unit/libstore-support/local.mk \
+  tests/unit/libfetchers/local.mk \
   tests/unit/libexpr/local.mk \
   tests/unit/libexpr-support/local.mk
 endif

src/libfetchers/unix/git.cc

@@ -147,9 +147,12 @@ std::vector<PublicKey> getPublicKeys(const Attrs & attrs)
 {
     std::vector<PublicKey> publicKeys;
     if (attrs.contains("publicKeys")) {
-        nlohmann::json publicKeysJson = nlohmann::json::parse(getStrAttr(attrs, "publicKeys"));
-        ensureType(publicKeysJson, nlohmann::json::value_t::array);
-        publicKeys = publicKeysJson.get<std::vector<PublicKey>>();
+        auto pubKeysJson = nlohmann::json::parse(getStrAttr(attrs, "publicKeys"));
+        auto & pubKeys = getArray(pubKeysJson);
+
+        for (auto & key : pubKeys) {
+            publicKeys.push_back(key);
+        }
     }
     if (attrs.contains("publicKey"))
         publicKeys.push_back(PublicKey{maybeGetStrAttr(attrs, "keytype").value_or("ssh-ed25519"),getStrAttr(attrs, "publicKey")});

src/libstore/derivations.cc

@@ -1239,16 +1239,14 @@ 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;
+    auto & json = getObject(_json);
 
     for (const auto & [key, _] : json)
         keys.insert(key);
 
     auto methodAlgo = [&]() -> std::pair<ContentAddressMethod, HashAlgorithm> {
-        std::string hashAlgoStr = json["hashAlgo"];
-        // remaining to parse, will be mutated by parsers
-        std::string_view s = hashAlgoStr;
+        auto & str = getString(valueAt(json, "hashAlgo"));
+        std::string_view s = str;
         ContentAddressMethod method = ContentAddressMethod::parsePrefix(s);
         if (method == TextIngestionMethod {})
             xpSettings.require(Xp::DynamicDerivations);
@@ -1258,7 +1256,7 @@ DerivationOutput DerivationOutput::fromJSON(
 
     if (keys == (std::set<std::string_view> { "path" })) {
         return DerivationOutput::InputAddressed {
-            .path = store.parseStorePath((std::string) json["path"]),
+            .path = store.parseStorePath(getString(valueAt(json, "path"))),
         };
     }
 
@@ -1267,10 +1265,10 @@ DerivationOutput DerivationOutput::fromJSON(
         auto dof = DerivationOutput::CAFixed {
             .ca = ContentAddress {
                 .method = std::move(method),
-                .hash = Hash::parseNonSRIUnprefixed((std::string) json["hash"], hashAlgo),
+                .hash = Hash::parseNonSRIUnprefixed(getString(valueAt(json, "hash")), hashAlgo),
             },
         };
-        if (dof.path(store, drvName, outputName) != store.parseStorePath((std::string) json["path"]))
+        if (dof.path(store, drvName, outputName) != store.parseStorePath(getString(valueAt(json, "path"))))
             throw Error("Path doesn't match derivation output");
         return dof;
     }
@@ -1357,20 +1355,19 @@ nlohmann::json Derivation::toJSON(const StoreDirConfig & store) const
 
 Derivation Derivation::fromJSON(
     const StoreDirConfig & store,
-    const nlohmann::json & json,
+    const nlohmann::json & _json,
     const ExperimentalFeatureSettings & xpSettings)
 {
     using nlohmann::detail::value_t;
 
     Derivation res;
 
-    ensureType(json, value_t::object);
+    auto & json = getObject(_json);
 
-    res.name = ensureType(valueAt(json, "name"), value_t::string);
+    res.name = getString(valueAt(json, "name"));
 
     try {
-        auto & outputsObj = ensureType(valueAt(json, "outputs"), value_t::object);
-        for (auto & [outputName, output] : outputsObj.items()) {
+        for (auto & [outputName, output] : getObject(valueAt(json, "outputs"))) {
             res.outputs.insert_or_assign(
                 outputName,
                 DerivationOutput::fromJSON(store, res.name, outputName, output));
@@ -1381,8 +1378,7 @@ Derivation Derivation::fromJSON(
     }
 
     try {
-        auto & inputsList = ensureType(valueAt(json, "inputSrcs"), value_t::array);
-        for (auto & input : inputsList)
+        for (auto & input : getArray(valueAt(json, "inputSrcs")))
             res.inputSrcs.insert(store.parseStorePath(static_cast<const std::string &>(input)));
     } catch (Error & e) {
         e.addTrace({}, "while reading key 'inputSrcs'");
@@ -1391,29 +1387,28 @@ Derivation Derivation::fromJSON(
 
     try {
         std::function<DerivedPathMap<StringSet>::ChildNode(const nlohmann::json &)> doInput;
-        doInput = [&](const auto & json) {
+        doInput = [&](const auto & _json) {
+            auto & json = getObject(_json);
             DerivedPathMap<StringSet>::ChildNode node;
-            node.value = static_cast<const StringSet &>(
-                ensureType(valueAt(json, "outputs"), value_t::array));
-            for (auto & [outputId, childNode] : ensureType(valueAt(json, "dynamicOutputs"), value_t::object).items()) {
+            node.value = getStringSet(valueAt(json, "outputs"));
+            for (auto & [outputId, childNode] : getObject(valueAt(json, "dynamicOutputs"))) {
                 xpSettings.require(Xp::DynamicDerivations);
                 node.childMap[outputId] = doInput(childNode);
             }
             return node;
         };
-        auto & inputDrvsObj = ensureType(valueAt(json, "inputDrvs"), value_t::object);
-        for (auto & [inputDrvPath, inputOutputs] : inputDrvsObj.items())
+        for (auto & [inputDrvPath, inputOutputs] : getObject(valueAt(json, "inputDrvs")))
             res.inputDrvs.map[store.parseStorePath(inputDrvPath)] =
                 doInput(inputOutputs);
     } catch (Error & e) {
         e.addTrace({}, "while reading key 'inputDrvs'");
         throw;
     }
 
-    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);
+    res.platform = getString(valueAt(json, "system"));
+    res.builder = getString(valueAt(json, "builder"));
+    res.args = getStringList(valueAt(json, "args"));
+    res.env = getStringMap(valueAt(json, "env"));
 
     return res;
 }

src/libstore/nar-info.cc

@@ -172,19 +172,18 @@ NarInfo NarInfo::fromJSON(
     };
 
     if (json.contains("url"))
-        res.url = ensureType(valueAt(json, "url"), value_t::string);
+        res.url = getString(valueAt(json, "url"));
 
     if (json.contains("compression"))
-        res.compression = ensureType(valueAt(json, "compression"), value_t::string);
+        res.compression = getString(valueAt(json, "compression"));
 
     if (json.contains("downloadHash"))
         res.fileHash = Hash::parseAny(
-            static_cast<const std::string &>(
-                ensureType(valueAt(json, "downloadHash"), value_t::string)),
+            getString(valueAt(json, "downloadHash")),
             std::nullopt);
 
     if (json.contains("downloadSize"))
-        res.fileSize = ensureType(valueAt(json, "downloadSize"), value_t::number_integer);
+        res.fileSize = getInteger(valueAt(json, "downloadSize"));
 
     return res;
 }

src/libstore/path-info.cc

@@ -190,23 +190,18 @@ nlohmann::json UnkeyedValidPathInfo::toJSON(
 
 UnkeyedValidPathInfo UnkeyedValidPathInfo::fromJSON(
     const Store & store,
-    const nlohmann::json & json)
+    const nlohmann::json & _json)
 {
-    using nlohmann::detail::value_t;
-
     UnkeyedValidPathInfo res {
         Hash(Hash::dummy),
     };
 
-    ensureType(json, value_t::object);
-    res.narHash = Hash::parseAny(
-        static_cast<const std::string &>(
-            ensureType(valueAt(json, "narHash"), value_t::string)),
-        std::nullopt);
-    res.narSize = ensureType(valueAt(json, "narSize"), value_t::number_integer);
+    auto & json = getObject(_json);
+    res.narHash = Hash::parseAny(getString(valueAt(json, "narHash")), std::nullopt);
+    res.narSize = getInteger(valueAt(json, "narSize"));
 
     try {
-        auto & references = ensureType(valueAt(json, "references"), value_t::array);
+        auto references = getStringList(valueAt(json, "references"));
         for (auto & input : references)
             res.references.insert(store.parseStorePath(static_cast<const std::string &>
 (input)));
@@ -216,20 +211,16 @@ UnkeyedValidPathInfo UnkeyedValidPathInfo::fromJSON(
     }
 
     if (json.contains("ca"))
-        res.ca = ContentAddress::parse(
-            static_cast<const std::string &>(
-                ensureType(valueAt(json, "ca"), value_t::string)));
+        res.ca = ContentAddress::parse(getString(valueAt(json, "ca")));
 
     if (json.contains("deriver"))
-        res.deriver = store.parseStorePath(
-            static_cast<const std::string &>(
-                ensureType(valueAt(json, "deriver"), value_t::string)));
+        res.deriver = store.parseStorePath(getString(valueAt(json, "deriver")));
 
     if (json.contains("registrationTime"))
-        res.registrationTime = ensureType(valueAt(json, "registrationTime"), value_t::number_integer);
+        res.registrationTime = getInteger(valueAt(json, "registrationTime"));
 
     if (json.contains("ultimate"))
-        res.ultimate = ensureType(valueAt(json, "ultimate"), value_t::boolean);
+        res.ultimate = getBoolean(valueAt(json, "ultimate"));
 
     if (json.contains("signatures"))
         res.sigs = valueAt(json, "signatures");

src/libutil/json-utils.cc

@@ -1,5 +1,8 @@
 #include "json-utils.hh"
 #include "error.hh"
+#include "types.hh"
+#include <nlohmann/json_fwd.hpp>
+#include <iostream>
 
 namespace nix {
 
@@ -18,26 +21,115 @@ nlohmann::json * get(nlohmann::json & map, const std::string & key)
 }
 
 const nlohmann::json & valueAt(
-    const nlohmann::json & map,
+    const nlohmann::json::object_t & map,
     const std::string & key)
 {
     if (!map.contains(key))
-        throw Error("Expected JSON object to contain key '%s' but it doesn't", key);
+        throw Error("Expected JSON object to contain key '%s' but it doesn't: %s", key, nlohmann::json(map).dump());
 
-    return map[key];
+    return map.at(key);
 }
 
-const nlohmann::json & ensureType(
+std::optional<nlohmann::json> optionalValueAt(const nlohmann::json & value, const std::string & key)
+{
+    try {
+        auto & v = valueAt(value, key);
+        return v.get<nlohmann::json>();
+    } catch (...) {
+        return std::nullopt;
+    }
+}
+
+
+std::optional<nlohmann::json> getNullable(const nlohmann::json & value)
+{
+    if (value.is_null())
+        return std::nullopt;
+
+    return value.get<nlohmann::json>();
+}
+
+/**
+ * Ensure the type of a JSON object is what you expect, failing with a
+ * ensure type if it isn't.
+ *
+ * Use before type conversions and element access to avoid ugly
+ * exceptions, but only part of this module to define the other `get*`
+ * functions. It is too cumbersome and easy to forget to expect regular
+ * JSON code to use it directly.
+ */
+static 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'",
+            "Expected JSON value to be of type '%s' but it is of type '%s': %s",
             nlohmann::json(expectedType).type_name(),
-            value.type_name());
+            value.type_name(), value.dump());
 
     return value;
 }
+
+const nlohmann::json::object_t & getObject(const nlohmann::json & value)
+{
+    return ensureType(value, nlohmann::json::value_t::object).get_ref<const nlohmann::json::object_t &>();
+}
+
+const nlohmann::json::array_t & getArray(const nlohmann::json & value)
+{
+    return ensureType(value, nlohmann::json::value_t::array).get_ref<const nlohmann::json::array_t &>();
+}
+
+const nlohmann::json::string_t & getString(const nlohmann::json & value)
+{
+    return ensureType(value, nlohmann::json::value_t::string).get_ref<const nlohmann::json::string_t &>();
+}
+
+const nlohmann::json::number_integer_t & getInteger(const nlohmann::json & value)
+{
+    return ensureType(value, nlohmann::json::value_t::number_integer).get_ref<const nlohmann::json::number_integer_t &>();
+}
+
+const nlohmann::json::boolean_t & getBoolean(const nlohmann::json & value)
+{
+    return ensureType(value, nlohmann::json::value_t::boolean).get_ref<const nlohmann::json::boolean_t &>();
+}
+
+Strings getStringList(const nlohmann::json & value)
+{
+    auto & jsonArray = getArray(value);
+
+    Strings stringList;
+
+    for (const auto & elem : jsonArray)
+        stringList.push_back(getString(elem));
+
+    return stringList;
+}
+
+StringMap getStringMap(const nlohmann::json & value)
+{
+    auto & jsonObject = getObject(value);
+
+    StringMap stringMap;
+
+    for (const auto & [key, value] : jsonObject)
+        stringMap[getString(key)] = getString(value);
+
+    return stringMap;
+}
+
+StringSet getStringSet(const nlohmann::json & value)
+{
+    auto & jsonArray = getArray(value);
+
+    StringSet stringSet;
+
+    for (const auto & elem : jsonArray)
+        stringSet.insert(getString(elem));
+
+    return stringSet;
+}
 }