diff --git a/docs/content/docs.md b/docs/content/docs.md index 7eb415d4e..3b836490c 100644 --- a/docs/content/docs.md +++ b/docs/content/docs.md @@ -952,15 +952,9 @@ To reduce risk of corrupting an existing configuration file, rclone will not write directly to it when saving changes. Instead it will first write to a new, temporary, file. If a configuration file already existed, it will (on Unix systems) try to mirror its permissions to -the new file. Then it will rename the existing file by adding suffix -".old" to its name (e.g. `rclone.conf.old`), if a file with the same -name already exists it will simply be replaced. Next, rclone will rename -the new file to the correct name, before finally cleaning up by deleting -the original file now which has now ".old" suffix to its name. Note that -one side-effect of this is that if you happen to have a file in the same -directory and with the same name as your configuration file, but with -suffix ".old", then rclone will end up deleting this file next time it -updates its configuration file! +the new file. Then it will rename the existing file to a temporary +name as backup. Next, rclone will rename the new file to the correct name, +before finally cleaning up by deleting the backup file. ### --contimeout=TIME ### diff --git a/fs/config/configfile/configfile.go b/fs/config/configfile/configfile.go index b242206a8..30aa8227c 100644 --- a/fs/config/configfile/configfile.go +++ b/fs/config/configfile/configfile.go @@ -106,7 +106,6 @@ func (s *Storage) Save() error { if configPath == "" { return fmt.Errorf("failed to save config file, path is empty") } - dir, name := filepath.Split(configPath) err := file.MkdirAll(dir, os.ModePerm) if err != nil { @@ -119,7 +118,7 @@ func (s *Storage) Save() error { defer func() { _ = f.Close() if err := os.Remove(f.Name()); err != nil && !os.IsNotExist(err) { - fs.Errorf(nil, "failed to remove temp config file: %v", err) + fs.Errorf(nil, "Failed to remove temp file for new config: %v", err) } }() @@ -154,15 +153,33 @@ func (s *Storage) Save() error { fs.Errorf(nil, "Failed to set permissions on config file: %v", err) } - if err = os.Rename(configPath, configPath+".old"); err != nil && !os.IsNotExist(err) { - return fmt.Errorf("failed to move previous config to backup location: %w", err) + fbackup, err := os.CreateTemp(dir, name+".old") + if err != nil { + return fmt.Errorf("failed to create temp file for old config backup: %w", err) + } + err = fbackup.Close() + if err != nil { + return fmt.Errorf("failed to close temp file for old config backup: %w", err) + } + keepBackup := true + defer func() { + if !keepBackup { + if err := os.Remove(fbackup.Name()); err != nil && !os.IsNotExist(err) { + fs.Errorf(nil, "Failed to remove temp file for old config backup: %v", err) + } + } + }() + + if err = os.Rename(configPath, fbackup.Name()); err != nil { + if !os.IsNotExist(err) { + return fmt.Errorf("failed to move previous config to backup location: %w", err) + } + keepBackup = false // no existing file, no need to keep backup even if writing of new file fails } if err = os.Rename(f.Name(), configPath); err != nil { return fmt.Errorf("failed to move newly written config from %s to final location: %v", f.Name(), err) } - if err := os.Remove(configPath + ".old"); err != nil && !os.IsNotExist(err) { - fs.Errorf(nil, "Failed to remove backup config file: %v", err) - } + keepBackup = false // new file was written, no need to keep backup // Update s.fi with the newly written file s.fi, _ = os.Stat(configPath)