Merge pull request #2354 from eipiminus1/issue1361

Add trailing folder seperator to allow symlinks as folder path (fixes #1361)

AUTHORS

@@ -66,3 +66,4 @@ Tomas Cerveny <kozec@kozec.com>
 Tully Robinson <tully@tojr.org>
 Veeti Paananen <veeti.paananen@rojekti.fi>
 Vil Brekin <vilbrekin@gmail.com>
+Yannic A. <eipiminusone+github@gmail.com> <eipiminus1@users.noreply.github.com>

NICKS

@@ -27,6 +27,7 @@ ceh		<emil@hessman.se>
 cqcallaw	<enlightened.despot@gmail.com>
 dva	<denisva@gmail.com>
 dzarda		<dzardacz@gmail.com>
+eipiminus1	<eipiminusone+github@gmail.com> <eipiminus1@users.noreply.github.com>
 facastagnini	<federico.castagnini@gmail.com>
 filoozoom	<philippe@schommers.be>
 frioux		<frew@afoolishmanifesto.com> <frioux@gmail.com>

build.go

@@ -186,7 +186,7 @@ func setup() {
 
 func test(pkg string) {
 	setBuildEnv()
-	runPrint("go", "test", "-short", "-timeout", "60s", pkg)
+	runPrint("go", "test", "-short", "-race", "-timeout", "60s", pkg)
 }
 
 func bench(pkg string) {

build.sh

@@ -108,10 +108,10 @@ case "${1:-default}" in
 		# For every package in the repo
 		for dir in $(go list ./...) ; do
 			# run the tests
-			godep go test -coverprofile=profile.out $dir
+			godep go test -race -coverprofile=profile.out $dir
 			if [ -f profile.out ] ; then
 				# and if there was test output, append it to coverage.out
-				grep -v "mode: set" profile.out >> coverage.out
+				grep -v "mode: " profile.out >> coverage.out
 				rm profile.out
 			fi
 		done

cmd/syncthing/gui.go

@@ -83,12 +83,7 @@ func newAPISvc(id protocol.DeviceID, cfg *config.Wrapper, assetDir string, m *mo
 	return svc, err
 }
 
-func (s *apiSvc) getListener(cfg config.GUIConfiguration) (net.Listener, error) {
-	if guiAddress != "" {
-		// Override from the environment
-		cfg.Address = guiAddress
-	}
-
+func (s *apiSvc) getListener(guiCfg config.GUIConfiguration) (net.Listener, error) {
 	cert, err := tls.LoadX509KeyPair(locations[locHTTPSCertFile], locations[locHTTPSKeyFile])
 	if err != nil {
 		l.Infoln("Loading HTTPS certificate:", err)
@@ -125,7 +120,7 @@ func (s *apiSvc) getListener(cfg config.GUIConfiguration) (net.Listener, error)
 		},
 	}
 
-	rawListener, err := net.Listen("tcp", cfg.Address)
+	rawListener, err := net.Listen("tcp", guiCfg.Address())
 	if err != nil {
 		return nil, err
 	}
@@ -202,14 +197,10 @@ func (s *apiSvc) Serve() {
 	})
 
 	guiCfg := s.cfg.GUI()
-	if guiAPIKey != "" {
-		// Override from the environment
-		guiCfg.APIKey = guiAPIKey
-	}
 
 	// Wrap everything in CSRF protection. The /rest prefix should be
 	// protected, other requests will grant cookies.
-	handler := csrfMiddleware(s.id.String()[:5], "/rest", guiCfg.APIKey, mux)
+	handler := csrfMiddleware(s.id.String()[:5], "/rest", guiCfg.APIKey(), mux)
 
 	// Add our version and ID as a header to responses
 	handler = withDetailsMiddleware(s.id, handler)
@@ -220,7 +211,7 @@ func (s *apiSvc) Serve() {
 	}
 
 	// Redirect to HTTPS if we are supposed to
-	if guiCfg.UseTLS {
+	if guiCfg.UseTLS() {
 		handler = redirectToHTTPSMiddleware(handler)
 	}
 
@@ -236,6 +227,7 @@ func (s *apiSvc) Serve() {
 	s.fss.ServeBackground()
 
 	l.Infoln("API listening on", s.listener.Addr())
+	l.Infoln("GUI URL is", guiCfg.URL())
 	err := srv.Serve(s.listener)
 
 	// The return could be due to an intentional close. Wait for the stop

cmd/syncthing/gui_auth.go

@@ -25,8 +25,9 @@ var (
 )
 
 func basicAuthAndSessionMiddleware(cookieName string, cfg config.GUIConfiguration, next http.Handler) http.Handler {
+	apiKey := cfg.APIKey()
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if cfg.APIKey != "" && r.Header.Get("X-API-Key") == cfg.APIKey {
+		if apiKey != "" && r.Header.Get("X-API-Key") == apiKey {
 			next.ServeHTTP(w, r)
 			return
 		}

cmd/syncthing/main.go

@@ -205,8 +205,6 @@ var (
 	paused         bool
 	noRestart      = os.Getenv("STNORESTART") != ""
 	noUpgrade      = os.Getenv("STNOUPGRADE") != ""
-	guiAddress     = os.Getenv("STGUIADDRESS") // legacy
-	guiAPIKey      = os.Getenv("STGUIAPIKEY")  // legacy
 	profiler       = os.Getenv("STPROFILER")
 	guiAssets      = os.Getenv("STGUIASSETS")
 	cpuProfile     = os.Getenv("STCPUPROFILE") != ""
@@ -226,6 +224,7 @@ func main() {
 		flag.StringVar(&logFile, "logfile", "-", "Log file name (use \"-\" for stdout)")
 	}
 
+	var guiAddress, guiAPIKey string
 	flag.StringVar(&generateDir, "generate", "", "Generate key and config in specified dir, then exit")
 	flag.StringVar(&guiAddress, "gui-address", guiAddress, "Override GUI address")
 	flag.StringVar(&guiAPIKey, "gui-apikey", guiAPIKey, "Override GUI API key")
@@ -246,6 +245,15 @@ func main() {
 	flag.Usage = usageFor(flag.CommandLine, usage, longUsage)
 	flag.Parse()
 
+	if guiAddress != "" {
+		// The config picks this up from the environment.
+		os.Setenv("STGUIADDRESS", guiAddress)
+	}
+	if guiAPIKey != "" {
+		// The config picks this up from the environment.
+		os.Setenv("STGUIAPIKEY", guiAPIKey)
+	}
+
 	if noConsole {
 		osutil.HideConsole()
 	}
@@ -422,14 +430,9 @@ func upgradeViaRest() error {
 	if err != nil {
 		return err
 	}
-	target := cfg.GUI().Address
-	if cfg.GUI().UseTLS {
-		target = "https://" + target
-	} else {
-		target = "http://" + target
-	}
+	target := cfg.GUI().URL()
 	r, _ := http.NewRequest("POST", target+"/rest/system/upgrade", nil)
-	r.Header.Set("X-API-Key", cfg.GUI().APIKey)
+	r.Header.Set("X-API-Key", cfg.GUI().APIKey())
 
 	tr := &http.Transport{
 		TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
@@ -910,48 +913,18 @@ func setupGUI(mainSvc *suture.Supervisor, cfg *config.Wrapper, m *model.Model, a
 	if !guiCfg.Enabled {
 		return
 	}
-	if guiCfg.Address == "" {
-		return
-	}
 
-	addr, err := net.ResolveTCPAddr("tcp", guiCfg.Address)
+	api, err := newAPISvc(myID, cfg, guiAssets, m, apiSub, discoverer, relaySvc, errors, systemLog)
 	if err != nil {
-		l.Fatalf("Cannot start GUI on %q: %v", guiCfg.Address, err)
-	} else {
-		var hostOpen, hostShow string
-		switch {
-		case addr.IP == nil:
-			hostOpen = "localhost"
-			hostShow = "0.0.0.0"
-		case addr.IP.IsUnspecified():
-			hostOpen = "localhost"
-			hostShow = addr.IP.String()
-		default:
-			hostOpen = addr.IP.String()
-			hostShow = hostOpen
-		}
+		l.Fatalln("Cannot start GUI:", err)
+	}
+	cfg.Subscribe(api)
+	mainSvc.Add(api)
 
-		var proto = "http"
-		if guiCfg.UseTLS {
-			proto = "https"
-		}
-
-		urlShow := fmt.Sprintf("%s://%s/", proto, net.JoinHostPort(hostShow, strconv.Itoa(addr.Port)))
-		l.Infoln("Starting web GUI on", urlShow)
-
-		api, err := newAPISvc(myID, cfg, guiAssets, m, apiSub, discoverer, relaySvc, errors, systemLog)
-		if err != nil {
-			l.Fatalln("Cannot start GUI:", err)
-		}
-		cfg.Subscribe(api)
-		mainSvc.Add(api)
-
-		if cfg.Options().StartBrowser && !noBrowser && !stRestarting {
-			urlOpen := fmt.Sprintf("%s://%s/", proto, net.JoinHostPort(hostOpen, strconv.Itoa(addr.Port)))
-			// Can potentially block if the utility we are invoking doesn't
-			// fork, and just execs, hence keep it in it's own routine.
-			go openURL(urlOpen)
-		}
+	if cfg.Options().StartBrowser && !noBrowser && !stRestarting {
+		// Can potentially block if the utility we are invoking doesn't
+		// fork, and just execs, hence keep it in it's own routine.
+		go openURL(guiCfg.URL())
 	}
 }
 
@@ -978,7 +951,7 @@ func defaultConfig(myName string) config.Configuration {
 	if err != nil {
 		l.Fatalln("get free port (GUI):", err)
 	}
-	newCfg.GUI.Address = fmt.Sprintf("127.0.0.1:%d", port)
+	newCfg.GUI.RawAddress = fmt.Sprintf("127.0.0.1:%d", port)
 
 	port, err = getFreePort("0.0.0.0", 22000)
 	if err != nil {

gui/syncthing/core/aboutModalView.html

@@ -78,6 +78,7 @@
         <li class="auto-generated">Tully Robinson</li>
         <li class="auto-generated">Veeti Paananen</li>
         <li class="auto-generated">Vil Brekin</li>
+        <li class="auto-generated">Yannic A.</li>
       </ul>
     </div>
   </div>

lib/config/config.go

@@ -12,6 +12,7 @@ import (
 	"fmt"
 	"io"
 	"math/rand"
+	"net/url"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -288,12 +289,72 @@ func (orig OptionsConfiguration) Copy() OptionsConfiguration {
 }
 
 type GUIConfiguration struct {
-	Enabled  bool   `xml:"enabled,attr" json:"enabled" default:"true"`
-	Address  string `xml:"address" json:"address" default:"127.0.0.1:8384"`
-	User     string `xml:"user,omitempty" json:"user"`
-	Password string `xml:"password,omitempty" json:"password"`
-	UseTLS   bool   `xml:"tls,attr" json:"useTLS"`
-	APIKey   string `xml:"apikey,omitempty" json:"apiKey"`
+	Enabled    bool   `xml:"enabled,attr" json:"enabled" default:"true"`
+	RawAddress string `xml:"address" json:"address" default:"127.0.0.1:8384"`
+	User       string `xml:"user,omitempty" json:"user"`
+	Password   string `xml:"password,omitempty" json:"password"`
+	RawUseTLS  bool   `xml:"tls,attr" json:"useTLS"`
+	RawAPIKey  string `xml:"apikey,omitempty" json:"apiKey"`
+}
+
+func (c GUIConfiguration) Address() string {
+	if override := os.Getenv("STGUIADDRESS"); override != "" {
+		// This value may be of the form "scheme://address:port" or just
+		// "address:port". We need to chop off the scheme. We try to parse it as
+		// an URL if it contains a slash. If that fails, return it as is and let
+		// some other error handling handle it.
+
+		if strings.Contains(override, "/") {
+			url, err := url.Parse(override)
+			if err != nil {
+				return override
+			}
+			return url.Host
+		}
+
+		return override
+	}
+
+	return c.RawAddress
+}
+
+func (c GUIConfiguration) UseTLS() bool {
+	if override := os.Getenv("STGUIADDRESS"); override != "" {
+		return strings.HasPrefix(override, "https:")
+	}
+	return c.RawUseTLS
+}
+
+func (c GUIConfiguration) URL() string {
+	u := url.URL{
+		Scheme: "http",
+		Host:   c.Address(),
+		Path:   "/",
+	}
+
+	if c.UseTLS() {
+		u.Scheme = "https"
+	}
+
+	if strings.HasPrefix(u.Host, ":") {
+		// Empty host, i.e. ":port", use IPv4 localhost
+		u.Host = "127.0.0.1" + u.Host
+	} else if strings.HasPrefix(u.Host, "0.0.0.0:") {
+		// IPv4 all zeroes host, convert to IPv4 localhost
+		u.Host = "127.0.0.1" + u.Host[7:]
+	} else if strings.HasPrefix(u.Host, "[::]:") {
+		// IPv6 all zeroes host, convert to IPv6 localhost
+		u.Host = "[::1]" + u.Host[4:]
+	}
+
+	return u.String()
+}
+
+func (c GUIConfiguration) APIKey() string {
+	if override := os.Getenv("STGUIAPIKEY"); override != "" {
+		return override
+	}
+	return c.RawAPIKey
 }
 
 func New(myID protocol.DeviceID) Configuration {
@@ -363,6 +424,9 @@ func (cfg *Configuration) prepare(myID protocol.DeviceID) {
 		// This way in the tests, we get away without OS specific separators
 		// in the test configs.
 		folder.RawPath = filepath.Dir(folder.RawPath + string(filepath.Separator))
+		if folder.RawPath[len(folder.RawPath)-1] != filepath.Separator {
+			folder.RawPath = folder.RawPath + string(filepath.Separator)
+		}
 
 		if folder.ID == "" {
 			folder.ID = "default"
@@ -463,8 +527,8 @@ func (cfg *Configuration) prepare(myID protocol.DeviceID) {
 		cfg.Options.ReconnectIntervalS = 5
 	}
 
-	if cfg.GUI.APIKey == "" {
-		cfg.GUI.APIKey = randomString(32)
+	if cfg.GUI.RawAPIKey == "" {
+		cfg.GUI.RawAPIKey = randomString(32)
 	}
 }
 

lib/config/config_test.go

@@ -91,7 +91,7 @@ func TestDeviceConfig(t *testing.T) {
 		expectedFolders := []FolderConfiguration{
 			{
 				ID:              "test",
-				RawPath:         "testdata",
+				RawPath:         "testdata" + string(filepath.Separator),
 				Devices:         []FolderDeviceConfiguration{{DeviceID: device1}, {DeviceID: device4}},
 				ReadOnly:        true,
 				RescanIntervalS: 600,
@@ -326,7 +326,7 @@ func TestIssue1262(t *testing.T) {
 	}
 
 	actual := cfg.Folders()["test"].RawPath
-	expected := "e:"
+	expected := "e:/"
 	if runtime.GOOS == "windows" {
 		expected = `e:\`
 	}
@@ -528,7 +528,7 @@ func TestRequiresRestart(t *testing.T) {
 	}
 
 	newCfg = cfg
-	newCfg.GUI.UseTLS = !cfg.GUI.UseTLS
+	newCfg.GUI.RawUseTLS = !cfg.GUI.RawUseTLS
 	if !ChangeRequiresRestart(cfg, newCfg) {
 		t.Error("Changing GUI options requires restart")
 	}
@@ -551,7 +551,7 @@ func TestCopy(t *testing.T) {
 	cfg.Devices[0].Addresses[0] = "wrong"
 	cfg.Folders[0].Devices[0].DeviceID = protocol.DeviceID{0, 1, 2, 3}
 	cfg.Options.ListenAddress[0] = "wrong"
-	cfg.GUI.APIKey = "wrong"
+	cfg.GUI.RawAPIKey = "wrong"
 
 	bsChanged, err := json.MarshalIndent(cfg, "", "  ")
 	if err != nil {
@@ -634,3 +634,25 @@ func TestLargeRescanInterval(t *testing.T) {
 		t.Error("negative rescan interval should become zero")
 	}
 }
+
+func TestGUIConfigURL(t *testing.T) {
+	testcases := [][2]string{
+		{"192.0.2.42:8080", "http://192.0.2.42:8080/"},
+		{":8080", "http://127.0.0.1:8080/"},
+		{"0.0.0.0:8080", "http://127.0.0.1:8080/"},
+		{"127.0.0.1:8080", "http://127.0.0.1:8080/"},
+		{"127.0.0.2:8080", "http://127.0.0.2:8080/"},
+		{"[::]:8080", "http://[::1]:8080/"},
+		{"[2001::42]:8080", "http://[2001::42]:8080/"},
+	}
+
+	for _, tc := range testcases {
+		c := GUIConfiguration{
+			RawAddress: tc[0],
+		}
+		u := c.URL()
+		if u != tc[1] {
+			t.Errorf("Incorrect URL %s != %s for addr %s", u, tc[1], tc[0])
+		}
+	}
+}

lib/model/rwfolder_test.go

@@ -445,11 +445,18 @@ func TestDeregisterOnFailInCopy(t *testing.T) {
 		finisherChan <- state
 		time.Sleep(100 * time.Millisecond)
 
-		if state.fd != nil {
+		state.mut.Lock()
+		stateFd := state.fd
+		state.mut.Unlock()
+		if stateFd != nil {
 			t.Fatal("File not closed?")
 		}
 
-		if len(p.progressEmitter.registry) != 0 || len(p.queue.progress) != 0 || len(p.queue.queued) != 0 {
+		p.queue.mut.Lock()
+		lenQProgr := len(p.queue.progress)
+		lenQQueued := len(p.queue.queued)
+		p.queue.mut.Unlock()
+		if len(p.progressEmitter.registry) != 0 || lenQProgr != 0 || lenQQueued != 0 {
 			t.Fatal("Still registered", len(p.progressEmitter.registry), len(p.queue.progress), len(p.queue.queued))
 		}
 
@@ -457,7 +464,11 @@ func TestDeregisterOnFailInCopy(t *testing.T) {
 		finisherChan <- state
 		time.Sleep(100 * time.Millisecond)
 
-		if len(p.progressEmitter.registry) != 0 || len(p.queue.progress) != 0 || len(p.queue.queued) != 0 {
+		p.queue.mut.Lock()
+		lenQProgr = len(p.queue.progress)
+		lenQQueued = len(p.queue.queued)
+		p.queue.mut.Unlock()
+		if len(p.progressEmitter.registry) != 0 || lenQProgr != 0 || lenQQueued != 0 {
 			t.Fatal("Still registered")
 		}
 	case <-time.After(time.Second):
@@ -527,12 +538,19 @@ func TestDeregisterOnFailInPull(t *testing.T) {
 		finisherChan <- state
 		time.Sleep(100 * time.Millisecond)
 
-		if state.fd != nil {
+		state.mut.Lock()
+		stateFd := state.fd
+		state.mut.Unlock()
+		if stateFd != nil {
 			t.Fatal("File not closed?")
 		}
 
-		if len(p.progressEmitter.registry) != 0 || len(p.queue.progress) != 0 || len(p.queue.queued) != 0 {
-			t.Fatal("Still registered", len(p.progressEmitter.registry), len(p.queue.progress), len(p.queue.queued))
+		p.queue.mut.Lock()
+		lenQProgr := len(p.queue.progress)
+		lenQQueued := len(p.queue.queued)
+		p.queue.mut.Unlock()
+		if len(p.progressEmitter.registry) != 0 || lenQProgr != 0 || lenQQueued != 0 {
+			t.Fatal("Still registered", len(p.progressEmitter.registry), lenQProgr, lenQQueued)
 		}
 
 		// Doing it again should have no effect

lib/relay/client/client.go

@@ -78,6 +78,7 @@ func (c *ProtocolClient) Serve() {
 	}
 
 	if err := c.conn.SetDeadline(time.Time{}); err != nil {
+		c.conn.Close()
 		l.Infoln("Relay set deadline:", err)
 		return
 	}

lib/scanner/walk.go

@@ -141,7 +141,7 @@ func (w *Walker) Walk() (chan protocol.FileInfo, error) {
 	// which it receives the files we ask it to hash.
 	go func() {
 		var filesToHash []protocol.FileInfo
-		var total, progress int64
+		var total, progress int64 = 1, 0
 		for file := range toHashChan {
 			filesToHash = append(filesToHash, file)
 			total += int64(file.CachedSize)