From 23a0d4a1e62085e8a868855b0cf3791efcb23cee Mon Sep 17 00:00:00 2001 From: albertony <12441419+albertony@users.noreply.github.com> Date: Thu, 8 Apr 2021 17:49:47 +0200 Subject: [PATCH] config: fix issues with memory-only config file paths Fixes #5222 --- cmd/version/version_test.go | 6 +-- fs/config/config.go | 54 ++++++++++++++++++----- fs/config/config_test.go | 6 +-- fs/config/configfile/configfile.go | 58 +++++++++++-------------- fs/config/configfile/configfile_test.go | 20 ++++----- fs/config/configflags/configflags.go | 11 +++-- fs/config/crypt_test.go | 30 ++++++------- fs/config/ui.go | 12 +++-- fs/config/ui_test.go | 6 +-- fs/fs.go | 3 +- fstest/fstest.go | 2 +- 11 files changed, 119 insertions(+), 89 deletions(-) diff --git a/cmd/version/version_test.go b/cmd/version/version_test.go index 1231169ff..d48110538 100644 --- a/cmd/version/version_test.go +++ b/cmd/version/version_test.go @@ -26,12 +26,12 @@ func TestVersionWorksWithoutAccessibleConfigFile(t *testing.T) { } // re-wire oldOsStdout := os.Stdout - oldConfigPath := config.ConfigPath - config.ConfigPath = path + oldConfigPath := config.GetConfigPath() + assert.NoError(t, config.SetConfigPath(path)) os.Stdout = nil defer func() { os.Stdout = oldOsStdout - config.ConfigPath = oldConfigPath + assert.NoError(t, config.SetConfigPath(oldConfigPath)) }() cmd.Root.SetArgs([]string{"version"}) diff --git a/fs/config/config.go b/fs/config/config.go index f2f0a7440..fe0e36312 100644 --- a/fs/config/config.go +++ b/fs/config/config.go @@ -28,6 +28,7 @@ import ( const ( configFileName = "rclone.conf" hiddenConfigFileName = "." + configFileName + noConfigFile = "/notfound" // ConfigToken is the key used to store the token under ConfigToken = "token" @@ -107,17 +108,21 @@ var ( // and any parents. CacheDir = makeCacheDir() - // ConfigPath points to the config file - ConfigPath = makeConfigPath() - // Password can be used to configure the random password generator Password = random.Password ) +var ( + configPath string + noConfigPath string +) + func init() { // Set the function pointers up in fs fs.ConfigFileGet = FileGetFlag fs.ConfigFileSet = SetValueAndSave + noConfigPath, _ = filepath.Abs(noConfigFile) + configPath = makeConfigPath() } // Return the path to the configuration file @@ -212,18 +217,44 @@ func makeConfigPath() string { return hiddenConfigFileName } +// GetConfigPath returns the current config file path +func GetConfigPath() string { + return configPath +} + +// SetConfigPath sets new config file path +// +// Checks for empty string, os null device, or special path, all of which indicates in-memory config. +func SetConfigPath(path string) (err error) { + if path == "" || path == os.DevNull { + configPath = "" + } else { + if configPath, err = filepath.Abs(path); err != nil { + return err + } + if configPath == noConfigPath { + configPath = "" + } + } + return nil +} + // LoadConfig loads the config file func LoadConfig(ctx context.Context) { // Set RCLONE_CONFIG_DIR for backend config and subprocesses - _ = os.Setenv("RCLONE_CONFIG_DIR", filepath.Dir(ConfigPath)) - - // Load configuration file. + // If empty configPath (in-memory only) the value will be "." + _ = os.Setenv("RCLONE_CONFIG_DIR", filepath.Dir(configPath)) + // Load configuration from file (or initialize sensible default if no file or error) if err := Data.Load(); err == ErrorConfigFileNotFound { - fs.Logf(nil, "Config file %q not found - using defaults", ConfigPath) + if configPath == "" { + fs.Debugf(nil, "Config is memory-only - using defaults") + } else { + fs.Logf(nil, "Config file %q not found - using defaults", configPath) + } } else if err != nil { - log.Fatalf("Failed to load config file %q: %v", ConfigPath, err) + log.Fatalf("Failed to load config file %q: %v", configPath, err) } else { - fs.Debugf(nil, "Using config file from %q", ConfigPath) + fs.Debugf(nil, "Using config file from %q", configPath) } } @@ -233,6 +264,10 @@ var ErrorConfigFileNotFound = errors.New("config file not found") // SaveConfig calling function which saves configuration file. // if SaveConfig returns error trying again after sleep. func SaveConfig() { + if configPath == "" { + fs.Debugf(nil, "Skipping save for memory-only config") + return + } ctx := context.Background() ci := fs.GetConfig(ctx) var err error @@ -244,7 +279,6 @@ func SaveConfig() { time.Sleep(time.Duration(waitingTimeMs) * time.Millisecond) } fs.Errorf(nil, "Failed to save config after %d tries: %v", ci.LowLevelRetries, err) - return } // SetValueAndSave sets the key to the value and saves just that diff --git a/fs/config/config_test.go b/fs/config/config_test.go index 2e597d9c3..1fff9ef60 100644 --- a/fs/config/config_test.go +++ b/fs/config/config_test.go @@ -12,10 +12,10 @@ import ( ) func TestConfigLoad(t *testing.T) { - oldConfigPath := config.ConfigPath - config.ConfigPath = "./testdata/plain.conf" + oldConfigPath := config.GetConfigPath() + assert.NoError(t, config.SetConfigPath("./testdata/plain.conf")) defer func() { - config.ConfigPath = oldConfigPath + assert.NoError(t, config.SetConfigPath(oldConfigPath)) }() config.ClearConfigPassword() configfile.LoadConfig(context.Background()) diff --git a/fs/config/configfile/configfile.go b/fs/config/configfile/configfile.go index fa2640db9..969921fde 100644 --- a/fs/config/configfile/configfile.go +++ b/fs/config/configfile/configfile.go @@ -15,9 +15,6 @@ import ( "github.com/rclone/rclone/fs/config" ) -// Special value indicating in memory config file. Empty string works also. -const noConfigFile = "/notfound" - // LoadConfig installs the config file handler and calls config.LoadConfig func LoadConfig(ctx context.Context) { config.Data = &Storage{} @@ -32,29 +29,22 @@ type Storage struct { fi os.FileInfo // stat of the file when last loaded } -// Return whether we have a real config file or not -func (s *Storage) noConfig() bool { - return config.ConfigPath == "" || config.ConfigPath == noConfigFile -} - // Check to see if we need to reload the config func (s *Storage) check() { s.mu.Lock() defer s.mu.Unlock() - if s.noConfig() { - return - } - - // Check to see if config file has changed since it was last loaded - fi, err := os.Stat(config.ConfigPath) - if err == nil { - // check to see if config file has changed and if it has, reload it - if s.fi == nil || !fi.ModTime().Equal(s.fi.ModTime()) || fi.Size() != s.fi.Size() { - fs.Debugf(nil, "Config file has changed externaly - reloading") - err := s._load() - if err != nil { - fs.Errorf(nil, "Failed to read config file - using previous config: %v", err) + if configPath := config.GetConfigPath(); configPath != "" { + // Check to see if config file has changed since it was last loaded + fi, err := os.Stat(configPath) + if err == nil { + // check to see if config file has changed and if it has, reload it + if s.fi == nil || !fi.ModTime().Equal(s.fi.ModTime()) || fi.Size() != s.fi.Size() { + fs.Debugf(nil, "Config file has changed externaly - reloading") + err := s._load() + if err != nil { + fs.Errorf(nil, "Failed to read config file - using previous config: %v", err) + } } } } @@ -71,11 +61,12 @@ func (s *Storage) _load() (err error) { } }() - if s.noConfig() { + configPath := config.GetConfigPath() + if configPath == "" { return config.ErrorConfigFileNotFound } - fd, err := os.Open(config.ConfigPath) + fd, err := os.Open(configPath) if err != nil { if os.IsNotExist(err) { return config.ErrorConfigFileNotFound @@ -85,7 +76,7 @@ func (s *Storage) _load() (err error) { defer fs.CheckClose(fd, &err) // Update s.fi with the current file info - s.fi, _ = os.Stat(config.ConfigPath) + s.fi, _ = os.Stat(configPath) cryptReader, err := config.Decrypt(fd) if err != nil { @@ -113,11 +104,12 @@ func (s *Storage) Save() error { s.mu.Lock() defer s.mu.Unlock() - if s.noConfig() { - return nil + configPath := config.GetConfigPath() + if configPath == "" { + return errors.Errorf("Failed to save config file: Path is empty") } - dir, name := filepath.Split(config.ConfigPath) + dir, name := filepath.Split(configPath) err := os.MkdirAll(dir, os.ModePerm) if err != nil { return errors.Wrap(err, "failed to create config directory") @@ -149,7 +141,7 @@ func (s *Storage) Save() error { } var fileMode os.FileMode = 0600 - info, err := os.Stat(config.ConfigPath) + info, err := os.Stat(configPath) if err != nil { fs.Debugf(nil, "Using default permissions for config file: %v", fileMode) } else if info.Mode() != fileMode { @@ -157,25 +149,25 @@ func (s *Storage) Save() error { fileMode = info.Mode() } - attemptCopyGroup(config.ConfigPath, f.Name()) + attemptCopyGroup(configPath, f.Name()) err = os.Chmod(f.Name(), fileMode) if err != nil { fs.Errorf(nil, "Failed to set permissions on config file: %v", err) } - if err = os.Rename(config.ConfigPath, config.ConfigPath+".old"); err != nil && !os.IsNotExist(err) { + if err = os.Rename(configPath, configPath+".old"); err != nil && !os.IsNotExist(err) { return errors.Errorf("Failed to move previous config to backup location: %v", err) } - if err = os.Rename(f.Name(), config.ConfigPath); err != nil { + if err = os.Rename(f.Name(), configPath); err != nil { return errors.Errorf("Failed to move newly written config from %s to final location: %v", f.Name(), err) } - if err := os.Remove(config.ConfigPath + ".old"); err != nil && !os.IsNotExist(err) { + if err := os.Remove(configPath + ".old"); err != nil && !os.IsNotExist(err) { fs.Errorf(nil, "Failed to remove backup config file: %v", err) } // Update s.fi with the newly written file - s.fi, _ = os.Stat(config.ConfigPath) + s.fi, _ = os.Stat(configPath) return nil } diff --git a/fs/config/configfile/configfile_test.go b/fs/config/configfile/configfile_test.go index e1ddc69bc..e35dc4c3c 100644 --- a/fs/config/configfile/configfile_test.go +++ b/fs/config/configfile/configfile_test.go @@ -39,10 +39,10 @@ func setConfigFile(t *testing.T, data string) func() { require.NoError(t, out.Close()) - old := config.ConfigPath - config.ConfigPath = filePath + old := config.GetConfigPath() + assert.NoError(t, config.SetConfigPath(filePath)) return func() { - config.ConfigPath = old + assert.NoError(t, config.SetConfigPath(old)) _ = os.Remove(filePath) } } @@ -160,7 +160,7 @@ type = number3 `, toUnix(buf)) t.Run("Save", func(t *testing.T) { require.NoError(t, data.Save()) - buf, err := ioutil.ReadFile(config.ConfigPath) + buf, err := ioutil.ReadFile(config.GetConfigPath()) require.NoError(t, err) assert.Equal(t, `[one] fruit = potato @@ -188,7 +188,7 @@ func TestConfigFileReload(t *testing.T) { assert.Equal(t, "", value) // Now write a new value on the end - out, err := os.OpenFile(config.ConfigPath, os.O_APPEND|os.O_WRONLY, 0777) + out, err := os.OpenFile(config.GetConfigPath(), os.O_APPEND|os.O_WRONLY, 0777) require.NoError(t, err) fmt.Fprintln(out, "appended = what magic") require.NoError(t, out.Close()) @@ -203,7 +203,7 @@ func TestConfigFileDoesNotExist(t *testing.T) { defer setConfigFile(t, configData)() data := &Storage{} - require.NoError(t, os.Remove(config.ConfigPath)) + require.NoError(t, os.Remove(config.GetConfigPath())) err := data.Load() require.Equal(t, config.ErrorConfigFileNotFound, err) @@ -215,7 +215,7 @@ func TestConfigFileDoesNotExist(t *testing.T) { } func testConfigFileNoConfig(t *testing.T, configPath string) { - config.ConfigPath = configPath + assert.NoError(t, config.SetConfigPath(configPath)) data := &Storage{} err := data.Load() @@ -227,13 +227,13 @@ func testConfigFileNoConfig(t *testing.T, configPath string) { assert.Equal(t, "42", value) err = data.Save() - require.NoError(t, err) + require.Error(t, err) } func TestConfigFileNoConfig(t *testing.T) { - old := config.ConfigPath + old := config.GetConfigPath() defer func() { - config.ConfigPath = old + assert.NoError(t, config.SetConfigPath(old)) }() t.Run("Empty", func(t *testing.T) { diff --git a/fs/config/configflags/configflags.go b/fs/config/configflags/configflags.go index 6ef83c055..c7b0d555a 100644 --- a/fs/config/configflags/configflags.go +++ b/fs/config/configflags/configflags.go @@ -6,7 +6,6 @@ package configflags import ( "log" "net" - "path/filepath" "strconv" "strings" @@ -23,6 +22,7 @@ var ( // these will get interpreted into fs.Config via SetFlags() below verbose int quiet bool + configPath string dumpHeaders bool dumpBodies bool deleteBefore bool @@ -45,7 +45,7 @@ func AddFlags(ci *fs.ConfigInfo, flagSet *pflag.FlagSet) { flags.DurationVarP(flagSet, &ci.ModifyWindow, "modify-window", "", ci.ModifyWindow, "Max time diff to be considered the same") flags.IntVarP(flagSet, &ci.Checkers, "checkers", "", ci.Checkers, "Number of checkers to run in parallel.") flags.IntVarP(flagSet, &ci.Transfers, "transfers", "", ci.Transfers, "Number of file transfers to run in parallel.") - flags.StringVarP(flagSet, &config.ConfigPath, "config", "", config.ConfigPath, "Config file.") + flags.StringVarP(flagSet, &configPath, "config", "", config.GetConfigPath(), "Config file.") flags.StringVarP(flagSet, &config.CacheDir, "cache-dir", "", config.CacheDir, "Directory rclone will use for caching.") flags.BoolVarP(flagSet, &ci.CheckSum, "checksum", "c", ci.CheckSum, "Skip based on checksum (if available) & size, not mod-time & size") flags.BoolVarP(flagSet, &ci.SizeOnly, "size-only", "", ci.SizeOnly, "Skip based on size only, not mod-time or checksum") @@ -267,10 +267,9 @@ func SetFlags(ci *fs.ConfigInfo) { } } - // Make the config file absolute - configPath, err := filepath.Abs(config.ConfigPath) - if err == nil { - config.ConfigPath = configPath + // Set path to configuration file + if err := config.SetConfigPath(configPath); err != nil { + log.Fatalf("--config: Failed to set %q as config path: %v", configPath, err) } // Set whether multi-thread-streams was set diff --git a/fs/config/crypt_test.go b/fs/config/crypt_test.go index 7febbfc6d..52ab16050 100644 --- a/fs/config/crypt_test.go +++ b/fs/config/crypt_test.go @@ -16,10 +16,10 @@ import ( func TestConfigLoadEncrypted(t *testing.T) { var err error - oldConfigPath := config.ConfigPath - config.ConfigPath = "./testdata/encrypted.conf" + oldConfigPath := config.GetConfigPath() + assert.NoError(t, config.SetConfigPath("./testdata/encrypted.conf")) defer func() { - config.ConfigPath = oldConfigPath + assert.NoError(t, config.SetConfigPath(oldConfigPath)) config.ClearConfigPassword() }() @@ -40,13 +40,13 @@ func TestConfigLoadEncrypted(t *testing.T) { func TestConfigLoadEncryptedWithValidPassCommand(t *testing.T) { ctx := context.Background() ci := fs.GetConfig(ctx) - oldConfigPath := config.ConfigPath + oldConfigPath := config.GetConfigPath() oldConfig := *ci - config.ConfigPath = "./testdata/encrypted.conf" + assert.NoError(t, config.SetConfigPath("./testdata/encrypted.conf")) // using ci.PasswordCommand, correct password ci.PasswordCommand = fs.SpaceSepList{"echo", "asdf"} defer func() { - config.ConfigPath = oldConfigPath + assert.NoError(t, config.SetConfigPath(oldConfigPath)) config.ClearConfigPassword() *ci = oldConfig ci.PasswordCommand = nil @@ -69,13 +69,13 @@ func TestConfigLoadEncryptedWithValidPassCommand(t *testing.T) { func TestConfigLoadEncryptedWithInvalidPassCommand(t *testing.T) { ctx := context.Background() ci := fs.GetConfig(ctx) - oldConfigPath := config.ConfigPath + oldConfigPath := config.GetConfigPath() oldConfig := *ci - config.ConfigPath = "./testdata/encrypted.conf" + assert.NoError(t, config.SetConfigPath("./testdata/encrypted.conf")) // using ci.PasswordCommand, incorrect password ci.PasswordCommand = fs.SpaceSepList{"echo", "asdf-blurfl"} defer func() { - config.ConfigPath = oldConfigPath + assert.NoError(t, config.SetConfigPath(oldConfigPath)) config.ClearConfigPassword() *ci = oldConfig ci.PasswordCommand = nil @@ -92,24 +92,24 @@ func TestConfigLoadEncryptedFailures(t *testing.T) { var err error // This file should be too short to be decoded. - oldConfigPath := config.ConfigPath - config.ConfigPath = "./testdata/enc-short.conf" - defer func() { config.ConfigPath = oldConfigPath }() + oldConfigPath := config.GetConfigPath() + assert.NoError(t, config.SetConfigPath("./testdata/enc-short.conf")) + defer func() { assert.NoError(t, config.SetConfigPath(oldConfigPath)) }() err = config.Data.Load() require.Error(t, err) // This file contains invalid base64 characters. - config.ConfigPath = "./testdata/enc-invalid.conf" + assert.NoError(t, config.SetConfigPath("./testdata/enc-invalid.conf")) err = config.Data.Load() require.Error(t, err) // This file contains invalid base64 characters. - config.ConfigPath = "./testdata/enc-too-new.conf" + assert.NoError(t, config.SetConfigPath("./testdata/enc-too-new.conf")) err = config.Data.Load() require.Error(t, err) // This file does not exist. - config.ConfigPath = "./testdata/filenotfound.conf" + assert.NoError(t, config.SetConfigPath("./testdata/filenotfound.conf")) err = config.Data.Load() assert.Equal(t, config.ErrorConfigFileNotFound, err) } diff --git a/fs/config/ui.go b/fs/config/ui.go index bbbd75a08..f6a838995 100644 --- a/fs/config/ui.go +++ b/fs/config/ui.go @@ -535,12 +535,16 @@ func CopyRemote(name string) { // ShowConfigLocation prints the location of the config file in use func ShowConfigLocation() { - if _, err := os.Stat(ConfigPath); os.IsNotExist(err) { - fmt.Println("Configuration file doesn't exist, but rclone will use this path:") + if configPath := GetConfigPath(); configPath == "" { + fmt.Println("Configuration is in memory only") } else { - fmt.Println("Configuration file is stored at:") + if _, err := os.Stat(configPath); os.IsNotExist(err) { + fmt.Println("Configuration file doesn't exist, but rclone will use this path:") + } else { + fmt.Println("Configuration file is stored at:") + } + fmt.Printf("%s\n", configPath) } - fmt.Printf("%s\n", ConfigPath) } // ShowConfig prints the (unencrypted) config options diff --git a/fs/config/ui_test.go b/fs/config/ui_test.go index 7bbaeab91..133eb12cb 100644 --- a/fs/config/ui_test.go +++ b/fs/config/ui_test.go @@ -34,13 +34,13 @@ func testConfigFile(t *testing.T, configFileName string) func() { // temporarily adapt configuration oldOsStdout := os.Stdout - oldConfigPath := config.ConfigPath + oldConfigPath := config.GetConfigPath() oldConfig := *ci oldConfigFile := config.Data oldReadLine := config.ReadLine oldPassword := config.Password os.Stdout = nil - config.ConfigPath = path + assert.NoError(t, config.SetConfigPath(path)) ci = &fs.ConfigInfo{} configfile.LoadConfig(ctx) @@ -69,7 +69,7 @@ func testConfigFile(t *testing.T, configFileName string) func() { assert.NoError(t, err) os.Stdout = oldOsStdout - config.ConfigPath = oldConfigPath + assert.NoError(t, config.SetConfigPath(oldConfigPath)) config.ReadLine = oldReadLine config.Password = oldPassword *ci = oldConfig diff --git a/fs/fs.go b/fs/fs.go index 946a75ee0..a114cf9e2 100644 --- a/fs/fs.go +++ b/fs/fs.go @@ -1317,7 +1317,8 @@ type setConfigFile string // Set a config item into the config file func (section setConfigFile) Set(key, value string) { if strings.HasPrefix(string(section), ":") { - Errorf(nil, "Can't save config %q = %q for on the fly backend %q", key, value, section) + Logf(nil, "Can't save config %q = %q for on the fly backend %q", key, value, section) + return } Debugf(nil, "Saving config %q = %q in section %q of the config file", key, value, section) err := ConfigFileSet(string(section), key, value) diff --git a/fstest/fstest.go b/fstest/fstest.go index f97bc2cae..7ff4f20a4 100644 --- a/fstest/fstest.go +++ b/fstest/fstest.go @@ -69,7 +69,7 @@ func Initialise() { // parse the flags any more so this doesn't happen // automatically if envConfig := os.Getenv("RCLONE_CONFIG"); envConfig != "" { - config.ConfigPath = envConfig + _ = config.SetConfigPath(envConfig) } configfile.LoadConfig(ctx) accounting.Start(ctx)