From def4b8cee5d4766e3138ecfe54e9b6bcb90aaab4 Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Sun, 14 Jul 2019 11:03:14 +0200 Subject: [PATCH] lib/config: Error on empty folder path (fixes #5853) (#5854) --- lib/api/api_test.go | 6 +++++- lib/config/config.go | 16 ++++++++++++++-- lib/config/config_test.go | 15 ++++++--------- lib/config/folderconfiguration.go | 6 ++---- lib/config/testdata/dupdevices.xml | 2 +- lib/config/testdata/dupfolders.xml | 4 ++-- lib/config/testdata/ignoredfolders.xml | 2 +- lib/config/testdata/pullorder.xml | 16 ++++++++-------- 8 files changed, 39 insertions(+), 28 deletions(-) diff --git a/lib/api/api_test.go b/lib/api/api_test.go index 61c978969..6ac1ada2c 100644 --- a/lib/api/api_test.go +++ b/lib/api/api_test.go @@ -666,7 +666,10 @@ func TestConfigPostOK(t *testing.T) { cfg := bytes.NewBuffer([]byte(`{ "version": 15, "folders": [ - {"id": "foo"} + { + "id": "foo", + "path": "TestConfigPostOK" + } ] }`)) @@ -677,6 +680,7 @@ func TestConfigPostOK(t *testing.T) { if resp.StatusCode != http.StatusOK { t.Error("Expected 200 OK, not", resp.Status) } + os.RemoveAll("TestConfigPostOK") } func TestConfigPostDupFolder(t *testing.T) { diff --git a/lib/config/config.go b/lib/config/config.go index e61798a56..33ec130b6 100644 --- a/lib/config/config.go +++ b/lib/config/config.go @@ -10,6 +10,7 @@ package config import ( "encoding/json" "encoding/xml" + "errors" "fmt" "io" "io/ioutil" @@ -91,6 +92,12 @@ var ( } ) +var ( + errFolderIDEmpty = errors.New("folder has empty ID") + errFolderIDDuplicate = errors.New("folder has duplicate ID") + errFolderPathEmpty = errors.New("folder has empty path") +) + func New(myID protocol.DeviceID) Configuration { var cfg Configuration cfg.Version = CurrentVersion @@ -273,12 +280,17 @@ func (cfg *Configuration) clean() error { folder.prepare() if folder.ID == "" { - return fmt.Errorf("folder with empty ID in configuration") + return errFolderIDEmpty + } + + if folder.Path == "" { + return fmt.Errorf("folder %q: %v", folder.ID, errFolderPathEmpty) } if _, ok := existingFolders[folder.ID]; ok { - return fmt.Errorf("duplicate folder ID %q in configuration", folder.ID) + return fmt.Errorf("folder %q: %v", folder.ID, errFolderIDDuplicate) } + existingFolders[folder.ID] = folder } diff --git a/lib/config/config_test.go b/lib/config/config_test.go index cf7bf7a0c..e3f20cfc1 100644 --- a/lib/config/config_test.go +++ b/lib/config/config_test.go @@ -711,23 +711,19 @@ func TestDuplicateFolders(t *testing.T) { // Duplicate folders are a loading error _, err := Load("testdata/dupfolders.xml", device1) - if err == nil || !strings.HasPrefix(err.Error(), "duplicate folder ID") { + if err == nil || !strings.Contains(err.Error(), errFolderIDDuplicate.Error()) { t.Fatal(`Expected error to mention "duplicate folder ID":`, err) } } func TestEmptyFolderPaths(t *testing.T) { - // Empty folder paths are allowed at the loading stage, and should not + // Empty folder paths are not allowed at the loading stage, and should not // get messed up by the prepare steps (e.g., become the current dir or // get a slash added so that it becomes the root directory or similar). - wrapper, err := Load("testdata/nopath.xml", device1) - if err != nil { - t.Fatal(err) - } - folder := wrapper.Folders()["f1"] - if folder.cachedFilesystem != nil { - t.Errorf("Expected %q to be empty", folder.cachedFilesystem) + _, err := Load("testdata/nopath.xml", device1) + if err == nil || !strings.Contains(err.Error(), errFolderPathEmpty.Error()) { + t.Fatal("Expected error due to empty folder path, got", err) } } @@ -929,6 +925,7 @@ func TestIssue4219(t *testing.T) { "folders": [ { "id": "abcd123", + "path": "testdata", "devices":[ {"deviceID": "GYRZZQB-IRNPV4Z-T7TC52W-EQYJ3TT-FDQW6MW-DFLMU42-SSSU6EM-FBK2VAY"} ] diff --git a/lib/config/folderconfiguration.go b/lib/config/folderconfiguration.go index 4a6ee780e..ad497add9 100644 --- a/lib/config/folderconfiguration.go +++ b/lib/config/folderconfiguration.go @@ -92,7 +92,7 @@ func (f FolderConfiguration) Copy() FolderConfiguration { func (f FolderConfiguration) Filesystem() fs.Filesystem { // This is intentionally not a pointer method, because things like // cfg.Folders["default"].Filesystem() should be valid. - if f.cachedFilesystem == nil && f.Path != "" { + if f.cachedFilesystem == nil { l.Infoln("bug: uncached filesystem call (should only happen in tests)") return fs.NewFilesystem(f.FilesystemType, f.Path) } @@ -209,9 +209,7 @@ func (f *FolderConfiguration) DeviceIDs() []protocol.DeviceID { } func (f *FolderConfiguration) prepare() { - if f.Path != "" { - f.cachedFilesystem = fs.NewFilesystem(f.FilesystemType, f.Path) - } + f.cachedFilesystem = fs.NewFilesystem(f.FilesystemType, f.Path) if f.RescanIntervalS > MaxRescanIntervalS { f.RescanIntervalS = MaxRescanIntervalS diff --git a/lib/config/testdata/dupdevices.xml b/lib/config/testdata/dupdevices.xml index 2eb2718d9..11d5d1364 100644 --- a/lib/config/testdata/dupdevices.xml +++ b/lib/config/testdata/dupdevices.xml @@ -15,7 +15,7 @@
192.0.2.5
- + diff --git a/lib/config/testdata/dupfolders.xml b/lib/config/testdata/dupfolders.xml index cc2cc39c7..dca01e78a 100644 --- a/lib/config/testdata/dupfolders.xml +++ b/lib/config/testdata/dupfolders.xml @@ -1,6 +1,6 @@ - + - + diff --git a/lib/config/testdata/ignoredfolders.xml b/lib/config/testdata/ignoredfolders.xml index 5d0c3a66d..d83490a58 100644 --- a/lib/config/testdata/ignoredfolders.xml +++ b/lib/config/testdata/ignoredfolders.xml @@ -8,7 +8,7 @@ - + diff --git a/lib/config/testdata/pullorder.xml b/lib/config/testdata/pullorder.xml index 59e04f20c..b6ef6a586 100644 --- a/lib/config/testdata/pullorder.xml +++ b/lib/config/testdata/pullorder.xml @@ -1,25 +1,25 @@ - + - + random - + alphabetic - + whatever - + smallestFirst - + largestFirst - + oldestFirst - + newestFirst