lib/config: Error on empty folder path (fixes #5853) (#5854)

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) {

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
 	}
 

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)
+	_, err := Load("testdata/nopath.xml", device1)
-	if err != nil {
+	if err == nil || !strings.Contains(err.Error(), errFolderPathEmpty.Error()) {
-		t.Fatal(err)
+		t.Fatal("Expected error due to empty folder path, got", err)
-	}
-	folder := wrapper.Folders()["f1"]
-	if folder.cachedFilesystem != nil {
-		t.Errorf("Expected %q to be empty", folder.cachedFilesystem)
 	}
 }
 
@@ -929,6 +925,7 @@ func TestIssue4219(t *testing.T) {
 		"folders": [
 			{
 				"id": "abcd123",
+				"path": "testdata",
 				"devices":[
 					{"deviceID": "GYRZZQB-IRNPV4Z-T7TC52W-EQYJ3TT-FDQW6MW-DFLMU42-SSSU6EM-FBK2VAY"}
 				]

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

lib/config/testdata/dupdevices.xml

@@ -15,7 +15,7 @@
         <!-- duplicate, will be removed -->
         <address>192.0.2.5</address>
     </device>
-    <folder id="f2" directory="testdata/">
+    <folder id="f2" path="testdata/">
         <device id="AIR6LPZ-7K4PTTV-UXQSMUU-CPQ5YWH-OEDFIIQ-JUG777G-2YQXXR5-YD6AWQR"></device>
         <device id="GYRZZQBIRNPV4T7TC52WEQYJ3TFDQW6MWDFLMU4SSSU6EMFBK2VA"></device>
         <!-- duplicate device, will be removed -->

lib/config/testdata/dupfolders.xml

@@ -1,6 +1,6 @@
 <configuration version="15">
-    <folder id="f1" directory="testdata/">
+    <folder id="f1" path="testdata/">
     </folder>
-    <folder id="f1" directory="testdata/">
+    <folder id="f1" path="testdata/">
     </folder>
 </configuration>

lib/config/testdata/ignoredfolders.xml

@@ -8,7 +8,7 @@
         <ignoredFolder id="folder1"/>
         <ignoredFolder id="folder2"/>
     </device>
-    <folder id="folder1" directory="testdata/">
+    <folder id="folder1" path="testdata/">
         <device id="GYRZZQB-IRNPV4Z-T7TC52W-EQYJ3TT-FDQW6MW-DFLMU42-SSSU6EM-FBK2VAY"></device>
     </folder>
 </configuration>

lib/config/testdata/pullorder.xml

@@ -1,25 +1,25 @@
 <configuration version="10">
-    <folder id="f1" directory="testdata/">
+    <folder id="f1" path="testdata/">
     </folder>
-    <folder id="f2" directory="testdata/">
+    <folder id="f2" path="testdata/">
         <order>random</order>
     </folder>
-    <folder id="f3" directory="testdata/">
+    <folder id="f3" path="testdata/">
         <order>alphabetic</order>
     </folder>
-    <folder id="f4" directory="testdata/">
+    <folder id="f4" path="testdata/">
         <order>whatever</order>
     </folder>
-    <folder id="f5" directory="testdata/">
+    <folder id="f5" path="testdata/">
         <order>smallestFirst</order>
     </folder>
-    <folder id="f6" directory="testdata/">
+    <folder id="f6" path="testdata/">
         <order>largestFirst</order>
     </folder>
-    <folder id="f7" directory="testdata/">
+    <folder id="f7" path="testdata/">
         <order>oldestFirst</order>
     </folder>
-    <folder id="f8" directory="testdata/">
+    <folder id="f8" path="testdata/">
         <order>newestFirst</order>
     </folder>
 </configuration>