TUN-7335: Fix cloudflared update not working in windows

This PR fixes some long standing bugs in the windows update
paths. We previously did not surface the errors at all leading to
this function failing silently.

This PR:

1. Now returns the ExitError if the bat run for update fails.
2. Fixes the errors surfaced by that return:
    a. The batch file doesnt play well with spaces. This is fixed by
    using PROGRA~1/2 which are aliases windows uses.
    b. The existing script also seemed to be irregular about where batch
    files were put and looked for. This is also fixed in this script.
This commit is contained in:
Sudarsan Reddy 2023-04-06 15:32:41 +01:00
parent 8d87d4facd
commit 5dbf76a7aa
2 changed files with 32 additions and 8 deletions

View File

@ -6,6 +6,7 @@ import (
"os" "os"
"path/filepath" "path/filepath"
"runtime" "runtime"
"strings"
"time" "time"
"github.com/facebookgo/grace/gracenet" "github.com/facebookgo/grace/gracenet"
@ -95,11 +96,24 @@ func CheckForUpdate(options updateOptions) (CheckResult, error) {
url = StagingUpdateURL url = StagingUpdateURL
} }
if runtime.GOOS == "windows" {
cfdPath = encodeWindowsPath(cfdPath)
}
s := NewWorkersService(version, url, cfdPath, Options{IsBeta: options.isBeta, s := NewWorkersService(version, url, cfdPath, Options{IsBeta: options.isBeta,
IsForced: options.isForced, RequestedVersion: options.intendedVersion}) IsForced: options.isForced, RequestedVersion: options.intendedVersion})
return s.Check() return s.Check()
} }
func encodeWindowsPath(path string) string {
// We do this because Windows allows spaces in directories such as
// Program Files but does not allow these directories to be spaced in batch files.
targetPath := strings.Replace(path, "Program Files (x86)", "PROGRA~2", -1)
// This is to do the same in 32 bit systems. We do this second so that the first
// replace is for x86 dirs.
targetPath = strings.Replace(targetPath, "Program Files", "PROGRA~1", -1)
return targetPath
}
func applyUpdate(options updateOptions, update CheckResult) UpdateOutcome { func applyUpdate(options updateOptions, update CheckResult) UpdateOutcome {
if update.Version() == "" || options.updateDisabled { if update.Version() == "" || options.updateDisabled {

View File

@ -25,14 +25,13 @@ const (
// rename cloudflared.exe.new to cloudflared.exe // rename cloudflared.exe.new to cloudflared.exe
// delete cloudflared.exe.old // delete cloudflared.exe.old
// start the service // start the service
// delete the batch file // exit with code 0 if we've reached this point indicating success.
windowsUpdateCommandTemplate = `@echo off windowsUpdateCommandTemplate = `sc stop cloudflared >nul 2>&1
sc stop cloudflared >nul 2>&1
rename "{{.TargetPath}}" {{.OldName}} rename "{{.TargetPath}}" {{.OldName}}
rename "{{.NewPath}}" {{.BinaryName}} rename "{{.NewPath}}" {{.BinaryName}}
del "{{.OldPath}}" del "{{.OldPath}}"
sc start cloudflared >nul 2>&1 sc start cloudflared >nul 2>&1
del {{.BatchName}}` exit /b 0`
batchFileName = "cfd_update.bat" batchFileName = "cfd_update.bat"
) )
@ -214,8 +213,9 @@ func isValidChecksum(checksum, filePath string) error {
// writeBatchFile writes a batch file out to disk // writeBatchFile writes a batch file out to disk
// see the dicussion on why it has to be done this way // see the dicussion on why it has to be done this way
func writeBatchFile(targetPath string, newPath string, oldPath string) error { func writeBatchFile(targetPath string, newPath string, oldPath string) error {
os.Remove(batchFileName) //remove any failed updates before download batchFilePath := filepath.Join(filepath.Dir(targetPath), batchFileName)
f, err := os.Create(batchFileName) os.Remove(batchFilePath) //remove any failed updates before download
f, err := os.Create(batchFilePath)
if err != nil { if err != nil {
return err return err
} }
@ -241,6 +241,16 @@ func writeBatchFile(targetPath string, newPath string, oldPath string) error {
// run each OS command for windows // run each OS command for windows
func runWindowsBatch(batchFile string) error { func runWindowsBatch(batchFile string) error {
cmd := exec.Command("cmd", "/c", batchFile) defer os.Remove(batchFile)
return cmd.Start() cmd := exec.Command("cmd", "/C", batchFile)
_, err := cmd.Output()
// Remove the batch file we created. Don't let this interfere with the error
// we report.
if err != nil {
if exitError, ok := err.(*exec.ExitError); ok {
return fmt.Errorf("Error during update : %s;", string(exitError.Stderr))
}
}
return err
} }