From 2393a001315c188bd12bb0560b9599825625d168 Mon Sep 17 00:00:00 2001 From: Billy O'Neal Date: Mon, 13 Mar 2023 12:00:11 -0700 Subject: [PATCH] Add additional logging and defenses to artifacts use. (#947) * Always run "set" on its own line. Resolves https://github.com/microsoft/vcpkg/issues/26172 * Try to add additional logging to downloads. Hopefully helps run down what's happening in https://github.com/microsoft/vcpkg/issues/30051 * Remove the old 7z download hack now that we no longer get it from nuget.org. * Remove debug if test. --- src/vcpkg/base/downloads.cpp | 66 ++++++++++++++++------------- src/vcpkg/base/hash.cpp | 59 ++++++++++++++------------ src/vcpkg/configure-environment.cpp | 6 +++ vcpkg-init/vcpkg-init.ps1 | 19 +++++++-- 4 files changed, 88 insertions(+), 62 deletions(-) diff --git a/src/vcpkg/base/downloads.cpp b/src/vcpkg/base/downloads.cpp index 8eacacbef4..46b750d481 100644 --- a/src/vcpkg/base/downloads.cpp +++ b/src/vcpkg/base/downloads.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -334,18 +335,6 @@ namespace vcpkg { std::string actual_hash = vcpkg::Hash::get_file_hash(fs, downloaded_path, Hash::Algorithm::Sha512).value_or_exit(VCPKG_LINE_INFO); - - // - // This is the NEW hash for 7zip - if (actual_hash == "a9dfaaafd15d98a2ac83682867ec5766720acf6e99d40d1a00d480692752603bf3f3742623f0ea85647a92374df" - "405f331afd6021c5cf36af43ee8db198129c0") - { - // This is the OLD hash for 7zip - actual_hash = "8c75314102e68d2b2347d592f8e3eb05812e1ebb525decbac472231633753f1d4ca31c8e6881a36144a8da26b257" - "1305b3ae3f4e2b85fc4a290aeda63d1a13b8"; - } - // - if (!Strings::case_insensitive_ascii_equals(sha512, actual_hash)) { return msg::format_error(msgDownloadFailedHashMismatch, @@ -366,22 +355,18 @@ namespace vcpkg try_verify_downloaded_file_hash(fs, url, downloaded_path, sha512).value_or_exit(VCPKG_LINE_INFO); } - static bool check_downloaded_file_hash(Filesystem& fs, - const Optional& hash, - StringView sanitized_url, - const Path& download_part_path, - std::vector& errors) + static ExpectedL check_downloaded_file_hash(Filesystem& fs, + const Optional& hash, + StringView sanitized_url, + const Path& download_part_path) { if (auto p = hash.get()) { - auto maybe_success = try_verify_downloaded_file_hash(fs, sanitized_url, download_part_path, *p); - if (!maybe_success.has_value()) - { - errors.push_back(std::move(maybe_success).error()); - return false; - } + return try_verify_downloaded_file_hash(fs, sanitized_url, download_part_path, *p); } - return true; + + Debug::println("Skipping hash check because none was specified."); + return Unit{}; } static void url_heads_inner(View urls, @@ -444,7 +429,11 @@ namespace vcpkg { url_heads_inner({urls.data() + i, batch_size}, headers, &ret, secrets); } - if (i != urls.size()) url_heads_inner({urls.begin() + i, urls.end()}, headers, &ret, secrets); + + if (i != urls.size()) + { + url_heads_inner({urls.begin() + i, urls.end()}, headers, &ret, secrets); + } return ret; } @@ -513,7 +502,11 @@ namespace vcpkg { download_files_inner(fs, {url_pairs.data() + i, batch_size}, headers, &ret); } - if (i != url_pairs.size()) download_files_inner(fs, {url_pairs.begin() + i, url_pairs.end()}, headers, &ret); + + if (i != url_pairs.size()) + { + download_files_inner(fs, {url_pairs.begin() + i, url_pairs.end()}, headers, &ret); + } Checks::msg_check_exit(VCPKG_LINE_INFO, ret.size() == url_pairs.size(), @@ -561,6 +554,7 @@ namespace vcpkg { cmd.string_arg("-H").string_arg(header); } + cmd.string_arg("-w").string_arg("\\n" + guid_marker.to_string() + "%{http_code}"); cmd.string_arg(url); cmd.string_arg("-T").string_arg(file); @@ -733,11 +727,16 @@ namespace vcpkg { if (download_winhttp(fs, download_path_part_path, split_uri, url, secrets, errors, progress_sink)) { - if (check_downloaded_file_hash(fs, sha512, url, download_path_part_path, errors)) + auto maybe_hash_check = check_downloaded_file_hash(fs, sha512, url, download_path_part_path); + if (maybe_hash_check.has_value()) { fs.rename(download_path_part_path, download_path, VCPKG_LINE_INFO); return true; } + else + { + errors.push_back(std::move(maybe_hash_check).error()); + } } return false; } @@ -758,7 +757,7 @@ namespace vcpkg } std::string non_progress_lines; - const auto maybe_exit_code = cmd_execute_and_stream_lines( + auto maybe_exit_code = cmd_execute_and_stream_lines( cmd, [&](StringView line) { const auto maybe_parsed = try_parse_curl_progress_data(line); @@ -782,15 +781,22 @@ namespace vcpkg if (*exit_code != 0) { errors.push_back( - msg::format_error(msgDownloadFailedCurl, msg::url = sanitized_url, msg::exit_code = *exit_code)); + msg::format_error(msgDownloadFailedCurl, msg::url = sanitized_url, msg::exit_code = *exit_code) + .append_raw('\n') + .append_raw(Strings::join("\n", non_progress_lines))); return false; } - if (check_downloaded_file_hash(fs, sha512, sanitized_url, download_path_part_path, errors)) + auto maybe_hash_check = check_downloaded_file_hash(fs, sha512, sanitized_url, download_path_part_path); + if (maybe_hash_check.has_value()) { fs.rename(download_path_part_path, download_path, VCPKG_LINE_INFO); return true; } + else + { + errors.push_back(std::move(maybe_hash_check).error()); + } } else { diff --git a/src/vcpkg/base/hash.cpp b/src/vcpkg/base/hash.cpp index 43c4740ee2..6899ce1d91 100644 --- a/src/vcpkg/base/hash.cpp +++ b/src/vcpkg/base/hash.cpp @@ -1,7 +1,9 @@ #include #include +#include #include #include +#include #include #include #include @@ -516,8 +518,8 @@ namespace vcpkg::Hash #endif } - template - static std::string do_hash(Algorithm algo, const F& f) + template + static ReturnType do_hash(Algorithm algo, const F& f) { #if defined(_WIN32) auto hasher = BCryptHasher(algo); @@ -542,7 +544,7 @@ namespace vcpkg::Hash std::string get_bytes_hash(const void* first, const void* last, Algorithm algo) { - return do_hash(algo, [first, last](Hasher& hasher) { + return do_hash(algo, [first, last](Hasher& hasher) { hasher.add_bytes(first, last); return hasher.get_hash(); }); @@ -557,36 +559,37 @@ namespace vcpkg::Hash ExpectedL get_file_hash(const Filesystem& fs, const Path& path, Algorithm algo) { + Debug::println("Trying to hash ", path); std::error_code ec; auto file = fs.open_for_read(path, ec); - if (!ec) + if (ec) { - auto result = do_hash(algo, [&file, &ec](Hasher& hasher) { - constexpr std::size_t buffer_size = 1024 * 32; - char buffer[buffer_size]; - do - { - const auto this_read = file.read(buffer, 1, buffer_size); - if (this_read != 0) - { - hasher.add_bytes(buffer, buffer + this_read); - } - else if ((ec = file.error())) - { - return std::string(); - } - } while (!file.eof()); - return hasher.get_hash(); - }); + return msg::format(msg::msgErrorMessage) + .append(msgHashFileFailureToRead, msg::path = path) + .append_raw(ec.message()); + } - if (!ec) + return do_hash>(algo, [&](Hasher& hasher) -> ExpectedL { + constexpr std::size_t buffer_size = 1024 * 32; + char buffer[buffer_size]; + do { - return std::move(result); - } - } + const auto this_read = file.read(buffer, 1, buffer_size); + if (this_read != 0) + { + hasher.add_bytes(buffer, buffer + this_read); + } + else if ((ec = file.error())) + { + return msg::format(msg::msgErrorMessage) + .append(msgHashFileFailureToRead, msg::path = path) + .append_raw(ec.message()); + } + } while (!file.eof()); - return msg::format(msg::msgErrorMessage) - .append(msgHashFileFailureToRead, msg::path = path) - .append_raw(ec.message()); + auto result_hash = hasher.get_hash(); + Debug::print(fmt::format("{} has hash {}\n", path, result_hash)); + return result_hash; + }); } } diff --git a/src/vcpkg/configure-environment.cpp b/src/vcpkg/configure-environment.cpp index fb94b3b021..806dd608a6 100644 --- a/src/vcpkg/configure-environment.cpp +++ b/src/vcpkg/configure-environment.cpp @@ -30,6 +30,12 @@ namespace const Path& ce_base_path) { auto& fs = paths.get_filesystem(); + if (!fs.is_regular_file(ce_tarball)) + { + Debug::println("Download succeeded but file isn't present?"); + Checks::msg_exit_with_error(VCPKG_LINE_INFO, msgFailedToProvisionCe); + } + fs.remove_all(ce_base_path, VCPKG_LINE_INFO); fs.create_directories(ce_base_path, VCPKG_LINE_INFO); Path node_root = node_path.parent_path(); diff --git a/vcpkg-init/vcpkg-init.ps1 b/vcpkg-init/vcpkg-init.ps1 index 5d9787e2a9..1665bc4460 100644 --- a/vcpkg-init/vcpkg-init.ps1 +++ b/vcpkg-init/vcpkg-init.ps1 @@ -108,14 +108,25 @@ return IF EXIST $null DEL $null :: Figure out where VCPKG_ROOT is -IF EXIST "%~dp0\.vcpkg-root" SET VCPKG_ROOT=%~dp0 -IF "%VCPKG_ROOT:~-1%"=="\" SET VCPKG_ROOT=%VCPKG_ROOT:~0,-1% -IF "%VCPKG_ROOT%"=="" SET VCPKG_ROOT=%USERPROFILE%\.vcpkg +IF EXIST "%~dp0.vcpkg-root" ( + SET VCPKG_ROOT=%~dp0 +) + +IF "%VCPKG_ROOT:~-1%"=="\" ( + SET VCPKG_ROOT=%VCPKG_ROOT:0,-1% +) + +IF "%VCPKG_ROOT%"=="" ( + SET VCPKG_ROOT=%USERPROFILE%\.vcpkg +) :: Call powershell which may or may not invoke bootstrap if there's a version mismatch SET Z_POWERSHELL_EXE= FOR %%i IN (pwsh.exe powershell.exe) DO ( - IF EXIST "%%~$PATH:i" SET Z_POWERSHELL_EXE=%%~$PATH:i & GOTO :gotpwsh + IF EXIST "%%~$PATH:i" ( + SET Z_POWERSHELL_EXE=%%~$PATH:i + GOTO :gotpwsh + ) ) :gotpwsh