From a4147d90191bedd5d69415eafb08c7c83caea926 Mon Sep 17 00:00:00 2001
From: Jakob Borg <jakob@kastelo.net>
Date: Wed, 13 Dec 2017 09:34:47 +0000
Subject: [PATCH] cmd/syncthing: Fix /rest/system/browse for folder path
 completion (fixes #4590)

Two issues since the filesystem migration;

- filepath.Base() doesn't do what the code expected when the path ends
  in a slash, and we make sure the path ends in a slash.

- the return should be fully qualified, not just the last component.

With this change it works as before, and passes the new test for it.

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4591
LGTM: imsodin
---
 cmd/syncthing/gui.go      | 24 ++++++++++-----
 cmd/syncthing/gui_test.go | 61 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 8 deletions(-)

diff --git a/cmd/syncthing/gui.go b/cmd/syncthing/gui.go
index b6a19d1b3..5bf7f73af 100644
--- a/cmd/syncthing/gui.go
+++ b/cmd/syncthing/gui.go
@@ -1280,18 +1280,21 @@ func (s *apiService) getPeerCompletion(w http.ResponseWriter, r *http.Request) {
 func (s *apiService) getSystemBrowse(w http.ResponseWriter, r *http.Request) {
 	qs := r.URL.Query()
 	current := qs.Get("current")
+
 	// Default value or in case of error unmarshalling ends up being basic fs.
 	var fsType fs.FilesystemType
 	fsType.UnmarshalText([]byte(qs.Get("filesystem")))
 
+	sendJSON(w, browseFiles(current, fsType))
+}
+
+func browseFiles(current string, fsType fs.FilesystemType) []string {
 	if current == "" {
 		filesystem := fs.NewFilesystem(fsType, "")
 		if roots, err := filesystem.Roots(); err == nil {
-			sendJSON(w, roots)
-		} else {
-			http.Error(w, err.Error(), 500)
+			return roots
 		}
-		return
+		return nil
 	}
 	search, _ := fs.ExpandTilde(current)
 	pathSeparator := string(fs.PathSeparator)
@@ -1300,7 +1303,13 @@ func (s *apiService) getSystemBrowse(w http.ResponseWriter, r *http.Request) {
 		search = search + pathSeparator
 	}
 	searchDir := filepath.Dir(search)
-	searchFile := filepath.Base(search)
+
+	// The searchFile should be the last component of search, or empty if it
+	// ends with a path separator
+	var searchFile string
+	if !strings.HasSuffix(search, pathSeparator) {
+		searchFile = filepath.Base(search)
+	}
 
 	fs := fs.NewFilesystem(fsType, searchDir)
 
@@ -1310,11 +1319,10 @@ func (s *apiService) getSystemBrowse(w http.ResponseWriter, r *http.Request) {
 	for _, subdirectory := range subdirectories {
 		info, err := fs.Stat(subdirectory)
 		if err == nil && info.IsDir() {
-			ret = append(ret, subdirectory+pathSeparator)
+			ret = append(ret, filepath.Join(searchDir, subdirectory)+pathSeparator)
 		}
 	}
-
-	sendJSON(w, ret)
+	return ret
 }
 
 func (s *apiService) getCPUProf(w http.ResponseWriter, r *http.Request) {
diff --git a/cmd/syncthing/gui_test.go b/cmd/syncthing/gui_test.go
index 1ab8176e9..b3adfd5a7 100644
--- a/cmd/syncthing/gui_test.go
+++ b/cmd/syncthing/gui_test.go
@@ -16,6 +16,8 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"os"
+	"path/filepath"
 	"strconv"
 	"strings"
 	"testing"
@@ -24,6 +26,7 @@ import (
 	"github.com/d4l3k/messagediff"
 	"github.com/syncthing/syncthing/lib/config"
 	"github.com/syncthing/syncthing/lib/events"
+	"github.com/syncthing/syncthing/lib/fs"
 	"github.com/syncthing/syncthing/lib/protocol"
 	"github.com/syncthing/syncthing/lib/sync"
 	"github.com/thejerf/suture"
@@ -957,3 +960,61 @@ func TestEventMasks(t *testing.T) {
 		t.Errorf("should have returned a valid, non-default event sub")
 	}
 }
+
+func TestBrowse(t *testing.T) {
+	pathSep := string(os.PathSeparator)
+
+	tmpDir, err := ioutil.TempDir("", "syncthing")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	if err := os.Mkdir(filepath.Join(tmpDir, "dir"), 0755); err != nil {
+		t.Fatal(err)
+	}
+	if err := ioutil.WriteFile(filepath.Join(tmpDir, "file"), []byte("hello"), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	// We expect completion to return the full path to the completed
+	// directory, with an ending slash.
+	dirPath := filepath.Join(tmpDir, "dir") + pathSep
+
+	cases := []struct {
+		current string
+		returns []string
+	}{
+		// The direcotory without slash is completed to one with slash.
+		{tmpDir, []string{tmpDir + pathSep}},
+		// With slash it's completed to its contents.
+		// Dirs are given pathSeps.
+		// Files are not returned.
+		{tmpDir + pathSep, []string{dirPath}},
+		// Globbing is automatic based on prefix.
+		{tmpDir + pathSep + "d", []string{dirPath}},
+		{tmpDir + pathSep + "di", []string{dirPath}},
+		{tmpDir + pathSep + "dir", []string{dirPath}},
+		{tmpDir + pathSep + "f", nil},
+		{tmpDir + pathSep + "q", nil},
+	}
+
+	for _, tc := range cases {
+		ret := browseFiles(tc.current, fs.FilesystemTypeBasic)
+		if !equalStrings(ret, tc.returns) {
+			t.Errorf("browseFiles(%q) => %q, expected %q", tc.current, ret, tc.returns)
+		}
+	}
+}
+
+func equalStrings(a, b []string) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i := range a {
+		if a[i] != b[i] {
+			return false
+		}
+	}
+	return true
+}