Add additional logging and defenses to artifacts use. (#947)

BillyONeal · web-flow · commit 2393a001315c · 2023-03-13T12:00:11.000-07:00
* Always run "set" on its own line. Resolves microsoft/vcpkg#26172 * Try to add additional logging to downloads. Hopefully helps run down what's happening in microsoft/vcpkg#30051 * Remove the old 7z download hack now that we no longer get it from nuget.org. * Remove debug if test.
diff --git a/src/vcpkg/base/downloads.cpp b/src/vcpkg/base/downloads.cpp
@@ -5,6 +5,7 @@
 #include <vcpkg/base/json.h>
 #include <vcpkg/base/message_sinks.h>
 #include <vcpkg/base/parse.h>
+#include <vcpkg/base/strings.h>
 #include <vcpkg/base/system.debug.h>
 #include <vcpkg/base/system.h>
 #include <vcpkg/base/system.process.h>
@@ -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);
-
-        // <HACK to handle NuGet.org changing nupkg hashes.>
-        // This is the NEW hash for 7zip
-        if (actual_hash == "a9dfaaafd15d98a2ac83682867ec5766720acf6e99d40d1a00d480692752603bf3f3742623f0ea85647a92374df"
-                           "405f331afd6021c5cf36af43ee8db198129c0")
-        {
-            // This is the OLD hash for 7zip
-            actual_hash = "8c75314102e68d2b2347d592f8e3eb05812e1ebb525decbac472231633753f1d4ca31c8e6881a36144a8da26b257"
-                          "1305b3ae3f4e2b85fc4a290aeda63d1a13b8";
-        }
-        // </HACK>
-
         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<std::string>& hash,
-                                           StringView sanitized_url,
-                                           const Path& download_part_path,
-                                           std::vector<LocalizedString>& errors)
+    static ExpectedL<Unit> check_downloaded_file_hash(Filesystem& fs,
+                                                      const Optional<std::string>& 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<std::string> 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
@@ -1,7 +1,9 @@
 #include <vcpkg/base/checks.h>
 #include <vcpkg/base/expected.h>
+#include <vcpkg/base/format.h>
 #include <vcpkg/base/hash.h>
 #include <vcpkg/base/strings.h>
+#include <vcpkg/base/system.debug.h>
 #include <vcpkg/base/system.process.h>
 #include <vcpkg/base/uint128.h>
 #include <vcpkg/base/util.h>
@@ -516,8 +518,8 @@ namespace vcpkg::Hash
 #endif
     }
 
-    template<class F>
-    static std::string do_hash(Algorithm algo, const F& f)
+    template<class ReturnType, class F>
+    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<std::string>(algo, [first, last](Hasher& hasher) {
             hasher.add_bytes(first, last);
             return hasher.get_hash();
         });
@@ -557,36 +559,37 @@ namespace vcpkg::Hash
 
     ExpectedL<std::string> 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<ExpectedL<std::string>>(algo, [&](Hasher& hasher) -> ExpectedL<std::string> {
+            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
@@ -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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,7 @@`
`5`	`5`	`#include <vcpkg/base/json.h>`
`6`	`6`	`#include <vcpkg/base/message_sinks.h>`
`7`	`7`	`#include <vcpkg/base/parse.h>`
	`8`	`+#include <vcpkg/base/strings.h>`
`8`	`9`	`#include <vcpkg/base/system.debug.h>`
`9`	`10`	`#include <vcpkg/base/system.h>`
`10`	`11`	`#include <vcpkg/base/system.process.h>`
`@@ -334,18 +335,6 @@ namespace vcpkg`
`334`	`335`	`{`
`335`	`336`	`std::string actual_hash =`
`336`	`337`	`vcpkg::Hash::get_file_hash(fs, downloaded_path, Hash::Algorithm::Sha512).value_or_exit(VCPKG_LINE_INFO);`
`337`		`-`
`338`		`- // <HACK to handle NuGet.org changing nupkg hashes.>`
`339`		`- // This is the NEW hash for 7zip`
`340`		`- if (actual_hash == "a9dfaaafd15d98a2ac83682867ec5766720acf6e99d40d1a00d480692752603bf3f3742623f0ea85647a92374df"`
`341`		`- "405f331afd6021c5cf36af43ee8db198129c0")`
`342`		`- {`
`343`		`- // This is the OLD hash for 7zip`
`344`		`- actual_hash = "8c75314102e68d2b2347d592f8e3eb05812e1ebb525decbac472231633753f1d4ca31c8e6881a36144a8da26b257"`
`345`		`- "1305b3ae3f4e2b85fc4a290aeda63d1a13b8";`
`346`		`- }`
`347`		`- // </HACK>`
`348`		`-`
`349`	`338`	`if (!Strings::case_insensitive_ascii_equals(sha512, actual_hash))`
`350`	`339`	`{`
`351`	`340`	`return msg::format_error(msgDownloadFailedHashMismatch,`
`@@ -366,22 +355,18 @@ namespace vcpkg`
`366`	`355`	`try_verify_downloaded_file_hash(fs, url, downloaded_path, sha512).value_or_exit(VCPKG_LINE_INFO);`
`367`	`356`	`}`
`368`	`357`
`369`		`- static bool check_downloaded_file_hash(Filesystem& fs,`
`370`		`- const Optional<std::string>& hash,`
`371`		`- StringView sanitized_url,`
`372`		`- const Path& download_part_path,`
`373`		`- std::vector<LocalizedString>& errors)`
	`358`	`+ static ExpectedL<Unit> check_downloaded_file_hash(Filesystem& fs,`
	`359`	`+ const Optional<std::string>& hash,`
	`360`	`+ StringView sanitized_url,`
	`361`	`+ const Path& download_part_path)`
`374`	`362`	`{`
`375`	`363`	`if (auto p = hash.get())`
`376`	`364`	`{`
`377`		`- auto maybe_success = try_verify_downloaded_file_hash(fs, sanitized_url, download_part_path, *p);`
`378`		`- if (!maybe_success.has_value())`
`379`		`- {`
`380`		`- errors.push_back(std::move(maybe_success).error());`
`381`		`- return false;`
`382`		`- }`
	`365`	`+ return try_verify_downloaded_file_hash(fs, sanitized_url, download_part_path, *p);`
`383`	`366`	`}`
`384`		`- return true;`
	`367`	`+`
	`368`	`+ Debug::println("Skipping hash check because none was specified.");`
	`369`	`+ return Unit{};`
`385`	`370`	`}`
`386`	`371`
`387`	`372`	`static void url_heads_inner(View<std::string> urls,`
`@@ -444,7 +429,11 @@ namespace vcpkg`
`444`	`429`	`{`
`445`	`430`	`url_heads_inner({urls.data() + i, batch_size}, headers, &ret, secrets);`
`446`	`431`	`}`
`447`		`- if (i != urls.size()) url_heads_inner({urls.begin() + i, urls.end()}, headers, &ret, secrets);`
	`432`	`+`
	`433`	`+ if (i != urls.size())`
	`434`	`+ {`
	`435`	`+ url_heads_inner({urls.begin() + i, urls.end()}, headers, &ret, secrets);`
	`436`	`+ }`
`448`	`437`
`449`	`438`	`return ret;`
`450`	`439`	`}`
`@@ -513,7 +502,11 @@ namespace vcpkg`
`513`	`502`	`{`
`514`	`503`	`download_files_inner(fs, {url_pairs.data() + i, batch_size}, headers, &ret);`
`515`	`504`	`}`
`516`		`- if (i != url_pairs.size()) download_files_inner(fs, {url_pairs.begin() + i, url_pairs.end()}, headers, &ret);`
	`505`	`+`
	`506`	`+ if (i != url_pairs.size())`
	`507`	`+ {`
	`508`	`+ download_files_inner(fs, {url_pairs.begin() + i, url_pairs.end()}, headers, &ret);`
	`509`	`+ }`
`517`	`510`
`518`	`511`	`Checks::msg_check_exit(VCPKG_LINE_INFO,`
`519`	`512`	`ret.size() == url_pairs.size(),`
`@@ -561,6 +554,7 @@ namespace vcpkg`
`561`	`554`	`{`
`562`	`555`	`cmd.string_arg("-H").string_arg(header);`
`563`	`556`	`}`
	`557`	`+`
`564`	`558`	`cmd.string_arg("-w").string_arg("\\n" + guid_marker.to_string() + "%{http_code}");`
`565`	`559`	`cmd.string_arg(url);`
`566`	`560`	`cmd.string_arg("-T").string_arg(file);`
`@@ -733,11 +727,16 @@ namespace vcpkg`
`733`	`727`	`{`
`734`	`728`	`if (download_winhttp(fs, download_path_part_path, split_uri, url, secrets, errors, progress_sink))`
`735`	`729`	`{`
`736`		`- if (check_downloaded_file_hash(fs, sha512, url, download_path_part_path, errors))`
	`730`	`+ auto maybe_hash_check = check_downloaded_file_hash(fs, sha512, url, download_path_part_path);`
	`731`	`+ if (maybe_hash_check.has_value())`
`737`	`732`	`{`
`738`	`733`	`fs.rename(download_path_part_path, download_path, VCPKG_LINE_INFO);`
`739`	`734`	`return true;`
`740`	`735`	`}`
	`736`	`+ else`
	`737`	`+ {`
	`738`	`+ errors.push_back(std::move(maybe_hash_check).error());`
	`739`	`+ }`
`741`	`740`	`}`
`742`	`741`	`return false;`
`743`	`742`	`}`
`@@ -758,7 +757,7 @@ namespace vcpkg`
`758`	`757`	`}`
`759`	`758`
`760`	`759`	`std::string non_progress_lines;`
`761`		`- const auto maybe_exit_code = cmd_execute_and_stream_lines(`
	`760`	`+ auto maybe_exit_code = cmd_execute_and_stream_lines(`
`762`	`761`	`cmd,`
`763`	`762`	`[&](StringView line) {`
`764`	`763`	`const auto maybe_parsed = try_parse_curl_progress_data(line);`
`@@ -782,15 +781,22 @@ namespace vcpkg`
`782`	`781`	`if (*exit_code != 0)`
`783`	`782`	`{`
`784`	`783`	`errors.push_back(`
`785`		`- msg::format_error(msgDownloadFailedCurl, msg::url = sanitized_url, msg::exit_code = *exit_code));`
	`784`	`+ msg::format_error(msgDownloadFailedCurl, msg::url = sanitized_url, msg::exit_code = *exit_code)`
	`785`	`+ .append_raw('\n')`
	`786`	`+ .append_raw(Strings::join("\n", non_progress_lines)));`
`786`	`787`	`return false;`
`787`	`788`	`}`
`788`	`789`
`789`		`- if (check_downloaded_file_hash(fs, sha512, sanitized_url, download_path_part_path, errors))`
	`790`	`+ auto maybe_hash_check = check_downloaded_file_hash(fs, sha512, sanitized_url, download_path_part_path);`
	`791`	`+ if (maybe_hash_check.has_value())`
`790`	`792`	`{`
`791`	`793`	`fs.rename(download_path_part_path, download_path, VCPKG_LINE_INFO);`
`792`	`794`	`return true;`
`793`	`795`	`}`
	`796`	`+ else`
	`797`	`+ {`
	`798`	`+ errors.push_back(std::move(maybe_hash_check).error());`
	`799`	`+ }`
`794`	`800`	`}`
`795`	`801`	`else`
`796`	`802`	`{`