From 3fefc2b28494468c7a6078430eaaf8a6c8f17230 Mon Sep 17 00:00:00 2001 From: Felix Uhl Date: Fri, 28 Jul 2023 22:23:56 +0200 Subject: [PATCH] Fix derivation load assertion errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When loading a derivation from a JSON, malformed input would trigger cryptic "assertion failed" errors. Simply replacing calls to `operator []` with calls to `.at()` was not enough, as this would cause json.execptions to be printed verbatim. Display nice error messages instead and give some indication where the error happened. *Before:* ``` $ echo 4 | nix derivation add error: [json.exception.type_error.305] cannot use operator[] with a string argument with number $ nix derivation show nixpkgs#hello | nix derivation add Assertion failed: (it != m_value.object->end()), function operator[], file /nix/store/8h9pxgq1776ns6qi5arx08ifgnhmgl22-nlohmann_json-3.11.2/include/nlohmann/json.hpp, line 2135. $ nix derivation show nixpkgs#hello | jq '.[] | .name = 5' | nix derivation add error: [json.exception.type_error.302] type must be string, but is object $ nix derivation show nixpkgs#hello | jq '.[] | .outputs = { out: "/nix/store/8j3f8j-hello" }' | nix derivation add error: [json.exception.type_error.302] type must be object, but is string ``` *After:* ``` $ echo 4 | nix derivation add error: Expected JSON of derivation to be of type 'object', but it is of type 'number' $ nix derivation show nixpkgs#hello | nix derivation add error: Expected JSON object to contain key 'name' but it doesn't $ nix derivation show nixpkgs#hello | jq '.[] | .name = 5' | nix derivation add error: Expected JSON value to be of type 'string' but it is of type 'number' $ nix derivation show nixpkgs#hello | jq '.[] | .outputs = { out: "/nix/store/8j3f8j-hello" }' | nix derivation add error: … while reading key 'outputs' error: Expected JSON value to be of type 'object' but it is of type 'string' ``` --- doc/manual/src/release-notes/rl-next.md | 2 ++ src/libstore/derivations.cc | 40 +++++++++++++++++-------- src/libutil/json-utils.cc | 24 +++++++++++++++ src/libutil/json-utils.hh | 22 ++++++++++++++ 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 keys; + ensureType(_json, nlohmann::detail::value_t::object); auto json = (std::map) _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(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(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 @@ -10,6 +10,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>` below, we need to track what * types are not already using `null`. Only for them can we use `null`