From 55d4bd6e0ea23dcf55dc33a964514230ee785bbe Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Mon, 22 Jun 2020 18:08:27 +0000
Subject: [PATCH] Improve content address parsing

 - Ensure hash is in form <algo>-<prefix> and not SRI.

 - Better errors if something goes wrong

 - string_view for no coppying
---
 src/libstore/content-address.cc | 77 ++++++++++++++++++++-------------
 src/libutil/hash.cc             |  4 +-
 src/libutil/hash.hh             |  4 +-
 3 files changed, 50 insertions(+), 35 deletions(-)

diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc
index 3d753836f..216d7eb03 100644
--- a/src/libstore/content-address.cc
+++ b/src/libstore/content-address.cc
@@ -1,3 +1,4 @@
+#include "args.hh"
 #include "content-address.hh"
 
 namespace nix {
@@ -40,38 +41,52 @@ std::string renderContentAddress(ContentAddress ca) {
 }
 
 ContentAddress parseContentAddress(std::string_view rawCa) {
-    auto prefixSeparator = rawCa.find(':');
-    if (prefixSeparator != string::npos) {
-        auto prefix = string(rawCa, 0, prefixSeparator);
-        if (prefix == "text") {
-            auto hashTypeAndHash = rawCa.substr(prefixSeparator+1, string::npos);
-            Hash hash = Hash(string(hashTypeAndHash));
-            if (*hash.type != htSHA256) {
-                throw Error("parseContentAddress: the text hash should have type SHA256");
-            }
-            return TextHash { hash };
-        } else if (prefix == "fixed") {
-            // This has to be an inverse of makeFixedOutputCA
-            auto methodAndHash = rawCa.substr(prefixSeparator+1, string::npos);
-            if (methodAndHash.substr(0,2) == "r:") {
-                std::string_view hashRaw = methodAndHash.substr(2,string::npos);
-                return FixedOutputHash {
-                    .method = FileIngestionMethod::Recursive,
-                    .hash = Hash(string(hashRaw)),
-                };
-            } else {
-                std::string_view hashRaw = methodAndHash;
-                return FixedOutputHash {
-                    .method = FileIngestionMethod::Flat,
-                    .hash = Hash(string(hashRaw)),
-                };
-            }
-        } else {
-            throw Error("parseContentAddress: format not recognized; has to be text or fixed");
+    auto rest = rawCa;
+
+    // Ensure prefix
+    const auto prefixSeparator = rawCa.find(':');
+    if (prefixSeparator == string::npos)
+        throw UsageError("not a content address because it is not in the form \"<prefix>:<rest>\": %s", rawCa);
+    auto prefix = rest.substr(0, prefixSeparator);
+    rest = rest.substr(prefixSeparator + 1);
+
+    auto parseHashType_ = [&](){
+        // Parse hash type
+        auto algoSeparator = rest.find(':');
+        HashType hashType;
+        if (algoSeparator == string::npos)
+            throw UsageError("content address hash must be in form \"<algo>:<hash>\", but found: %s", rest);
+        hashType = parseHashType(rest.substr(0, algoSeparator));
+
+        rest = rest.substr(algoSeparator + 1);
+
+        return std::move(hashType);
+    };
+
+    // Switch on prefix
+    if (prefix == "text") {
+        // No parsing of the method, "text" only support flat.
+        HashType hashType = parseHashType_();
+        if (hashType != htSHA256)
+            throw Error("text content address hash should use %s, but instead uses %s",
+                printHashType(htSHA256), printHashType(hashType));
+        return TextHash {
+            .hash = Hash { rest, std::move(hashType) },
+        };
+    } else if (prefix == "fixed") {
+        // Parse method
+        auto method = FileIngestionMethod::Flat;
+        if (rest.substr(0, 2) == "r:") {
+            method = FileIngestionMethod::Recursive;
+            rest = rest.substr(2);
         }
-    } else {
-        throw Error("Not a content address because it lacks an appropriate prefix");
-    }
+        HashType hashType = parseHashType_();
+        return FixedOutputHash {
+            .method = method,
+            .hash = Hash { rest, std::move(hashType) },
+        };
+    } else
+        throw UsageError("content address prefix \"%s\" is unrecognized. Recogonized prefixes are \"text\" or \"fixed\"", prefix);
 };
 
 std::optional<ContentAddress> parseContentAddressOpt(std::string_view rawCaOpt) {
diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc
index c8fcdfed0..012fa727d 100644
--- a/src/libutil/hash.cc
+++ b/src/libutil/hash.cc
@@ -342,7 +342,7 @@ Hash compressHash(const Hash & hash, unsigned int newSize)
 }
 
 
-std::optional<HashType> parseHashTypeOpt(const string & s)
+std::optional<HashType> parseHashTypeOpt(std::string_view s)
 {
     if (s == "md5") return htMD5;
     else if (s == "sha1") return htSHA1;
@@ -351,7 +351,7 @@ std::optional<HashType> parseHashTypeOpt(const string & s)
     else return std::optional<HashType> {};
 }
 
-HashType parseHashType(const string & s)
+HashType parseHashType(std::string_view s)
 {
     auto opt_h = parseHashTypeOpt(s);
     if (opt_h)
diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh
index 0d9916508..e4abe72ce 100644
--- a/src/libutil/hash.hh
+++ b/src/libutil/hash.hh
@@ -121,9 +121,9 @@ HashResult hashPath(HashType ht, const Path & path,
 Hash compressHash(const Hash & hash, unsigned int newSize);
 
 /* Parse a string representing a hash type. */
-HashType parseHashType(const string & s);
+HashType parseHashType(std::string_view s);
 /* Will return nothing on parse error */
-std::optional<HashType> parseHashTypeOpt(const string & s);
+std::optional<HashType> parseHashTypeOpt(std::string_view s);
 
 /* And the reverse. */
 string printHashType(HashType ht);