Merge remote-tracking branch 'obsidian/no-hash-type-unknown' into validPathInfo-ca-proper-datatype

16 files changed, 89 insertions, 64 deletions

diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index b6e889e15..98742e0c8 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -718,14 +718,14 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * *
         if (outputs.size() != 1 || *(outputs.begin()) != "out")
             throw Error(format("multiple outputs are not supported in fixed-output derivations, at %1%") % posDrvName);
 
-        HashType ht = outputHashAlgo.empty() ? HashType::Unknown : parseHashType(outputHashAlgo);
+        std::optional<HashType> ht = parseHashTypeOpt(outputHashAlgo);
         Hash h(*outputHash, ht);
 
         auto outPath = state.store->makeFixedOutputPath(ingestionMethod, h, drvName);
         if (!jsonObject) drv.env["out"] = state.store->printStorePath(outPath);
         drv.outputs.insert_or_assign("out", DerivationOutput(
-                std::move(outPath),
-                FileSystemHash(ingestionMethod, std::move(h))));
+            std::move(outPath),
+            FileSystemHash(ingestionMethod, std::move(h))));
     }
 
     else {
@@ -931,14 +931,14 @@ static void prim_findFile(EvalState & state, const Pos & pos, Value * * args, Va
 static void prim_hashFile(EvalState & state, const Pos & pos, Value * * args, Value & v)
 {
     string type = state.forceStringNoCtx(*args[0], pos);
-    HashType ht = parseHashType(type);
-    if (ht == HashType::Unknown)
+    std::optional<HashType> ht = parseHashType(type);
+    if (!ht)
       throw Error(format("unknown hash type '%1%', at %2%") % type % pos);
 
     PathSet context; // discarded
     Path p = state.coerceToPath(pos, *args[1], context);
 
-    mkString(v, hashFile(ht, state.checkSourcePath(p)).to_string(Base::Base16, false), context);
+    mkString(v, hashFile(*ht, state.checkSourcePath(p)).to_string(Base::Base16, false), context);
 }
 
 /* Read a directory (without . or ..) */
@@ -1809,14 +1809,14 @@ static void prim_stringLength(EvalState & state, const Pos & pos, Value * * args
 static void prim_hashString(EvalState & state, const Pos & pos, Value * * args, Value & v)
 {
     string type = state.forceStringNoCtx(*args[0], pos);
-    HashType ht = parseHashType(type);
-    if (ht == HashType::Unknown)
+    std::optional<HashType> ht = parseHashType(type);
+    if (!ht)
       throw Error(format("unknown hash type '%1%', at %2%") % type % pos);
 
     PathSet context; // discarded
     string s = state.forceString(*args[1], context, pos);
 
-    mkString(v, hashString(ht, s).to_string(Base::Base16, false), context);
+    mkString(v, hashString(*ht, s).to_string(Base::Base16, false), context);
 }
 
 
diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 7f79d6a2a..ea7319150 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -3723,8 +3723,8 @@ void DerivationGoal::registerOutputs()
             /* Check the hash. In hash mode, move the path produced by
                the derivation to its content-addressed location. */
             Hash h2 = i.second.hash->method == FileIngestionMethod::Recursive
-                ? hashPath(i.second.hash->hash.type, actualPath).first
-                : hashFile(i.second.hash->hash.type, actualPath);
+                ? hashPath(*i.second.hash->hash.type, actualPath).first
+                : hashFile(*i.second.hash->hash.type, actualPath);
 
             auto dest = worker.store.makeFixedOutputPath(i.second.hash->method, h2, i.second.path.name());
 
@@ -5004,7 +5004,7 @@ bool Worker::pathContentsGood(const StorePath & path)
     if (!pathExists(store.printStorePath(path)))
         res = false;
     else {
-        HashResult current = hashPath(info->narHash.type, store.printStorePath(path));
+        HashResult current = hashPath(*info->narHash.type, store.printStorePath(path));
         Hash nullHash(HashType::SHA256);
         res = info->narHash == nullHash || info->narHash == current.first;
     }
diff --git a/src/libstore/builtins/fetchurl.cc b/src/libstore/builtins/fetchurl.cc
index b70e960f8..770df2927 100644
--- a/src/libstore/builtins/fetchurl.cc
+++ b/src/libstore/builtins/fetchurl.cc
@@ -63,9 +63,9 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData)
         for (auto hashedMirror : settings.hashedMirrors.get())
             try {
                 if (!hasSuffix(hashedMirror, "/")) hashedMirror += '/';
-                auto ht = parseHashType(getAttr("outputHashAlgo"));
+                auto ht = parseHashTypeOpt(getAttr("outputHashAlgo"));
                 auto h = Hash(getAttr("outputHash"), ht);
-                fetch(hashedMirror + printHashType(h.type) + "/" + h.to_string(Base::Base16, false));
+                fetch(hashedMirror + printHashType(*h.type) + "/" + h.to_string(Base::Base16, false));
                 return;
             } catch (Error & e) {
                 debug(e.what());
diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc
index 718c7ffc3..f30f53ff8 100644
--- a/src/libstore/content-address.cc
+++ b/src/libstore/content-address.cc
@@ -3,7 +3,7 @@
 namespace nix {
 
 std::string FileSystemHash::printMethodAlgo() const {
-    return makeFileIngestionPrefix(method) + printHashType(hash.type);
+    return makeFileIngestionPrefix(method) + printHashType(*hash.type);
 }
 
 std::string makeFileIngestionPrefix(const FileIngestionMethod m) {
@@ -46,7 +46,7 @@ ContentAddress parseContentAddress(std::string_view rawCa) {
         if (prefix == "text") {
             auto hashTypeAndHash = rawCa.substr(prefixSeparator+1, string::npos);
             Hash hash = Hash(string(hashTypeAndHash));
-            if (hash.type != HashType::SHA256) {
+            if (*hash.type != HashType::SHA256) {
                 throw Error("parseContentAddress: the text hash should have type SHA256");
             }
             return TextHash { hash };
diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc
index f1569bf22..a522eb950 100644
--- a/src/libstore/derivations.cc
+++ b/src/libstore/derivations.cc
@@ -138,8 +138,6 @@ static DerivationOutput parseDerivationOutput(const Store & store, istringstream
             hashAlgo = string(hashAlgo, 2);
         }
         const HashType hashType = parseHashType(hashAlgo);
-        if (hashType == HashType::Unknown)
-            throw Error("unknown hash hashAlgorithm '%s'", hashAlgo);
         fsh = FileSystemHash {
             std::move(method),
             Hash(hash, hashType),
@@ -428,8 +426,6 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store)
             hashAlgo = string(hashAlgo, 2);
         }
         HashType hashType = parseHashType(hashAlgo);
-        if (hashType == HashType::Unknown)
-            throw Error("unknown hash hashAlgorithm '%s'", hashAlgo);
         fsh = FileSystemHash {
             std::move(method),
             Hash(hash, hashType),
diff --git a/src/libstore/export-import.cc b/src/libstore/export-import.cc
index c9731a6bd..aef5cc6da 100644
--- a/src/libstore/export-import.cc
+++ b/src/libstore/export-import.cc
@@ -55,7 +55,7 @@ void Store::exportPath(const StorePath & path, Sink & sink)
        filesystem corruption from spreading to other machines.
        Don't complain if the stored hash is zero (unknown). */
     Hash hash = hashAndWriteSink.currentHash();
-    if (hash != info->narHash && info->narHash != Hash(info->narHash.type))
+    if (hash != info->narHash && info->narHash != Hash(*info->narHash.type))
         throw Error("hash of path '%s' has changed from '%s' to '%s'!",
             printStorePath(path), info->narHash.to_string(), hash.to_string());
 
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 93697ae47..5f0053753 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -1269,9 +1269,9 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
 
                 std::unique_ptr<AbstractHashSink> hashSink;
                 if (info->ca || !info->references.count(info->path))
-                    hashSink = std::make_unique<HashSink>(info->narHash.type);
+                    hashSink = std::make_unique<HashSink>(*info->narHash.type);
                 else
-                    hashSink = std::make_unique<HashModuloSink>(info->narHash.type, storePathToHash(printStorePath(info->path)));
+                    hashSink = std::make_unique<HashModuloSink>(*info->narHash.type, storePathToHash(printStorePath(info->path)));
 
                 dumpPath(Store::toRealPath(i), *hashSink);
                 auto current = hashSink->finish();
diff --git a/src/libutil/args.cc b/src/libutil/args.cc
index 4a3f5aae8..4fe9539e4 100644
--- a/src/libutil/args.cc
+++ b/src/libutil/args.cc
@@ -162,8 +162,18 @@ Args::Flag Args::Flag::mkHashTypeFlag(std::string && longName, HashType * ht)
         .labels = {"hash-algo"},
         .handler = {[ht](std::string s) {
             *ht = parseHashType(s);
-            if (*ht == HashType::Unknown)
-                throw UsageError("unknown hash type '%1%'", s);
+        }}
+    };
+}
+
+Args::Flag Args::Flag::mkHashTypeOptFlag(std::string && longName, std::optional<HashType> * oht)
+{
+    return Flag {
+        .longName = std::move(longName),
+        .description = "hash algorithm ('md5', 'sha1', 'sha256', or 'sha512'). Optional as can also be gotten from SRI hash itself.",
+        .labels = {"hash-algo"},
+        .handler = {[oht](std::string s) {
+            *oht = std::optional<HashType> { parseHashType(s) };
         }}
     };
 }
diff --git a/src/libutil/args.hh b/src/libutil/args.hh
index fc8f82af5..f2315f67a 100644
--- a/src/libutil/args.hh
+++ b/src/libutil/args.hh
@@ -85,6 +85,7 @@ protected:
         Handler handler;
 
         static Flag mkHashTypeFlag(std::string && longName, HashType * ht);
+        static Flag mkHashTypeOptFlag(std::string && longName, std::optional<HashType> * oht);
     };
 
     std::map<std::string, Flag::ptr> longFlags;
diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc
index 5e6edeec3..6b9effdd2 100644
--- a/src/libutil/hash.cc
+++ b/src/libutil/hash.cc
@@ -4,6 +4,7 @@
 #include <openssl/md5.h>
 #include <openssl/sha.h>
 
+#include "args.hh"
 #include "hash.hh"
 #include "archive.hh"
 #include "util.hh"
@@ -18,11 +19,13 @@ namespace nix {
 
 void Hash::init()
 {
-    if (type == HashType::MD5) hashSize = md5HashSize;
-    else if (type == HashType::SHA1) hashSize = sha1HashSize;
-    else if (type == HashType::SHA256) hashSize = sha256HashSize;
-    else if (type == HashType::SHA512) hashSize = sha512HashSize;
-    else abort();
+    if (!type) abort();
+    switch (*type) {
+    case HashType::MD5: hashSize = md5HashSize; break;
+    case HashType::SHA1: hashSize = sha1HashSize; break;
+    case HashType::SHA256: hashSize = sha256HashSize; break;
+    case HashType::SHA512: hashSize = sha512HashSize; break;
+    }
     assert(hashSize <= maxHashSize);
     memset(hash, 0, maxHashSize);
 }
@@ -102,11 +105,18 @@ string printHash16or32(const Hash & hash)
 }
 
 
+HashType assertInitHashType(const Hash & h) {
+    if (h.type)
+        return *h.type;
+    else
+        abort();
+}
+
 std::string Hash::to_string(Base base, bool includeType) const
 {
     std::string s;
     if (base == Base::SRI || includeType) {
-        s += printHashType(type);
+        s += printHashType(assertInitHashType(*this));
         s += base == Base::SRI ? '-' : ':';
     }
     switch (base) {
@@ -124,8 +134,10 @@ std::string Hash::to_string(Base base, bool includeType) const
     return s;
 }
 
+Hash::Hash(const std::string & s, HashType type) : Hash(s, std::optional { type }) { }
+Hash::Hash(const std::string & s) : Hash(s, std::optional<HashType>{}) { }
 
-Hash::Hash(const std::string & s, HashType type)
+Hash::Hash(const std::string & s, std::optional<HashType> type)
     : type(type)
 {
     size_t pos = 0;
@@ -136,17 +148,17 @@ Hash::Hash(const std::string & s, HashType type)
         sep = s.find('-');
         if (sep != string::npos) {
             isSRI = true;
-        } else if (type == HashType::Unknown)
+        } else if (! type)
             throw BadHash("hash '%s' does not include a type", s);
     }
 
     if (sep != string::npos) {
         string hts = string(s, 0, sep);
         this->type = parseHashType(hts);
-        if (this->type == HashType::Unknown)
+        if (!this->type)
             throw BadHash("unknown hash type '%s'", hts);
-        if (type != HashType::Unknown && type != this->type)
-            throw BadHash("hash '%s' should have type '%s'", s, printHashType(type));
+        if (type && type != this->type)
+            throw BadHash("hash '%s' should have type '%s'", s, printHashType(*type));
         pos = sep + 1;
     }
 
@@ -202,7 +214,7 @@ Hash::Hash(const std::string & s, HashType type)
     }
 
     else
-        throw BadHash("hash '%s' has wrong length for hash type '%s'", s, printHashType(type));
+        throw BadHash("hash '%s' has wrong length for hash type '%s'", s, printHashType(*type));
 }
 
 
@@ -318,24 +330,34 @@ Hash compressHash(const Hash & hash, unsigned int newSize)
 }
 
 
-HashType parseHashType(const string & s)
+std::optional<HashType> parseHashTypeOpt(const string & s)
 {
     if (s == "md5") return HashType::MD5;
     else if (s == "sha1") return HashType::SHA1;
     else if (s == "sha256") return HashType::SHA256;
     else if (s == "sha512") return HashType::SHA512;
-    else return HashType::Unknown;
+    else return std::optional<HashType> {};
 }
 
+HashType parseHashType(const string & s)
+{
+    auto opt_h = parseHashTypeOpt(s);
+    if (opt_h)
+        return *opt_h;
+    else
+        throw UsageError("unknown hash algorithm '%1%'", s);
+}
 
 string printHashType(HashType ht)
 {
-    if (ht == HashType::MD5) return "md5";
-    else if (ht == HashType::SHA1) return "sha1";
-    else if (ht == HashType::SHA256) return "sha256";
-    else if (ht == HashType::SHA512) return "sha512";
-    else abort();
+    string ret;
+    switch (ht) {
+    case HashType::MD5: ret = "md5"; break;
+    case HashType::SHA1: ret = "sha1"; break;
+    case HashType::SHA256: ret = "sha256"; break;
+    case HashType::SHA512: ret = "sha512"; break;
+    }
+    return ret;
 }
 
-
 }
diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh
index da557e585..8dc0b5364 100644
--- a/src/libutil/hash.hh
+++ b/src/libutil/hash.hh
@@ -11,7 +11,6 @@ MakeError(BadHash, Error);
 
 
 enum struct HashType : char {
-    Unknown,
     MD5,
     SHA1,
     SHA256,
@@ -40,7 +39,7 @@ struct Hash
     unsigned int hashSize = 0;
     unsigned char hash[maxHashSize] = {};
 
-    HashType type = HashType::Unknown;
+    std::optional<HashType> type = {};
 
     /* Create an unset hash object. */
     Hash() { };
@@ -51,9 +50,13 @@ struct Hash
     /* Initialize the hash from a string representation, in the format
        "[<type>:]<base16|base32|base64>" or "<type>-<base64>" (a
        Subresource Integrity hash expression). If the 'type' argument
-       is HashType::Unknown, then the hash type must be specified in the
+       is not present, then the hash type must be specified in the
        string. */
-    Hash(const std::string & s, HashType type = HashType::Unknown);
+    Hash(const std::string & s, std::optional<HashType> type);
+    // type must be provided
+    Hash(const std::string & s, HashType type);
+    // hash type must be part of string
+    Hash(const std::string & s);
 
     Hash(const Hash &) = default;
 
@@ -64,7 +67,7 @@ struct Hash
     void init();
 
     /* Check whether a hash is set. */
-    operator bool () const { return type != HashType::Unknown; }
+    operator bool () const { return (bool) type; }
 
     /* Check whether two hash are equal. */
     bool operator == (const Hash & h2) const;
@@ -133,6 +136,8 @@ Hash compressHash(const Hash & hash, unsigned int newSize);
 
 /* Parse a string representing a hash type. */
 HashType parseHashType(const string & s);
+/* Will return nothing on parse error */
+std::optional<HashType> parseHashTypeOpt(const string & s);
 
 /* And the reverse. */
 string printHashType(HashType ht);
diff --git a/src/libutil/tests/hash.cc b/src/libutil/tests/hash.cc
index c513ce4ac..ecc0d4a03 100644
--- a/src/libutil/tests/hash.cc
+++ b/src/libutil/tests/hash.cc
@@ -72,9 +72,4 @@ namespace nix {
                 "7299aeadb6889018501d289e4900f7e4331b99dec4b5433a"
                 "c7d329eeb6dd26545e96e55b874be909");
     }
-
-    TEST(hashString, hashingWithUnknownAlgoExits) {
-        auto s = "unknown";
-        ASSERT_DEATH(hashString(HashType::Unknown, s), "");
-    }
 }
diff --git a/src/nix-prefetch-url/nix-prefetch-url.cc b/src/nix-prefetch-url/nix-prefetch-url.cc
index 29c32b39b..e60c6615d 100644
--- a/src/nix-prefetch-url/nix-prefetch-url.cc
+++ b/src/nix-prefetch-url/nix-prefetch-url.cc
@@ -72,8 +72,6 @@ static int _main(int argc, char * * argv)
             else if (*arg == "--type") {
                 string s = getArg(*arg, arg, end);
                 ht = parseHashType(s);
-                if (ht == HashType::Unknown)
-                    throw UsageError(format("unknown hash type '%1%'") % s);
             }
             else if (*arg == "--print-path")
                 printPath = true;
diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc
index c74847394..e8696277e 100644
--- a/src/nix-store/nix-store.cc
+++ b/src/nix-store/nix-store.cc
@@ -722,7 +722,7 @@ static void opVerifyPath(Strings opFlags, Strings opArgs)
         auto path = store->followLinksToStorePath(i);
         printMsg(Verbosity::Talkative, "checking path '%s'...", store->printStorePath(path));
         auto info = store->queryPathInfo(path);
-        HashSink sink(info->narHash.type);
+        HashSink sink(*info->narHash.type);
         store->narFromPath(path, sink);
         auto current = sink.finish();
         if (current.first != info->narHash) {
diff --git a/src/nix/hash.cc b/src/nix/hash.cc
index 26bbcef1c..a36364f50 100644
--- a/src/nix/hash.cc
+++ b/src/nix/hash.cc
@@ -80,12 +80,12 @@ static RegisterCommand r2("hash-path", [](){ return make_ref<CmdHash>(FileIngest
 struct CmdToBase : Command
 {
     Base base;
-    HashType ht = HashType::Unknown;
+    std::optional<HashType> ht;
     std::vector<std::string> args;
 
     CmdToBase(Base base) : base(base)
     {
-        addFlag(Flag::mkHashTypeFlag("type", &ht));
+        addFlag(Flag::mkHashTypeOptFlag("type", &ht));
         expectArgs("strings", &args);
     }
 
@@ -133,8 +133,6 @@ static int compatNixHash(int argc, char * * argv)
         else if (*arg == "--type") {
             string s = getArg(*arg, arg, end);
             ht = parseHashType(s);
-            if (ht == HashType::Unknown)
-                throw UsageError(format("unknown hash type '%1%'") % s);
         }
         else if (*arg == "--to-base16") op = opTo16;
         else if (*arg == "--to-base32") op = opTo32;
diff --git a/src/nix/verify.cc b/src/nix/verify.cc
index d9b200591..58ffc7e4b 100644
--- a/src/nix/verify.cc
+++ b/src/nix/verify.cc
@@ -88,9 +88,9 @@ struct CmdVerify : StorePathsCommand
 
                     std::unique_ptr<AbstractHashSink> hashSink;
                     if (!info->ca)
-                        hashSink = std::make_unique<HashSink>(info->narHash.type);
+                        hashSink = std::make_unique<HashSink>(*info->narHash.type);
                     else
-                        hashSink = std::make_unique<HashModuloSink>(info->narHash.type, storePathToHash(store->printStorePath(info->path)));
+                        hashSink = std::make_unique<HashModuloSink>(*info->narHash.type, storePathToHash(store->printStorePath(info->path)));
 
                     store->narFromPath(info->path, *hashSink);