Merge pull request #11189 from shuffle2/updater-elevation

WinUpdater: Defer modifying any files until Updater.exe

2 files changed, 63 insertions, 32 deletions

diff --git a/Source/Core/UICommon/AutoUpdate.cpp b/Source/Core/UICommon/AutoUpdate.cpp
index 814f66c87b..d54a488c46 100644
--- a/Source/Core/UICommon/AutoUpdate.cpp
+++ b/Source/Core/UICommon/AutoUpdate.cpp
@@ -25,7 +25,7 @@
 #include <unistd.h>
 #endif
 
-#if defined _WIN32 || defined __APPLE__
+#if defined(_WIN32) || defined(__APPLE__)
 #define OS_SUPPORTS_UPDATER
 #endif
 
@@ -34,28 +34,35 @@
 namespace
 {
 bool s_update_triggered = false;
-#ifdef _WIN32
-
-const char UPDATER_FILENAME[] = "Updater.exe";
-const char UPDATER_RELOC_FILENAME[] = "Updater.2.exe";
-
-#elif defined(__APPLE__)
-
-const char UPDATER_FILENAME[] = "Dolphin Updater.app";
-const char UPDATER_RELOC_FILENAME[] = ".Dolphin Updater.2.app";
 
+#ifdef __APPLE__
+const char UPDATER_CONTENT_PATH[] = "/Contents/MacOS/Dolphin Updater";
 #endif
 
 #ifdef OS_SUPPORTS_UPDATER
+
 const char UPDATER_LOG_FILE[] = "Updater.log";
 
+std::string UpdaterPath(bool relocated = false)
+{
+  std::string path(File::GetExeDirectory() + DIR_SEP);
+#ifdef __APPLE__
+  if (relocated)
+    path += ".Dolphin Updater.2.app";
+  else
+    path += "Dolphin Updater.app";
+  return path;
+#else
+  return path + "Updater.exe";
+#endif
+}
+
 std::string MakeUpdaterCommandLine(const std::map<std::string, std::string>& flags)
 {
 #ifdef __APPLE__
-  std::string cmdline = "\"" + File::GetExeDirectory() + DIR_SEP + UPDATER_RELOC_FILENAME +
-                        "/Contents/MacOS/Dolphin Updater\"";
+  std::string cmdline = "\"" + UpdaterPath(true) + UPDATER_CONTENT_PATH + "\"";
 #else
-  std::string cmdline = File::GetExeDirectory() + DIR_SEP + UPDATER_RELOC_FILENAME;
+  std::string cmdline = UpdaterPath();
 #endif
 
   cmdline += " ";
@@ -70,19 +77,16 @@ std::string MakeUpdaterCommandLine(const std::map<std::string, std::string>& fla
   return cmdline;
 }
 
-// Used to remove the relocated updater file once we don't need it anymore.
+#ifdef __APPLE__
 void CleanupFromPreviousUpdate()
 {
-  std::string reloc_updater_path = File::GetExeDirectory() + DIR_SEP + UPDATER_RELOC_FILENAME;
-
-#ifdef __APPLE__
-  File::DeleteDirRecursively(reloc_updater_path);
-#else
-  File::Delete(reloc_updater_path);
-#endif
+  // Remove the relocated updater file.
+  File::DeleteDirRecursively(UpdaterPath(true));
 }
 #endif
 
+#endif
+
 // This ignores i18n because most of the text in there (change descriptions) is only going to be
 // written in english anyway.
 std::string GenerateChangelog(const picojson::array& versions)
@@ -128,7 +132,7 @@ std::string GenerateChangelog(const picojson::array& versions)
 
 bool AutoUpdateChecker::SystemSupportsAutoUpdates()
 {
-#if defined(AUTOUPDATE) && (defined(_WIN32) || defined(__APPLE__))
+#if defined(AUTOUPDATE) && defined(OS_SUPPORTS_UPDATER)
   return true;
 #else
   return false;
@@ -161,7 +165,7 @@ void AutoUpdateChecker::CheckForUpdate(std::string_view update_track,
   if (!SystemSupportsAutoUpdates() || update_track.empty())
     return;
 
-#ifdef OS_SUPPORTS_UPDATER
+#ifdef __APPLE__
   CleanupFromPreviousUpdate();
 #endif
 
@@ -234,15 +238,11 @@ void AutoUpdateChecker::TriggerUpdate(const AutoUpdateChecker::NewVersionInforma
   if (restart_mode == RestartMode::RESTART_AFTER_UPDATE)
     updater_flags["binary-to-restart"] = File::GetExePath();
 
-  // Copy the updater so it can update itself if needed.
-  std::string updater_path = File::GetExeDirectory() + DIR_SEP + UPDATER_FILENAME;
-  std::string reloc_updater_path = File::GetExeDirectory() + DIR_SEP + UPDATER_RELOC_FILENAME;
-
 #ifdef __APPLE__
-  File::CopyDir(updater_path, reloc_updater_path);
-  chmod((reloc_updater_path + "/Contents/MacOS/Dolphin Updater").c_str(), 0700);
-#else
-  File::Copy(updater_path, reloc_updater_path);
+  // Copy the updater so it can update itself if needed.
+  const std::string reloc_updater_path = UpdaterPath(true);
+  File::CopyDir(UpdaterPath(), reloc_updater_path);
+  chmod((reloc_updater_path + UPDATER_CONTENT_PATH).c_str(), 0700);
 #endif
 
   // Run the updater!
@@ -253,7 +253,7 @@ void AutoUpdateChecker::TriggerUpdate(const AutoUpdateChecker::NewVersionInforma
   STARTUPINFO sinfo{.cb = sizeof(sinfo)};
   sinfo.dwFlags = STARTF_FORCEOFFFEEDBACK;  // No hourglass cursor after starting the process.
   PROCESS_INFORMATION pinfo;
-  if (CreateProcessW(UTF8ToWString(reloc_updater_path).c_str(), UTF8ToWString(command_line).data(),
+  if (CreateProcessW(UTF8ToWString(UpdaterPath()).c_str(), UTF8ToWString(command_line).data(),
                      nullptr, nullptr, FALSE, 0, nullptr, nullptr, &sinfo, &pinfo))
   {
     CloseHandle(pinfo.hThread);
diff --git a/Source/Core/UpdaterCommon/UpdaterCommon.cpp b/Source/Core/UpdaterCommon/UpdaterCommon.cpp
index dee55965bf..6728a33d6e 100644
--- a/Source/Core/UpdaterCommon/UpdaterCommon.cpp
+++ b/Source/Core/UpdaterCommon/UpdaterCommon.cpp
@@ -13,6 +13,7 @@
 #include <mbedtls/sha256.h>
 #include <zlib.h>
 
+#include "Common/CommonFuncs.h"
 #include "Common/CommonPaths.h"
 #include "Common/FileUtil.h"
 #include "Common/HttpRequest.h"
@@ -26,6 +27,11 @@
 #include <sys/types.h>
 #endif
 
+#ifdef _WIN32
+#include <Windows.h>
+#include <filesystem>
+#endif
+
 // Refer to docs/autoupdate_overview.md for a detailed overview of the autoupdate process
 
 namespace
@@ -414,6 +420,11 @@ bool DeleteObsoleteFiles(const std::vector<TodoList::DeleteOp>& to_delete,
 bool UpdateFiles(const std::vector<TodoList::UpdateOp>& to_update,
                  const std::string& install_base_path, const std::string& temp_path)
 {
+#ifdef _WIN32
+  const auto self_path = std::filesystem::path(GetModuleName(nullptr).value());
+  const auto self_filename = self_path.filename();
+#endif
+
   for (const auto& op : to_update)
   {
     std::string path = install_base_path + DIR_SEP + op.filename;
@@ -445,6 +456,26 @@ bool UpdateFiles(const std::vector<TodoList::UpdateOp>& to_update,
 
       permission = file_stats.st_mode;
 #endif
+
+#ifdef _WIN32
+      // If incoming file would overwrite the currently executing file, rename ourself to allow the
+      // overwrite to complete. Renaming ourself while executing is fine, but deleting ourself is
+      // rather tricky. The best way to handle that would be to execute the newly-placed Updater.exe
+      // after entire update has completed, and have it delete our relocated executable. For now we
+      // just let the relocated file hang around.
+      // It is enough to match based on filename, don't need File/VolumeId etc.
+      if (op.filename == self_filename)
+      {
+        auto reloc_path = self_path;
+        reloc_path.replace_filename("Updater.2.exe");
+        if (!MoveFile(self_path.wstring().c_str(), reloc_path.wstring().c_str()))
+        {
+          fprintf(log_fp, "Failed to relocate %s.\n", op.filename.c_str());
+          // Just let the Copy fail, later.
+        }
+      }
+#endif
+
       std::string contents;
       if (!File::ReadFileToString(path, contents))
       {