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 <John.Ericson@Obsidian.Systems>

.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"));
+        auto pubKeysJson = nlohmann::json::parse(getStrAttr(attrs, "publicKeys"));
-        ensureType(publicKeysJson, nlohmann::json::value_t::array);
+        auto & pubKeys = getArray(pubKeysJson);
-        publicKeys = publicKeysJson.get<std::vector<PublicKey>>();
+
+        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 = getObject(_json);
-    auto json = (std::map<std::string, nlohmann::json>) _json;
 
     for (const auto & [key, _] : json)
         keys.insert(key);
 
     auto methodAlgo = [&]() -> std::pair<ContentAddressMethod, HashAlgorithm> {
-        std::string hashAlgoStr = json["hashAlgo"];
+        auto & str = getString(valueAt(json, "hashAlgo"));
-        // remaining to parse, will be mutated by parsers
+        std::string_view s = str;
-        std::string_view s = hashAlgoStr;
         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] : getObject(valueAt(json, "outputs"))) {
-        for (auto & [outputName, output] : outputsObj.items()) {
             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 : getArray(valueAt(json, "inputSrcs")))
-        for (auto & input : inputsList)
             res.inputSrcs.insert(store.parseStorePath(static_cast<const std::string &>(input)));
     } catch (Error & e) {
         e.addTrace({}, "while reading key 'inputSrcs'");
@@ -1391,18 +1387,17 @@ 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 &>(
+            node.value = getStringSet(valueAt(json, "outputs"));
-                ensureType(valueAt(json, "outputs"), value_t::array));
+            for (auto & [outputId, childNode] : getObject(valueAt(json, "dynamicOutputs"))) {
-            for (auto & [outputId, childNode] : ensureType(valueAt(json, "dynamicOutputs"), value_t::object).items()) {
                 xpSettings.require(Xp::DynamicDerivations);
                 node.childMap[outputId] = doInput(childNode);
             }
             return node;
         };
-        auto & inputDrvsObj = ensureType(valueAt(json, "inputDrvs"), value_t::object);
+        for (auto & [inputDrvPath, inputOutputs] : getObject(valueAt(json, "inputDrvs")))
-        for (auto & [inputDrvPath, inputOutputs] : inputDrvsObj.items())
             res.inputDrvs.map[store.parseStorePath(inputDrvPath)] =
                 doInput(inputOutputs);
     } catch (Error & e) {
@@ -1410,10 +1405,10 @@ Derivation Derivation::fromJSON(
         throw;
     }
 
-    res.platform = ensureType(valueAt(json, "system"), value_t::string);
+    res.platform = getString(valueAt(json, "system"));
-    res.builder = ensureType(valueAt(json, "builder"), value_t::string);
+    res.builder = getString(valueAt(json, "builder"));
-    res.args = ensureType(valueAt(json, "args"), value_t::array);
+    res.args = getStringList(valueAt(json, "args"));
-    res.env = ensureType(valueAt(json, "env"), value_t::object);
+    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 &>(
+            getString(valueAt(json, "downloadHash")),
-                ensureType(valueAt(json, "downloadHash"), value_t::string)),
             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);
+    auto & json = getObject(_json);
-    res.narHash = Hash::parseAny(
+    res.narHash = Hash::parseAny(getString(valueAt(json, "narHash")), std::nullopt);
-        static_cast<const std::string &>(
+    res.narSize = getInteger(valueAt(json, "narSize"));
-            ensureType(valueAt(json, "narHash"), value_t::string)),
-        std::nullopt);
-    res.narSize = ensureType(valueAt(json, "narSize"), value_t::number_integer);
 
     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(
+        res.ca = ContentAddress::parse(getString(valueAt(json, "ca")));
-            static_cast<const std::string &>(
-                ensureType(valueAt(json, "ca"), value_t::string)));
 
     if (json.contains("deriver"))
-        res.deriver = store.parseStorePath(
+        res.deriver = store.parseStorePath(getString(valueAt(json, "deriver")));
-            static_cast<const std::string &>(
-                ensureType(valueAt(json, "deriver"), value_t::string)));
 
     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;
+}
 }

src/libutil/json-utils.hh

@@ -3,6 +3,9 @@
 
 #include <nlohmann/json.hpp>
 #include <list>
+#include <nlohmann/json_fwd.hpp>
+
+#include "types.hh"
 
 namespace nix {
 
@@ -11,26 +14,30 @@ 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
+ * Get the value of a json object at a key safely, failing with a nice
- * with a Nix Error if the key does not exist.
+ * 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 nlohmann::json::object_t & map,
     const std::string & key);
 
+std::optional<nlohmann::json> optionalValueAt(const nlohmann::json & value, const std::string & key);
+
 /**
- * Ensure the type of a json object is what you expect, failing
+ * Downcast the json object, failing with a nice error if the conversion fails.
- * with a Nix Error if it isn't.
+ * See https://json.nlohmann.me/features/types/
- *
- * Use before type conversions and element access to avoid ugly exceptions.
  */
-const nlohmann::json & ensureType(
+std::optional<nlohmann::json> getNullable(const nlohmann::json & value);
-    const nlohmann::json & value,
+const nlohmann::json::object_t & getObject(const nlohmann::json & value);
-    nlohmann::json::value_type expectedType);
+const nlohmann::json::array_t & getArray(const nlohmann::json & value);
+const nlohmann::json::string_t & getString(const nlohmann::json & value);
+const nlohmann::json::number_integer_t & getInteger(const nlohmann::json & value);
+const nlohmann::json::boolean_t & getBoolean(const nlohmann::json & value);
+Strings getStringList(const nlohmann::json & value);
+StringMap getStringMap(const nlohmann::json & value);
+StringSet getStringSet(const nlohmann::json & value);
 
 /**
  * For `adl_serializer<std::optional<T>>` below, we need to track what

tests/unit/libfetchers/local.mk

@@ -0,0 +1,32 @@
+check: libfetchers-tests_RUN
+
+programs += libfetchers-tests
+
+libfetchers-tests_NAME = libnixfetchers-tests
+
+libfetchers-tests_ENV := _NIX_TEST_UNIT_DATA=$(d)/data
+
+libfetchers-tests_DIR := $(d)
+
+ifeq ($(INSTALL_UNIT_TESTS), yes)
+  libfetchers-tests_INSTALL_DIR := $(checkbindir)
+else
+  libfetchers-tests_INSTALL_DIR :=
+endif
+
+libfetchers-tests_SOURCES := $(wildcard $(d)/*.cc)
+
+libfetchers-tests_EXTRA_INCLUDES = \
+    -I tests/unit/libstore-support \
+    -I tests/unit/libutil-support \
+    $(INCLUDE_libfetchers) \
+    $(INCLUDE_libstore) \
+    $(INCLUDE_libutil)
+
+libfetchers-tests_CXXFLAGS += $(libfetchers-tests_EXTRA_INCLUDES)
+
+libfetchers-tests_LIBS = \
+    libstore-test-support libutil-test-support \
+    libfetchers libstore libutil
+
+libfetchers-tests_LDFLAGS := -lrapidcheck $(GTEST_LIBS)

tests/unit/libfetchers/public-key.cc

@@ -0,0 +1,18 @@
+#include <gtest/gtest.h>
+#include "fetchers.hh"
+#include "json-utils.hh"
+
+namespace nix {
+    TEST(PublicKey, jsonSerialization) {
+        auto json = nlohmann::json(fetchers::PublicKey { .key = "ABCDE" });
+
+        ASSERT_EQ(json, R"({ "key": "ABCDE", "type": "ssh-ed25519" })"_json);
+    }
+    TEST(PublicKey, jsonDeserialization) {
+        auto pubKeyJson = R"({ "key": "ABCDE", "type": "ssh-ed25519" })"_json;
+        fetchers::PublicKey pubKey = pubKeyJson;
+
+        ASSERT_EQ(pubKey.key, "ABCDE");
+        ASSERT_EQ(pubKey.type, "ssh-ed25519");
+    }
+}

tests/unit/libutil/json-utils.cc

@@ -3,6 +3,7 @@
 
 #include <gtest/gtest.h>
 
+#include "error.hh"
 #include "json-utils.hh"
 
 namespace nix {
@@ -55,4 +56,108 @@ TEST(from_json, vectorOfOptionalInts) {
     ASSERT_FALSE(vals.at(1).has_value());
 }
 
+TEST(valueAt, simpleObject) {
+    auto simple = R"({ "hello": "world" })"_json;
+
+    ASSERT_EQ(valueAt(getObject(simple), "hello"), "world");
+
+    auto nested = R"({ "hello": { "world": "" } })"_json;
+
+    auto & nestedObject = valueAt(getObject(nested), "hello");
+
+    ASSERT_EQ(valueAt(nestedObject, "world"), "");
+}
+
+TEST(valueAt, missingKey) {
+    auto json = R"({ "hello": { "nested": "world" } })"_json;
+
+    auto & obj = getObject(json);
+
+    ASSERT_THROW(valueAt(obj, "foo"), Error);
+}
+
+TEST(getObject, rightAssertions) {
+    auto simple = R"({ "object": {} })"_json;
+
+    ASSERT_EQ(getObject(valueAt(getObject(simple), "object")), (nlohmann::json::object_t {}));
+
+    auto nested = R"({ "object": { "object": {} } })"_json;
+
+    auto & nestedObject = getObject(valueAt(getObject(nested), "object"));
+
+    ASSERT_EQ(nestedObject, getObject(nlohmann::json::parse(R"({ "object": {} })")));
+    ASSERT_EQ(getObject(valueAt(getObject(nestedObject), "object")), (nlohmann::json::object_t {}));
+}
+
+TEST(getObject, wrongAssertions) {
+    auto json = R"({ "object": {}, "array": [], "string": "", "int": 0, "boolean": false })"_json;
+
+    auto & obj = getObject(json);
+
+    ASSERT_THROW(getObject(valueAt(obj, "array")), Error);
+    ASSERT_THROW(getObject(valueAt(obj, "string")), Error);
+    ASSERT_THROW(getObject(valueAt(obj, "int")), Error);
+    ASSERT_THROW(getObject(valueAt(obj, "boolean")), Error);
+}
+
+TEST(getArray, rightAssertions) {
+    auto simple = R"({ "array": [] })"_json;
+
+    ASSERT_EQ(getArray(valueAt(getObject(simple), "array")), (nlohmann::json::array_t {}));
+}
+
+TEST(getArray, wrongAssertions) {
+    auto json = R"({ "object": {}, "array": [], "string": "", "int": 0, "boolean": false })"_json;
+
+    ASSERT_THROW(getArray(valueAt(json, "object")), Error);
+    ASSERT_THROW(getArray(valueAt(json, "string")), Error);
+    ASSERT_THROW(getArray(valueAt(json, "int")), Error);
+    ASSERT_THROW(getArray(valueAt(json, "boolean")), Error);
+}
+
+TEST(getString, rightAssertions) {
+    auto simple = R"({ "string": "" })"_json;
+
+    ASSERT_EQ(getString(valueAt(getObject(simple), "string")), "");
+}
+
+TEST(getString, wrongAssertions) {
+    auto json = R"({ "object": {}, "array": [], "string": "", "int": 0, "boolean": false })"_json;
+
+    ASSERT_THROW(getString(valueAt(json, "object")), Error);
+    ASSERT_THROW(getString(valueAt(json, "array")), Error);
+    ASSERT_THROW(getString(valueAt(json, "int")), Error);
+    ASSERT_THROW(getString(valueAt(json, "boolean")), Error);
+}
+
+TEST(getInteger, rightAssertions) {
+    auto simple = R"({ "int": 0 })"_json;
+
+    ASSERT_EQ(getInteger(valueAt(getObject(simple), "int")), 0);
+}
+
+TEST(getInteger, wrongAssertions) {
+    auto json = R"({ "object": {}, "array": [], "string": "", "int": 0, "boolean": false })"_json;
+
+    ASSERT_THROW(getInteger(valueAt(json, "object")), Error);
+    ASSERT_THROW(getInteger(valueAt(json, "array")), Error);
+    ASSERT_THROW(getInteger(valueAt(json, "string")), Error);
+    ASSERT_THROW(getInteger(valueAt(json, "boolean")), Error);
+}
+
+TEST(getBoolean, rightAssertions) {
+    auto simple = R"({ "boolean": false })"_json;
+
+    ASSERT_EQ(getBoolean(valueAt(getObject(simple), "boolean")), false);
+}
+
+TEST(getBoolean, wrongAssertions) {
+    auto json = R"({ "object": {}, "array": [], "string": "", "int": 0, "boolean": false })"_json;
+
+    ASSERT_THROW(getBoolean(valueAt(json, "object")), Error);
+    ASSERT_THROW(getBoolean(valueAt(json, "array")), Error);
+    ASSERT_THROW(getBoolean(valueAt(json, "string")), Error);
+    ASSERT_THROW(getBoolean(valueAt(json, "int")), Error);
+}
+
 } /* namespace nix */