diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index a1ed2cf5a46..e889bcc7720 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -106,7 +106,11 @@ DownloadFileResult downloadFile( } static DownloadTarballResult downloadTarball_( - const Settings & settings, const std::string & urlS, const Headers & headers, const std::string & displayPrefix) + const Settings & settings, + const std::string & urlS, + const Headers & headers, + const std::string & displayPrefix, + const std::optional & expectedNarHash) { ParsedURL url = parseURL(urlS); @@ -151,10 +155,23 @@ static DownloadTarballResult downloadTarball_( if (cached && !settings.getTarballCache()->hasObject(getRevAttr(cached->value, "treeHash"))) cached.reset(); - if (cached && !cached->expired) - /* We previously downloaded this tarball and it's younger than - `tarballTtl`, so no need to check the server. */ - return attrsToResult(cached->value); + if (cached) { + if (!cached->expired) + /* We previously downloaded this tarball and it's younger than + `tarballTtl`, so no need to check the server. */ + return attrsToResult(cached->value); + + /* The cached entry is stale, but if its content still matches + the pinned NAR hash, there's no need to re-check the server + either. Exception: a TTL of 0 (e.g. `--refresh`) means the + caller explicitly wants us to verify against the server, so + don't let a merely-matching old hash short-circuit that. */ + if (expectedNarHash && settings.tarballTtl.get() != 0) { + auto treeHash = getRevAttr(cached->value, "treeHash"); + if (settings.getTarballCache()->treeHashToNarHash(settings, treeHash) == *expectedNarHash) + return attrsToResult(cached->value); + } + } auto _res = std::make_shared>(); @@ -493,7 +510,8 @@ struct TarballInputScheme : CurlInputScheme { auto input(_input); - auto result = downloadTarball_(settings, getStrAttr(input.attrs, "url"), {}, "«" + input.to_string() + "»"); + auto result = downloadTarball_( + settings, getStrAttr(input.attrs, "url"), {}, "«" + input.to_string() + "»", input.getNarHash()); if (result.immutableUrl) { auto immutableInput = Input::fromURL(*result.immutableUrl); diff --git a/tests/functional/tarball.sh b/tests/functional/tarball.sh index cfb6c674665..bc48b2835e5 100755 --- a/tests/functional/tarball.sh +++ b/tests/functional/tarball.sh @@ -31,6 +31,33 @@ test_tarball() { nix-build -o "$TEST_ROOT"/result -E "import (fetchTree { type = \"tarball\"; url = \"file://$tarball\"; })" nix-build -o "$TEST_ROOT"/result -E "import (fetchTree { type = \"tarball\"; url = \"file://$tarball\"; narHash = \"$hash\"; })" + # Once a URL has been fetched with a given narHash, re-fetching the same + # URL after its cache entry has genuinely expired (a nonzero TTL that has + # elapsed) should not require re-reading it. Prove this by corrupting the + # file in place and expecting the fetch to still succeed. (`--tarball-ttl + # 0` is excluded here since that's the explicit "always re-verify" + # signal used by `--refresh`, which must NOT be short-circuited.) + cp "$tarball" "$tarball.bak" + echo garbage > "$tarball" + sleep 2 + nix-build -o "$TEST_ROOT"/result --tarball-ttl 1 -E "import (fetchTree { type = \"tarball\"; url = \"file://$tarball\"; narHash = \"$hash\"; })" + mv "$tarball.bak" "$tarball" + + # `--refresh` (tarball-ttl 0) must always re-verify against the source, + # even if a narHash is pinned, so a mismatch must still be reported. + # Use a different URL with different (but validly archived) content, so + # the failure is a hash mismatch rather than an unpack error, and so the + # cache entry for `$tarball` itself isn't disturbed by the expected + # failure. + otherRoot=$TEST_ROOT/tarball-other + rm -rf "$otherRoot" + mkdir -p "$otherRoot" + echo "different content" > "$otherRoot/default.nix" + cp "${config_nix}" dependencies.builder*.sh "$otherRoot/" + otherTarball=$TEST_ROOT/tarball-other.tar$ext + (cd "$TEST_ROOT" && GNUTAR_REPRODUCIBLE=1 tar --mtime="$otherRoot"/default.nix --owner=0 --group=0 --numeric-owner --sort=name -c -f - tarball-other) | $compressor > "$otherTarball" + expectStderr 102 nix eval --refresh --raw --expr "(fetchTree { type = \"tarball\"; url = \"file://$otherTarball\"; narHash = \"$hash\"; }).outPath" | grepQuiet "NAR hash mismatch" + [[ $(nix eval --impure --expr "(fetchTree \"file://$tarball\").lastModified") = 1000000000 ]] nix-instantiate --strict --eval -E "!((import (fetchTree { type = \"tarball\"; url = \"file://$tarball\"; narHash = \"$hash\"; })) ? submodules)" >&2