Fix handling of default values in config (fixes #83)

Fix isTempName to work on Windows (fixes #80)

cmd/syncthing/config.go

@@ -46,7 +46,7 @@ type OptionsConfiguration struct {
 	MaxChangeKbps      int      `xml:"maxChangeKbps" default:"1000" ini:"max-change-bw"`
 }
 
-func setDefaults(data interface{}, setEmptySlices bool) error {
+func setDefaults(data interface{}) error {
 	s := reflect.ValueOf(data).Elem()
 	t := s.Type()
 
@@ -56,21 +56,10 @@ func setDefaults(data interface{}, setEmptySlices bool) error {
 
 		v := tag.Get("default")
 		if len(v) > 0 {
-			if f.Kind().String() == "slice" && f.Len() != 0 {
-				continue
-			}
-
 			switch f.Interface().(type) {
 			case string:
 				f.SetString(v)
 
-			case []string:
-				if setEmptySlices {
-					rv := reflect.MakeSlice(reflect.TypeOf([]string{}), 1, 1)
-					rv.Index(0).SetString(v)
-					f.Set(rv)
-				}
-
 			case int:
 				i, err := strconv.ParseInt(v, 10, 64)
 				if err != nil {
@@ -81,6 +70,11 @@ func setDefaults(data interface{}, setEmptySlices bool) error {
 			case bool:
 				f.SetBool(v == "true")
 
+			case []string:
+				// We don't do anything with string slices here. Any default
+				// we set will be appended to by the XML decoder, so we fill
+				// those after decoding.
+
 			default:
 				panic(f.Type())
 			}
@@ -89,6 +83,30 @@ func setDefaults(data interface{}, setEmptySlices bool) error {
 	return nil
 }
 
+// fillNilSlices sets default value on slices that are still nil.
+func fillNilSlices(data interface{}) error {
+	s := reflect.ValueOf(data).Elem()
+	t := s.Type()
+
+	for i := 0; i < s.NumField(); i++ {
+		f := s.Field(i)
+		tag := t.Field(i).Tag
+
+		v := tag.Get("default")
+		if len(v) > 0 {
+			switch f.Interface().(type) {
+			case []string:
+				if f.IsNil() {
+					rv := reflect.MakeSlice(reflect.TypeOf([]string{}), 1, 1)
+					rv.Index(0).SetString(v)
+					f.Set(rv)
+				}
+			}
+		}
+	}
+	return nil
+}
+
 func readConfigINI(m map[string]string, data interface{}) error {
 	s := reflect.ValueOf(data).Elem()
 	t := s.Type()
@@ -152,15 +170,15 @@ func uniqueStrings(ss []string) []string {
 func readConfigXML(rd io.Reader) (Configuration, error) {
 	var cfg Configuration
 
-	setDefaults(&cfg, false)
-	setDefaults(&cfg.Options, false)
+	setDefaults(&cfg)
+	setDefaults(&cfg.Options)
 
 	var err error
 	if rd != nil {
 		err = xml.NewDecoder(rd).Decode(&cfg)
 	}
 
-	setDefaults(&cfg.Options, true)
+	fillNilSlices(&cfg.Options)
 
 	cfg.Options.ListenAddress = uniqueStrings(cfg.Options.ListenAddress)
 	return cfg, err

cmd/syncthing/config_test.go

@@ -0,0 +1,113 @@
+package main
+
+import (
+	"bytes"
+	"io"
+	"reflect"
+	"testing"
+)
+
+func TestDefaultValues(t *testing.T) {
+	expected := OptionsConfiguration{
+		ListenAddress:      []string{":22000"},
+		ReadOnly:           false,
+		AllowDelete:        true,
+		FollowSymlinks:     true,
+		GUIEnabled:         true,
+		GUIAddress:         "127.0.0.1:8080",
+		GlobalAnnServer:    "announce.syncthing.net:22025",
+		GlobalAnnEnabled:   true,
+		LocalAnnEnabled:    true,
+		ParallelRequests:   16,
+		MaxSendKbps:        0,
+		RescanIntervalS:    60,
+		ReconnectIntervalS: 60,
+		MaxChangeKbps:      1000,
+	}
+
+	cfg, err := readConfigXML(bytes.NewReader(nil))
+	if err != io.EOF {
+		t.Error(err)
+	}
+
+	if !reflect.DeepEqual(cfg.Options, expected) {
+		t.Errorf("Default config differs;\n  E: %#v\n  A: %#v", expected, cfg.Options)
+	}
+}
+
+func TestNoListenAddress(t *testing.T) {
+	data := []byte(`<configuration version="1">
+    <repository directory="~/Sync">
+        <node id="..." name="...">
+            <address>dynamic</address>
+        </node>
+    </repository>
+    <options>
+        <listenAddress></listenAddress>
+    </options>
+</configuration>
+`)
+
+	cfg, err := readConfigXML(bytes.NewReader(data))
+	if err != nil {
+		t.Error(err)
+	}
+
+	expected := []string{""}
+	if !reflect.DeepEqual(cfg.Options.ListenAddress, expected) {
+		t.Errorf("Unexpected ListenAddress %#v", cfg.Options.ListenAddress)
+	}
+}
+
+func TestOverriddenValues(t *testing.T) {
+	data := []byte(`<configuration version="1">
+    <repository directory="~/Sync">
+        <node id="..." name="...">
+            <address>dynamic</address>
+        </node>
+    </repository>
+    <options>
+       <listenAddress>:23000</listenAddress>
+        <readOnly>true</readOnly>
+        <allowDelete>false</allowDelete>
+        <followSymlinks>false</followSymlinks>
+        <guiEnabled>false</guiEnabled>
+        <guiAddress>125.2.2.2:8080</guiAddress>
+        <globalAnnounceServer>syncthing.nym.se:22025</globalAnnounceServer>
+        <globalAnnounceEnabled>false</globalAnnounceEnabled>
+        <localAnnounceEnabled>false</localAnnounceEnabled>
+        <parallelRequests>32</parallelRequests>
+        <maxSendKbps>1234</maxSendKbps>
+        <rescanIntervalS>600</rescanIntervalS>
+        <reconnectionIntervalS>6000</reconnectionIntervalS>
+        <maxChangeKbps>2345</maxChangeKbps>
+    </options>
+</configuration>
+`)
+
+	expected := OptionsConfiguration{
+		ListenAddress:      []string{":23000"},
+		ReadOnly:           true,
+		AllowDelete:        false,
+		FollowSymlinks:     false,
+		GUIEnabled:         false,
+		GUIAddress:         "125.2.2.2:8080",
+		GlobalAnnServer:    "syncthing.nym.se:22025",
+		GlobalAnnEnabled:   false,
+		LocalAnnEnabled:    false,
+		ParallelRequests:   32,
+		MaxSendKbps:        1234,
+		RescanIntervalS:    600,
+		ReconnectIntervalS: 6000,
+		MaxChangeKbps:      2345,
+	}
+
+	cfg, err := readConfigXML(bytes.NewReader(data))
+	if err != nil {
+		t.Error(err)
+	}
+
+	if !reflect.DeepEqual(cfg.Options, expected) {
+		t.Errorf("Overridden config differs;\n  E: %#v\n  A: %#v", expected, cfg.Options)
+	}
+}

cmd/syncthing/walk.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"time"
 
@@ -39,6 +40,9 @@ func (f File) NewerThan(o File) bool {
 }
 
 func isTempName(name string) bool {
+	if runtime.GOOS == "windows" {
+		name = filepath.ToSlash(name)
+	}
 	return strings.HasPrefix(path.Base(name), ".syncthing.")
 }