Skip to content

Commit

Permalink
Store parsed hashes in DerivationOutput
Browse files Browse the repository at this point in the history
It's best to detect invalid data as soon as possible, with data types
that make storing it impossible.
  • Loading branch information
Ericson2314 committed Mar 26, 2020
1 parent e433d4a commit 3ad4f8f
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 54 deletions.
12 changes: 7 additions & 5 deletions src/libexpr/primops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -724,9 +724,11 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * *

auto outPath = state.store->makeFixedOutputPath(outputHashRecursive, h, drvName);
if (!jsonObject) drv.env["out"] = state.store->printStorePath(outPath);
drv.outputs.insert_or_assign("out", DerivationOutput(std::move(outPath),
(outputHashRecursive ? "r:" : "") + printHashType(h.type),
h.to_string(Base16, false)));
drv.outputs.insert_or_assign("out", DerivationOutput(
std::move(outPath),
FileSystemHash(
(outputHashRecursive ? recursive : flat),
std::move(h))));
}

else {
Expand All @@ -739,7 +741,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * *
for (auto & i : outputs) {
if (!jsonObject) drv.env[i] = "";
drv.outputs.insert_or_assign(i,
DerivationOutput(StorePath::dummy.clone(), "", ""));
DerivationOutput(StorePath::dummy.clone(), std::optional<FileSystemHash>()));
}

Hash h = hashDerivationModulo(*state.store, Derivation(drv), true);
Expand All @@ -748,7 +750,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * *
auto outPath = state.store->makeOutputPath(i, h, drvName);
if (!jsonObject) drv.env[i] = state.store->printStorePath(outPath);
drv.outputs.insert_or_assign(i,
DerivationOutput(std::move(outPath), "", ""));
DerivationOutput(std::move(outPath), std::optional<FileSystemHash>()));
}
}

Expand Down
15 changes: 8 additions & 7 deletions src/libstore/build.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3647,10 +3647,7 @@ void DerivationGoal::registerOutputs()

if (fixedOutput) {

bool recursive; Hash h;
i.second.parseHashInfo(recursive, h);

if (!recursive) {
if (i.second.hash->method == flat) {
/* The output path should be a regular file without execute permission. */
if (!S_ISREG(st.st_mode) || (st.st_mode & S_IXUSR) != 0)
throw BuildError(
Expand All @@ -3659,18 +3656,22 @@ void DerivationGoal::registerOutputs()

/* Check the hash. In hash mode, move the path produced by
the derivation to its content-addressed location. */
Hash h2 = recursive ? hashPath(h.type, actualPath).first : hashFile(h.type, actualPath);
Hash h2 = i.second.hash->method == recursive
? hashPath(i.second.hash->hash.type, actualPath).first
: hashFile(i.second.hash->hash.type, actualPath);

auto dest = worker.store.makeFixedOutputPath(recursive, h2, i.second.path.name());

if (h != h2) {
if (i.second.hash->hash != h2) {

/* Throw an error after registering the path as
valid. */
worker.hashMismatch = true;
delayedException = std::make_exception_ptr(
BuildError("hash mismatch in fixed-output derivation '%s':\n wanted: %s\n got: %s",
worker.store.printStorePath(dest), h.to_string(SRI), h2.to_string(SRI)));
worker.store.printStorePath(dest),
i.second.hash->hash.to_string(SRI),
h2.to_string(SRI)));

Path actualDest = worker.store.Store::toRealPath(dest);

Expand Down
106 changes: 76 additions & 30 deletions src/libstore/derivations.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,19 @@

namespace nix {


void DerivationOutput::parseHashInfo(bool & recursive, Hash & hash) const
{
recursive = false;
string algo = hashAlgo;

if (string(algo, 0, 2) == "r:") {
recursive = true;
algo = string(algo, 2);
string fileIngestionPrefix(const FileIngestionMethod m) {
switch (m) {
case flat:
return "";
case recursive:
return "r:";
default:
throw Error("impossible, caught both cases");
}
}

HashType hashType = parseHashType(algo);
if (hashType == htUnknown)
throw Error("unknown hash algorithm '%s'", algo);

hash = Hash(this->hash, hashType);
std::string FileSystemHash::printMethodAlgo() const {
return fileIngestionPrefix(method) + printHashType(hash.type);
}


Expand All @@ -35,7 +32,7 @@ BasicDerivation::BasicDerivation(const BasicDerivation & other)
{
for (auto & i : other.outputs)
outputs.insert_or_assign(i.first,
DerivationOutput(i.second.path.clone(), std::string(i.second.hashAlgo), std::string(i.second.hash)));
DerivationOutput(i.second.path.clone(), std::optional<FileSystemHash>(i.second.hash)));
for (auto & i : other.inputSrcs)
inputSrcs.insert(i.clone());
}
Expand Down Expand Up @@ -142,6 +139,33 @@ static StringSet parseStrings(std::istream & str, bool arePaths)
}


static DerivationOutput parseDerivationOutput(const Store & store, istringstream_nocopy & str)
{
expect(str, ","); auto path = store.parseStorePath(parsePath(str));
expect(str, ","); auto hashAlgo = parseString(str);
expect(str, ","); const auto hash = parseString(str);
expect(str, ")");

auto method = flat;
std::optional<FileSystemHash> fsh;
if (hashAlgo != "") {
if (string(hashAlgo, 0, 2) == "r:") {
method = recursive;
hashAlgo = string(hashAlgo, 2);
}
const HashType hashType = parseHashType(hashAlgo);
if (hashType == htUnknown)
throw Error("unknown hash hashAlgorithm '%s'", hashAlgo);
fsh = FileSystemHash {
std::move(method),
Hash(hash, hashType),
};
}

return DerivationOutput(std::move(path), std::move(fsh));
}


static Derivation parseDerivation(const Store & store, const string & s)
{
Derivation drv;
Expand All @@ -151,11 +175,7 @@ static Derivation parseDerivation(const Store & store, const string & s)
/* Parse the list of outputs. */
while (!endOfList(str)) {
expect(str, "("); std::string id = parseString(str);
expect(str, ","); auto path = store.parseStorePath(parsePath(str));
expect(str, ","); auto hashAlgo = parseString(str);
expect(str, ","); auto hash = parseString(str);
expect(str, ")");
drv.outputs.emplace(id, DerivationOutput(std::move(path), std::move(hashAlgo), std::move(hash)));
drv.outputs.emplace(id, parseDerivationOutput(store, str));
}

/* Parse the list of input derivations. */
Expand Down Expand Up @@ -275,8 +295,9 @@ string Derivation::unparse(const Store & store, bool maskOutputs,
if (first) first = false; else s += ',';
s += '('; printUnquotedString(s, i.first);
s += ','; printUnquotedString(s, maskOutputs ? "" : store.printStorePath(i.second.path));
s += ','; printUnquotedString(s, i.second.hashAlgo);
s += ','; printUnquotedString(s, i.second.hash);
s += ','; printUnquotedString(s, i.second.hash ? i.second.hash->printMethodAlgo() : "");
s += ','; printUnquotedString(s,
i.second.hash ? i.second.hash->hash.to_string(Base16, false) : "");
s += ')';
}

Expand Down Expand Up @@ -332,7 +353,7 @@ bool BasicDerivation::isFixedOutput() const
{
return outputs.size() == 1 &&
outputs.begin()->first == "out" &&
outputs.begin()->second.hash != "";
outputs.begin()->second.hash;
}


Expand Down Expand Up @@ -365,8 +386,8 @@ Hash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutput
if (drv.isFixedOutput()) {
DerivationOutputs::const_iterator i = drv.outputs.begin();
return hashString(htSHA256, "fixed:out:"
+ i->second.hashAlgo + ":"
+ i->second.hash + ":"
+ i->second.hash->printMethodAlgo() + ":"
+ i->second.hash->hash.to_string(Base16, false) + ":"
+ store.printStorePath(i->second.path));
}

Expand Down Expand Up @@ -409,17 +430,39 @@ StorePathSet BasicDerivation::outputPaths() const
return paths;
}

static DerivationOutput readDerivationOutput(Source & in, const Store & store)
{
auto path = store.parseStorePath(readString(in));
auto hashAlgo = readString(in);
const auto hash = readString(in);

auto method = flat;
std::optional<FileSystemHash> fsh;
if (hashAlgo != "") {
if (string(hashAlgo, 0, 2) == "r:") {
method = recursive;
hashAlgo = string(hashAlgo, 2);
}
HashType hashType = parseHashType(hashAlgo);
if (hashType == htUnknown)
throw Error("unknown hash hashAlgorithm '%s'", hashAlgo);
fsh = FileSystemHash {
std::move(method),
Hash(hash, hashType),
};
}

return DerivationOutput(std::move(path), std::move(fsh));
}

Source & readDerivation(Source & in, const Store & store, BasicDerivation & drv)
{
drv.outputs.clear();
auto nr = readNum<size_t>(in);
for (size_t n = 0; n < nr; n++) {
auto name = readString(in);
auto path = store.parseStorePath(readString(in));
auto hashAlgo = readString(in);
auto hash = readString(in);
drv.outputs.emplace(name, DerivationOutput(std::move(path), std::move(hashAlgo), std::move(hash)));
auto output = readDerivationOutput(in, store);
drv.outputs.emplace(name, output);
}

drv.inputSrcs = readStorePaths<StorePathSet>(store, in);
Expand All @@ -441,7 +484,10 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr
{
out << drv.outputs.size();
for (auto & i : drv.outputs)
out << i.first << store.printStorePath(i.second.path) << i.second.hashAlgo << i.second.hash;
out << i.first
<< store.printStorePath(i.second.path)
<< i.second.hash->printMethodAlgo()
<< i.second.hash->hash.to_string(Base16, false);
writeStorePaths(store, out, drv.inputSrcs);
out << drv.platform << drv.builder << drv.args;
out << drv.env.size();
Expand Down
28 changes: 23 additions & 5 deletions src/libstore/derivations.hh
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,38 @@ namespace nix {

/* Abstract syntax of derivations. */

enum FileIngestionMethod : uint8_t {
flat,
recursive,
};

std::string fileIngestionPrefix(FileIngestionMethod);

/// Pair of a hash, and how the file system was ingested
struct FileSystemHash {
FileIngestionMethod method;
Hash hash;
FileSystemHash(FileIngestionMethod && method, Hash && hash)
: method(std::move(method))
, hash(std::move(hash))
{ }
FileSystemHash(const FileSystemHash &) = default;
FileSystemHash(FileSystemHash &&) = default;
FileSystemHash & operator = (const FileSystemHash &) = default;
std::string printMethodAlgo() const;
};

struct DerivationOutput
{
StorePath path;
std::string hashAlgo; /* hash used for expected hash computation */
std::string hash; /* expected hash, may be null */
DerivationOutput(StorePath && path, std::string && hashAlgo, std::string && hash)
std::optional<FileSystemHash> hash; /* hash used for expected hash computation */
DerivationOutput(StorePath && path, std::optional<FileSystemHash> && hash)
: path(std::move(path))
, hashAlgo(std::move(hashAlgo))
, hash(std::move(hash))
{ }
DerivationOutput(const DerivationOutput &) = default;
DerivationOutput(DerivationOutput &&) = default;
DerivationOutput & operator = (const DerivationOutput &) = default;
void parseHashInfo(bool & recursive, Hash & hash) const;
};

typedef std::map<string, DerivationOutput> DerivationOutputs;
Expand Down
11 changes: 7 additions & 4 deletions src/libstore/local-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -557,10 +557,13 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat
if (out == drv.outputs.end())
throw Error("derivation '%s' does not have an output named 'out'", printStorePath(drvPath));

bool recursive; Hash h;
out->second.parseHashInfo(recursive, h);

check(makeFixedOutputPath(recursive, h, drvName), out->second.path, "out");
check(
makeFixedOutputPath(
// TODO convert `makeFixedOutputPath` to take the enum
out->second.hash->method == recursive,
out->second.hash->hash,
drvName),
out->second.path, "out");
}

else {
Expand Down
6 changes: 3 additions & 3 deletions src/nix/show-derivation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ struct CmdShowDerivation : InstallablesCommand
for (auto & output : drv.outputs) {
auto outputObj(outputsObj.object(output.first));
outputObj.attr("path", store->printStorePath(output.second.path));
if (output.second.hash != "") {
outputObj.attr("hashAlgo", output.second.hashAlgo);
outputObj.attr("hash", output.second.hash);
if (output.second.hash) {
outputObj.attr("hashAlgo", output.second.hash->printMethodAlgo());
outputObj.attr("hash", output.second.hash->hash.to_string(Base16, false));
}
}
}
Expand Down

0 comments on commit 3ad4f8f

Please sign in to comment.