Skip to content

Commit

Permalink
Fix derivation load assertion errors
Browse files Browse the repository at this point in the history
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 with key 'name' 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 with key 'out' to be of type 'object' but it is of type 'string'
```
  • Loading branch information
iFreilicht committed Jul 31, 2023
1 parent dcdd5fe commit c02e6e6
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 13 deletions.
2 changes: 2 additions & 0 deletions doc/manual/src/release-notes/rl-next.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` are now clearer and more detailed.
47 changes: 34 additions & 13 deletions src/libstore/derivations.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1097,36 +1097,57 @@ Derivation Derivation::fromJSON(
const Store & store,
const nlohmann::json & json)
{
using nlohmann::detail::value_t;

Derivation res;

res.name = json["name"];
if (json.type() != value_t::object) {
throw Error(
"Expected JSON of derivation to be of type '%s', but it is of type '%s'",
"object", // Colour this word as well
json.type_name());
}

res.name = valueOfTypeAt(json, value_t::string, "name");

{
auto & outputsObj = json["outputs"];
for (auto & [outputName, output] : outputsObj.items()) {
try {
auto & outputsObj = valueOfTypeAt(json, value_t::object, "outputs");
for (auto & [outputName, _] : outputsObj.items()) {
auto & output = valueOfTypeAt(outputsObj, value_t::object, outputName);
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 = valueOfTypeAt(json, value_t::array, "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'");
throw;
}

{
auto & inputDrvsObj = json["inputDrvs"];
for (auto & [inputDrvPath, inputOutputs] : inputDrvsObj.items())
try {
auto & inputDrvsObj = valueOfTypeAt(json, value_t::object, "inputDrvs");
for (auto & [inputDrvPath, _] : inputDrvsObj.items()) {
auto & inputOutputs = valueOfTypeAt(inputDrvsObj, value_t::array, inputDrvPath);
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 = valueOfTypeAt(json, value_t::string, "system");
res.builder = valueOfTypeAt(json, value_t::string, "builder");
res.args = valueOfTypeAt(json, value_t::array, "args");
res.env = valueOfTypeAt(json, value_t::object, "env");

return res;
}
Expand Down
20 changes: 20 additions & 0 deletions src/libutil/json-utils.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "json-utils.hh"
#include "error.hh"

namespace nix {

Expand All @@ -16,4 +17,23 @@ nlohmann::json * get(nlohmann::json & map, const std::string & key)
return &*i;
}

const nlohmann::json & valueOfTypeAt(
const nlohmann::json & map,
nlohmann::json::value_type expectedType,
const std::string & key)
{
if (!map.contains(key))
throw Error("Expected JSON object to contain key '%s' but it doesn't", key);

auto & value = map[key];

if (value.type() != expectedType)
throw Error(
"Expected JSON value with key '%s' to be of type '%s' but it is of type '%s'",
key,
nlohmann::json(expectedType).type_name(),
value.type_name());

return value;
}
}
5 changes: 5 additions & 0 deletions src/libutil/json-utils.hh
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ const nlohmann::json * get(const nlohmann::json & map, const std::string & key);

nlohmann::json * get(nlohmann::json & map, const std::string & key);

const nlohmann::json & valueOfTypeAt(
const nlohmann::json & map,
nlohmann::json::value_type expectedType,
const std::string & key);

/**
* 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`
Expand Down

0 comments on commit c02e6e6

Please sign in to comment.