diff --git a/cmd/syncthing/gui.go b/cmd/syncthing/gui.go
index d6d6d4c3..aac6511d 100644
--- a/cmd/syncthing/gui.go
+++ b/cmd/syncthing/gui.go
@@ -577,7 +577,7 @@ func (s *apiService) getDBStatus(w http.ResponseWriter, r *http.Request) {
func folderSummary(cfg configIntf, m modelIntf, folder string) map[string]interface{} {
var res = make(map[string]interface{})
- res["invalid"] = cfg.Folders()[folder].Invalid
+ res["invalid"] = "" // Deprecated, retains external API for now
globalFiles, globalDeleted, globalBytes := m.GlobalSize(folder)
res["globalFiles"], res["globalDeleted"], res["globalBytes"] = globalFiles, globalDeleted, globalBytes
@@ -690,7 +690,7 @@ func (s *apiService) postSystemConfig(w http.ResponseWriter, r *http.Request) {
to, err := config.ReadJSON(r.Body, myID)
r.Body.Close()
if err != nil {
- l.Warnln("decoding posted config:", err)
+ l.Warnln("Decoding posted config:", err)
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
diff --git a/cmd/syncthing/gui_test.go b/cmd/syncthing/gui_test.go
index 4dbf7649..bdbf58a1 100644
--- a/cmd/syncthing/gui_test.go
+++ b/cmd/syncthing/gui_test.go
@@ -11,6 +11,7 @@ import (
"compress/gzip"
"encoding/json"
"fmt"
+ "io"
"io/ioutil"
"net"
"net/http"
@@ -613,3 +614,55 @@ func TestRandomString(t *testing.T) {
t.Errorf("Expected 27 random characters, got %q of length %d", res["random"], len(res["random"]))
}
}
+
+func TestConfigPostOK(t *testing.T) {
+ cfg := bytes.NewBuffer([]byte(`{
+ "version": 15,
+ "folders": [
+ {"id": "foo"}
+ ]
+ }`))
+
+ resp, err := testConfigPost(cfg)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if resp.StatusCode != http.StatusOK {
+ t.Error("Expected 200 OK, not", resp.Status)
+ }
+}
+
+func TestConfigPostDupFolder(t *testing.T) {
+ cfg := bytes.NewBuffer([]byte(`{
+ "version": 15,
+ "folders": [
+ {"id": "foo"},
+ {"id": "foo"}
+ ]
+ }`))
+
+ resp, err := testConfigPost(cfg)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if resp.StatusCode != http.StatusBadRequest {
+ t.Error("Expected 400 Bad Request, not", resp.Status)
+ }
+}
+
+func testConfigPost(data io.Reader) (*http.Response, error) {
+ const testAPIKey = "foobarbaz"
+ cfg := new(mockedConfig)
+ cfg.gui.APIKey = testAPIKey
+ baseURL, err := startHTTP(cfg)
+ if err != nil {
+ return nil, err
+ }
+ cli := &http.Client{
+ Timeout: time.Second,
+ }
+
+ req, _ := http.NewRequest("POST", baseURL+"/rest/system/config", data)
+ req.Header.Set("X-API-Key", testAPIKey)
+ return cli.Do(req)
+}
diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go
index 794fa239..6130bc28 100644
--- a/cmd/syncthing/main.go
+++ b/cmd/syncthing/main.go
@@ -863,7 +863,6 @@ func loadConfig() (*config.Wrapper, error) {
cfg, err := config.Load(cfgFile, myID)
if err != nil {
- l.Infoln("Error loading config file; using defaults for now")
myName, _ := os.Hostname()
newCfg := defaultConfig(myName)
cfg = config.Wrap(cfgFile, newCfg)
diff --git a/lib/config/config.go b/lib/config/config.go
index aa8c1279..97e72c93 100644
--- a/lib/config/config.go
+++ b/lib/config/config.go
@@ -10,6 +10,7 @@ package config
import (
"encoding/json"
"encoding/xml"
+ "fmt"
"io"
"io/ioutil"
"net/url"
@@ -81,11 +82,15 @@ func ReadXML(r io.Reader, myID protocol.DeviceID) (Configuration, error) {
util.SetDefaults(&cfg.Options)
util.SetDefaults(&cfg.GUI)
- err := xml.NewDecoder(r).Decode(&cfg)
+ if err := xml.NewDecoder(r).Decode(&cfg); err != nil {
+ return Configuration{}, err
+ }
cfg.OriginalVersion = cfg.Version
- cfg.prepare(myID)
- return cfg, err
+ if err := cfg.prepare(myID); err != nil {
+ return Configuration{}, err
+ }
+ return cfg, nil
}
func ReadJSON(r io.Reader, myID protocol.DeviceID) (Configuration, error) {
@@ -97,14 +102,16 @@ func ReadJSON(r io.Reader, myID protocol.DeviceID) (Configuration, error) {
bs, err := ioutil.ReadAll(r)
if err != nil {
- return cfg, err
+ return Configuration{}, err
}
err = json.Unmarshal(bs, &cfg)
cfg.OriginalVersion = cfg.Version
- cfg.prepare(myID)
- return cfg, err
+ if err := cfg.prepare(myID); err != nil {
+ return Configuration{}, err
+ }
+ return cfg, nil
}
type Configuration struct {
@@ -154,7 +161,7 @@ func (cfg *Configuration) WriteXML(w io.Writer) error {
return err
}
-func (cfg *Configuration) prepare(myID protocol.DeviceID) {
+func (cfg *Configuration) prepare(myID protocol.DeviceID) error {
util.FillNilSlices(&cfg.Options)
// Initialize any empty slices
@@ -168,19 +175,19 @@ func (cfg *Configuration) prepare(myID protocol.DeviceID) {
cfg.Options.AlwaysLocalNets = []string{}
}
- // Check for missing, bad or duplicate folder ID:s
- var seenFolders = map[string]*FolderConfiguration{}
+ // Prepare folders and check for duplicates. Duplicates are bad and
+ // dangerous, can't currently be resolved in the GUI, and shouldn't
+ // happen when configured by the GUI. We return with an error in that
+ // situation.
+ seenFolders := make(map[string]struct{})
for i := range cfg.Folders {
folder := &cfg.Folders[i]
folder.prepare()
- if seen, ok := seenFolders[folder.ID]; ok {
- l.Warnf("Multiple folders with ID %q; disabling", folder.ID)
- seen.Invalid = "duplicate folder ID"
- folder.Invalid = "duplicate folder ID"
- } else {
- seenFolders[folder.ID] = folder
+ if _, ok := seenFolders[folder.ID]; ok {
+ return fmt.Errorf("duplicate folder ID %q in configuration", folder.ID)
}
+ seenFolders[folder.ID] = struct{}{}
}
cfg.Options.ListenAddresses = util.UniqueStrings(cfg.Options.ListenAddresses)
@@ -257,6 +264,8 @@ func (cfg *Configuration) prepare(myID protocol.DeviceID) {
if cfg.GUI.APIKey == "" {
cfg.GUI.APIKey = rand.String(32)
}
+
+ return nil
}
func convertV14V15(cfg *Configuration) {
diff --git a/lib/config/config_test.go b/lib/config/config_test.go
index f2288ee3..a4802ca4 100644
--- a/lib/config/config_test.go
+++ b/lib/config/config_test.go
@@ -595,22 +595,14 @@ func TestGUIConfigURL(t *testing.T) {
}
}
-func TestRemoveDuplicateDevicesFolders(t *testing.T) {
- wrapper, err := Load("testdata/duplicates.xml", device1)
+func TestDuplicateDevices(t *testing.T) {
+ // Duplicate devices should be removed
+
+ wrapper, err := Load("testdata/dupdevices.xml", device1)
if err != nil {
t.Fatal(err)
}
- // All folders are loaded, but the duplicate ones are disabled.
- if l := len(wrapper.Raw().Folders); l != 3 {
- t.Errorf("Incorrect number of folders, %d != 3", l)
- }
- for i, f := range wrapper.Raw().Folders {
- if f.ID == "f1" && f.Invalid == "" {
- t.Errorf("Folder %d (%q) is not set invalid", i, f.ID)
- }
- }
-
if l := len(wrapper.Raw().Devices); l != 3 {
t.Errorf("Incorrect number of devices, %d != 3", l)
}
@@ -621,6 +613,30 @@ func TestRemoveDuplicateDevicesFolders(t *testing.T) {
}
}
+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") {
+ 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
+ // 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.Path() != "" {
+ t.Errorf("Expected %q to be empty", folder.Path())
+ }
+}
+
func TestV14ListenAddressesMigration(t *testing.T) {
tcs := [][3][]string{
diff --git a/lib/config/folderconfiguration.go b/lib/config/folderconfiguration.go
index 5782d0ed..15c426fd 100644
--- a/lib/config/folderconfiguration.go
+++ b/lib/config/folderconfiguration.go
@@ -39,7 +39,6 @@ type FolderConfiguration struct {
DisableSparseFiles bool `xml:"disableSparseFiles" json:"disableSparseFiles"`
DisableTempIndexes bool `xml:"disableTempIndexes" json:"disableTempIndexes"`
- Invalid string `xml:"-" json:"invalid"` // Set at runtime when there is an error, not saved
cachedPath string
DeprecatedReadOnly bool `xml:"ro,attr,omitempty" json:"-"`
@@ -70,7 +69,7 @@ func (f FolderConfiguration) Path() string {
// This is intentionally not a pointer method, because things like
// cfg.Folders["default"].Path() should be valid.
- if f.cachedPath == "" {
+ if f.cachedPath == "" && f.RawPath != "" {
l.Infoln("bug: uncached path call (should only happen in tests)")
return f.cleanedPath()
}
@@ -108,31 +107,24 @@ func (f *FolderConfiguration) DeviceIDs() []protocol.DeviceID {
}
func (f *FolderConfiguration) prepare() {
- if len(f.RawPath) == 0 {
- f.Invalid = "no directory configured"
- return
- }
+ if f.RawPath != "" {
+ // The reason it's done like this:
+ // C: -> C:\ -> C:\ (issue that this is trying to fix)
+ // C:\somedir -> C:\somedir\ -> C:\somedir
+ // C:\somedir\ -> C:\somedir\\ -> C:\somedir
+ // This way in the tests, we get away without OS specific separators
+ // in the test configs.
+ f.RawPath = filepath.Dir(f.RawPath + string(filepath.Separator))
- // The reason it's done like this:
- // C: -> C:\ -> C:\ (issue that this is trying to fix)
- // C:\somedir -> C:\somedir\ -> C:\somedir
- // C:\somedir\ -> C:\somedir\\ -> C:\somedir
- // This way in the tests, we get away without OS specific separators
- // in the test configs.
- f.RawPath = filepath.Dir(f.RawPath + string(filepath.Separator))
-
- // If we're not on Windows, we want the path to end with a slash to
- // penetrate symlinks. On Windows, paths must not end with a slash.
- if runtime.GOOS != "windows" && f.RawPath[len(f.RawPath)-1] != filepath.Separator {
- f.RawPath = f.RawPath + string(filepath.Separator)
+ // If we're not on Windows, we want the path to end with a slash to
+ // penetrate symlinks. On Windows, paths must not end with a slash.
+ if runtime.GOOS != "windows" && f.RawPath[len(f.RawPath)-1] != filepath.Separator {
+ f.RawPath = f.RawPath + string(filepath.Separator)
+ }
}
f.cachedPath = f.cleanedPath()
- if f.ID == "" {
- f.ID = "default"
- }
-
if f.RescanIntervalS > MaxRescanIntervalS {
f.RescanIntervalS = MaxRescanIntervalS
} else if f.RescanIntervalS < 0 {
@@ -145,6 +137,10 @@ func (f *FolderConfiguration) prepare() {
}
func (f *FolderConfiguration) cleanedPath() string {
+ if f.RawPath == "" {
+ return ""
+ }
+
cleaned := f.RawPath
// Attempt tilde expansion; leave unchanged in case of error
diff --git a/lib/config/testdata/duplicates.xml b/lib/config/testdata/dupdevices.xml
similarity index 70%
rename from lib/config/testdata/duplicates.xml
rename to lib/config/testdata/dupdevices.xml
index a6fbbd15..2eb2718d 100644
--- a/lib/config/testdata/duplicates.xml
+++ b/lib/config/testdata/dupdevices.xml
@@ -15,15 +15,6 @@
192.0.2.5
-
-
-
-
-
-
-
-
-
diff --git a/lib/config/testdata/dupfolders.xml b/lib/config/testdata/dupfolders.xml
new file mode 100644
index 00000000..cc2cc39c
--- /dev/null
+++ b/lib/config/testdata/dupfolders.xml
@@ -0,0 +1,6 @@
+
+
+
+
+
+
diff --git a/lib/config/testdata/nopath.xml b/lib/config/testdata/nopath.xml
new file mode 100644
index 00000000..706f93f2
--- /dev/null
+++ b/lib/config/testdata/nopath.xml
@@ -0,0 +1,4 @@
+
+
+
+
diff --git a/lib/model/model.go b/lib/model/model.go
index 0e055514..d4e87406 100644
--- a/lib/model/model.go
+++ b/lib/model/model.go
@@ -110,6 +110,7 @@ var (
// errors returned by the CheckFolderHealth method
var (
+ errFolderPathEmpty = errors.New("folder path empty")
errFolderPathMissing = errors.New("folder path missing")
errFolderMarkerMissing = errors.New("folder marker missing")
errHomeDiskNoSpace = errors.New("home disk has insufficient free space")
@@ -658,23 +659,19 @@ func (m *Model) ClusterConfig(deviceID protocol.DeviceID, cm protocol.ClusterCon
tempIndexFolders := make([]string, 0, len(cm.Folders))
- m.fmut.Lock()
-nextFolder:
for _, folder := range cm.Folders {
- cfg := m.folderCfgs[folder.ID]
-
if folder.Flags&^protocol.FlagFolderAll != 0 {
- // There are flags set that we don't know what they mean. Scary!
+ // There are flags set that we don't know what they mean. Fatal!
l.Warnf("Device %v: unknown flags for folder %s", deviceID, folder.ID)
- cfg.Invalid = fmt.Sprintf("Unknown flags from device %v", deviceID)
- m.cfg.SetFolder(cfg)
- if srv := m.folderRunners[folder.ID]; srv != nil {
- srv.setError(fmt.Errorf(cfg.Invalid))
- }
- continue nextFolder
+ m.fmut.Unlock()
+ m.Close(deviceID, fmt.Errorf("Unknown folder flags from device %v", deviceID))
+ return
}
- if !m.folderSharedWithUnlocked(folder.ID, deviceID) {
+ m.fmut.Lock()
+ shared := m.folderSharedWithUnlocked(folder.ID, deviceID)
+ m.fmut.Unlock()
+ if !shared {
events.Default.Log(events.FolderRejected, map[string]string{
"folder": folder.ID,
"folderLabel": folder.Label,
@@ -687,7 +684,6 @@ nextFolder:
tempIndexFolders = append(tempIndexFolders, folder.ID)
}
}
- m.fmut.Unlock()
// This breaks if we send multiple CM messages during the same connection.
if len(tempIndexFolders) > 0 {
@@ -1953,6 +1949,10 @@ func (m *Model) CheckFolderHealth(id string) error {
// checkFolderPath returns nil if the folder path exists and has the marker file.
func (m *Model) checkFolderPath(folder config.FolderConfiguration) error {
+ if folder.Path() == "" {
+ return errFolderPathEmpty
+ }
+
if fi, err := os.Stat(folder.Path()); err != nil || !fi.IsDir() {
return errFolderPathMissing
}
diff --git a/lib/model/model_test.go b/lib/model/model_test.go
index 7e335603..baab865b 100644
--- a/lib/model/model_test.go
+++ b/lib/model/model_test.go
@@ -642,9 +642,6 @@ func TestROScanRecovery(t *testing.T) {
waitFor := func(status string) error {
timeout := time.Now().Add(2 * time.Second)
for {
- if time.Now().After(timeout) {
- return fmt.Errorf("Timed out waiting for status: %s, current status: %s", status, m.cfg.Folders()["default"].Invalid)
- }
_, _, err := m.State("default")
if err == nil && status == "" {
return nil
@@ -652,6 +649,10 @@ func TestROScanRecovery(t *testing.T) {
if err != nil && err.Error() == status {
return nil
}
+
+ if time.Now().After(timeout) {
+ return fmt.Errorf("Timed out waiting for status: %s, current status: %v", status, err)
+ }
time.Sleep(10 * time.Millisecond)
}
}
@@ -727,9 +728,6 @@ func TestRWScanRecovery(t *testing.T) {
waitFor := func(status string) error {
timeout := time.Now().Add(2 * time.Second)
for {
- if time.Now().After(timeout) {
- return fmt.Errorf("Timed out waiting for status: %s, current status: %s", status, m.cfg.Folders()["default"].Invalid)
- }
_, _, err := m.State("default")
if err == nil && status == "" {
return nil
@@ -737,6 +735,10 @@ func TestRWScanRecovery(t *testing.T) {
if err != nil && err.Error() == status {
return nil
}
+
+ if time.Now().After(timeout) {
+ return fmt.Errorf("Timed out waiting for status: %s, current status: %v", status, err)
+ }
time.Sleep(10 * time.Millisecond)
}
}