From 0ba9d4ced7a15f9553c14d42184abf5d06122509 Mon Sep 17 00:00:00 2001 From: Matt LaPlante Date: Mon, 23 Aug 2021 12:21:09 -0500 Subject: [PATCH] Refactor file handing for self-update. * Write new file payload to a temp file before touching the original binary. Minimizes the possibility of failing mid-write and corrupting the binary. * On Windows, move the original binary out to a temp file rather than removing it as the running binary is locked. Fixes issue #2248. --- changelog/unreleased/issue-2248 | 8 ++++ internal/selfupdate/download.go | 52 +++++++++++++------------ internal/selfupdate/download_unix.go | 10 +++++ internal/selfupdate/download_windows.go | 23 +++++++++++ 4 files changed, 69 insertions(+), 24 deletions(-) create mode 100644 changelog/unreleased/issue-2248 create mode 100644 internal/selfupdate/download_unix.go create mode 100644 internal/selfupdate/download_windows.go diff --git a/changelog/unreleased/issue-2248 b/changelog/unreleased/issue-2248 new file mode 100644 index 000000000..184e5e17e --- /dev/null +++ b/changelog/unreleased/issue-2248 @@ -0,0 +1,8 @@ +Bugfix: Support self-update on Windows + +Restic self-update would fail in situations where the operating system +locks running binaries, including Windows. The new behavior works around +this by renaming the running file and swapping the updated file in place. + +https://github.com/restic/restic/issues/2248 +https://github.com/restic/restic/pull/3675 \ No newline at end of file diff --git a/internal/selfupdate/download.go b/internal/selfupdate/download.go index 888007c4c..b12afd61c 100644 --- a/internal/selfupdate/download.go +++ b/internal/selfupdate/download.go @@ -10,6 +10,7 @@ import ( "encoding/hex" "fmt" "io" + "io/ioutil" "os" "path/filepath" "runtime" @@ -40,14 +41,6 @@ func findHash(buf []byte, filename string) (hash []byte, err error) { } func extractToFile(buf []byte, filename, target string, printf func(string, ...interface{})) error { - var mode = os.FileMode(0755) - - // get information about the target file - fi, err := os.Lstat(target) - if err == nil { - mode = fi.Mode() - } - var rd io.Reader = bytes.NewReader(buf) switch filepath.Ext(filename) { case ".bz2": @@ -74,33 +67,44 @@ func extractToFile(buf []byte, filename, target string, printf func(string, ...i rd = file } - err = os.Remove(target) - if os.IsNotExist(err) { - err = nil - } - if err != nil { - return fmt.Errorf("unable to remove target file: %v", err) - } - - dest, err := os.OpenFile(target, os.O_CREATE|os.O_EXCL|os.O_WRONLY, mode) + // Write everything to a temp file + dir := filepath.Dir(target) + new, err := ioutil.TempFile(dir, "restic") if err != nil { return err } - n, err := io.Copy(dest, rd) + n, err := io.Copy(new, rd) if err != nil { - _ = dest.Close() - _ = os.Remove(dest.Name()) + _ = new.Close() + _ = os.Remove(new.Name()) + return err + } + if err = new.Sync(); err != nil { + return err + } + if err = new.Close(); err != nil { return err } - err = dest.Close() - if err != nil { + mode := os.FileMode(0755) + // attempt to find the original mode + if fi, err := os.Lstat(target); err == nil { + mode = fi.Mode() + } + + // Remove the original binary. + if err := removeResticBinary(dir, target); err != nil { return err } - printf("saved %d bytes in %v\n", n, dest.Name()) - return nil + // Rename the temp file to the final location atomically. + if err := os.Rename(new.Name(), target); err != nil { + return err + } + + printf("saved %d bytes in %v\n", n, target) + return os.Chmod(target, mode) } // DownloadLatestStableRelease downloads the latest stable released version of diff --git a/internal/selfupdate/download_unix.go b/internal/selfupdate/download_unix.go new file mode 100644 index 000000000..c6189e9d9 --- /dev/null +++ b/internal/selfupdate/download_unix.go @@ -0,0 +1,10 @@ +//go:build !windows +// +build !windows + +package selfupdate + +// Remove the target binary. +func removeResticBinary(dir, target string) error { + // removed on rename on this platform + return nil +} diff --git a/internal/selfupdate/download_windows.go b/internal/selfupdate/download_windows.go new file mode 100644 index 000000000..4f2797385 --- /dev/null +++ b/internal/selfupdate/download_windows.go @@ -0,0 +1,23 @@ +//go:build windows +// +build windows + +package selfupdate + +import ( + "fmt" + "os" + "path/filepath" +) + +// Rename (rather than remove) the running version. The running binary will be locked +// on Windows and cannot be removed while still executing. +func removeResticBinary(dir, target string) error { + backup := filepath.Join(dir, filepath.Base(target)+".bak") + if _, err := os.Stat(backup); err == nil { + _ = os.Remove(backup) + } + if err := os.Rename(target, backup); err != nil { + return fmt.Errorf("unable to rename target file: %v", err) + } + return nil +}