From b61885786db866ee0b129e8e322bd2d6d26a07fa Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 5 Dec 2025 11:21:47 -0500 Subject: [PATCH] Improve wrong format message with `nix hash convert` We have the machinery to make a more informative error, telling the user what format was actually encountered, and not just that it is not the format that was requested. --- src/libutil/hash.cc | 37 +++++++++++++++++++--------- src/libutil/include/nix/util/hash.hh | 6 +++++ src/nix/hash-convert.md | 2 +- src/nix/hash.cc | 12 +++++---- tests/functional/hash-convert.sh | 8 +++--- 5 files changed, 43 insertions(+), 22 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index c614f1f1c..d051225a7 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -182,8 +182,10 @@ Hash Hash::parseSRI(std::string_view original) * * @param resolveAlgo resolves the parsed type (or throws an error when it is not * possible.) + * + * @return the parsed hash and the format it was parsed from */ -static Hash parseAnyHelper(std::string_view rest, auto resolveAlgo) +static std::pair parseAnyHelper(std::string_view rest, auto resolveAlgo) { bool isSRI = false; @@ -203,34 +205,45 @@ static Hash parseAnyHelper(std::string_view rest, auto resolveAlgo) HashAlgorithm algo = resolveAlgo(std::move(optParsedAlgo)); - auto [decode, formatName] = [&]() -> DecodeNamePair { + auto [decode, formatName, format] = [&]() -> std::tuple { if (isSRI) { /* In the SRI case, we always are using Base64. If the length is wrong, get an error later. */ - return {base64::decode, "SRI"}; + return {base64::decode, "SRI", HashFormat::SRI}; } else { /* Otherwise, decide via the length of the hash (for the given algorithm) what base encoding it is. */ - return baseExplicit(baseFromSize(rest, algo)); + auto format = baseFromSize(rest, algo); + auto [decode, formatName] = baseExplicit(format); + return {decode, formatName, format}; } }(); - return parseLowLevel(rest, algo, {decode, formatName}); + return {parseLowLevel(rest, algo, {decode, formatName}), format}; } Hash Hash::parseAnyPrefixed(std::string_view original) { - return parseAnyHelper(original, [&](std::optional optParsedAlgo) { - // Either the string or user must provide the type, if they both do they - // must agree. - if (!optParsedAlgo) - throw BadHash("hash '%s' does not include a type", original); + return parseAnyHelper( + original, + [&](std::optional optParsedAlgo) { + // Either the string or user must provide the type, if they both do they + // must agree. + if (!optParsedAlgo) + throw BadHash("hash '%s' does not include a type", original); - return *optParsedAlgo; - }); + return *optParsedAlgo; + }) + .first; } Hash Hash::parseAny(std::string_view original, std::optional optAlgo) +{ + return parseAnyReturningFormat(original, optAlgo).first; +} + +std::pair +Hash::parseAnyReturningFormat(std::string_view original, std::optional optAlgo) { return parseAnyHelper(original, [&](std::optional optParsedAlgo) { // Either the string or user must provide the type, if they both do they diff --git a/src/libutil/include/nix/util/hash.hh b/src/libutil/include/nix/util/hash.hh index e4f596091..199a9dd49 100644 --- a/src/libutil/include/nix/util/hash.hh +++ b/src/libutil/include/nix/util/hash.hh @@ -79,6 +79,12 @@ struct Hash */ static Hash parseAny(std::string_view s, std::optional optAlgo); + /** + * Like `parseAny`, but also returns the format the hash was parsed from. + */ + static std::pair + parseAnyReturningFormat(std::string_view s, std::optional optAlgo); + /** * Parse a hash from a string representation like the above, except the * type prefix is mandatory is there is no separate argument. diff --git a/src/nix/hash-convert.md b/src/nix/hash-convert.md index dfb215443..dcebda74a 100644 --- a/src/nix/hash-convert.md +++ b/src/nix/hash-convert.md @@ -27,7 +27,7 @@ R""( ```console # nix hash convert --hash-algo sha256 --from nix32 ungWv48Bz+pBQUDeXa4iI7ADYaOWF3qctBD/YfIAFa0= - error: input hash 'ungWv48Bz+pBQUDeXa4iI7ADYaOWF3qctBD/YfIAFa0=' does not have the expected format '--from nix32' + error: input hash 'ungWv48Bz+pBQUDeXa4iI7ADYaOWF3qctBD/YfIAFa0=' has format 'base64', but '--from nix32' was specified # nix hash convert --hash-algo sha256 --from nix32 1b8m03r63zqhnjf7l5wnldhh7c134ap5vpj0850ymkq1iyzicy5s sha256-ungWv48Bz+pBQUDeXa4iI7ADYaOWF3qctBD/YfIAFa0= diff --git a/src/nix/hash.cc b/src/nix/hash.cc index d3c9ccb66..2945c672c 100644 --- a/src/nix/hash.cc +++ b/src/nix/hash.cc @@ -248,11 +248,13 @@ struct CmdHashConvert : Command void run() override { for (const auto & s : hashStrings) { - Hash h = from == HashFormat::SRI ? Hash::parseSRI(s) : Hash::parseAny(s, algo); - if (from && from != HashFormat::SRI - && h.to_string(*from, false) != (from == HashFormat::Base16 ? toLower(s) : s)) { - auto from_as_string = printHashFormat(*from); - throw BadHash("input hash '%s' does not have the expected format for '--from %s'", s, from_as_string); + auto [h, parsedFormat] = Hash::parseAnyReturningFormat(s, algo); + if (from && *from != parsedFormat) { + throw BadHash( + "input hash '%s' has format '%s', but '--from %s' was specified", + s, + printHashFormat(parsedFormat), + printHashFormat(*from)); } logger->cout(h.to_string(to, to == HashFormat::SRI)); } diff --git a/tests/functional/hash-convert.sh b/tests/functional/hash-convert.sh index 9ef4c189d..be76179ca 100755 --- a/tests/functional/hash-convert.sh +++ b/tests/functional/hash-convert.sh @@ -93,10 +93,10 @@ try3() { # Asserting input format fails. # - expectStderr 1 nix hash convert --hash-algo "$1" --from sri "$2" | grepQuiet "is not SRI" - expectStderr 1 nix hash convert --hash-algo "$1" --from nix32 "$2" | grepQuiet "input hash" - expectStderr 1 nix hash convert --hash-algo "$1" --from base16 "$3" | grepQuiet "input hash" - expectStderr 1 nix hash convert --hash-algo "$1" --from nix32 "$4" | grepQuiet "input hash" + expectStderr 1 nix hash convert --hash-algo "$1" --from sri "$2" | grepQuiet "'base16', but '--from sri'" + expectStderr 1 nix hash convert --hash-algo "$1" --from nix32 "$2" | grepQuiet "'base16', but '--from nix32'" + expectStderr 1 nix hash convert --hash-algo "$1" --from base16 "$3" | grepQuiet "'nix32', but '--from base16'" + expectStderr 1 nix hash convert --hash-algo "$1" --from nix32 "$4" | grepQuiet "'base64', but '--from nix32'" # Base-16 hashes can be in uppercase. nix hash convert --hash-algo "$1" --from base16 "$(echo "$2" | tr '[:lower:]' '[:upper:]')"