Fix STGUIASSETS search paths & order (fixes #2827)

cmd/syncthing/gui.go

@@ -88,6 +88,7 @@ func newAPIService(id protocol.DeviceID, cfg *config.Wrapper, assetDir string, m
 	}
 
 	seen := make(map[string]struct{})
+	// Load themes from compiled in assets.
 	for file := range auto.Assets() {
 		theme := strings.Split(file, "/")[0]
 		if _, ok := seen[theme]; !ok {
@@ -95,6 +96,15 @@ func newAPIService(id protocol.DeviceID, cfg *config.Wrapper, assetDir string, m
 			service.themes = append(service.themes, theme)
 		}
 	}
+	if assetDir != "" {
+		// Load any extra themes from the asset override dir.
+		for _, dir := range dirNames(assetDir) {
+			if _, ok := seen[dir]; !ok {
+				seen[dir] = struct{}{}
+				service.themes = append(service.themes, dir)
+			}
+		}
+	}
 
 	var err error
 	service.listener, err = service.getListener(cfg.GUI())
@@ -1124,22 +1134,33 @@ func (s embeddedStatic) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		file = "index.html"
 	}
 
-	if s.assetDir != "" {
-		p := filepath.Join(s.assetDir, filepath.FromSlash(file))
-		_, err := os.Stat(p)
-		if err == nil {
-			http.ServeFile(w, r, p)
-			return
-		}
-	}
-
 	s.mut.RLock()
 	theme := s.theme
 	modified := s.lastModified
 	s.mut.RUnlock()
 
+	// Check for an override for the current theme.
+	if s.assetDir != "" {
+		p := filepath.Join(s.assetDir, s.theme, filepath.FromSlash(file))
+		if _, err := os.Stat(p); err == nil {
+			http.ServeFile(w, r, p)
+			return
+		}
+	}
+
+	// Check for a compiled in asset for the current theme.
 	bs, ok := s.assets[theme+"/"+file]
 	if !ok {
+		// Check for an overriden default asset.
+		if s.assetDir != "" {
+			p := filepath.Join(s.assetDir, config.DefaultTheme, filepath.FromSlash(file))
+			if _, err := os.Stat(p); err == nil {
+				http.ServeFile(w, r, p)
+				return
+			}
+		}
+
+		// Check for a compiled in default asset.
 		bs, ok = s.assets[config.DefaultTheme+"/"+file]
 		if !ok {
 			http.NotFound(w, r)
@@ -1266,3 +1287,26 @@ func (v jsonVersionVector) MarshalJSON() ([]byte, error) {
 	}
 	return json.Marshal(res)
 }
+
+func dirNames(dir string) []string {
+	fd, err := os.Open(dir)
+	if err != nil {
+		return nil
+	}
+	defer fd.Close()
+
+	fis, err := fd.Readdir(-1)
+	if err != nil {
+		return nil
+	}
+
+	var dirs []string
+	for _, fi := range fis {
+		if fi.IsDir() {
+			dirs = append(dirs, filepath.Base(fi.Name()))
+		}
+	}
+
+	sort.Strings(dirs)
+	return dirs
+}

cmd/syncthing/gui_test.go

@@ -7,10 +7,17 @@
 package main
 
 import (
+	"bytes"
+	"compress/gzip"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
 	"testing"
 
+	"github.com/d4l3k/messagediff"
 	"github.com/syncthing/syncthing/lib/config"
 	"github.com/syncthing/syncthing/lib/protocol"
+	"github.com/syncthing/syncthing/lib/sync"
 	"github.com/thejerf/suture"
 )
 
@@ -89,3 +96,85 @@ func TestStopAfterBrokenConfig(t *testing.T) {
 
 	sup.Stop()
 }
+
+func TestAssetsDir(t *testing.T) {
+	// For any given request to $FILE, we should return the first found of
+	//  - assetsdir/$THEME/$FILE
+	//  - compiled in asset $THEME/$FILE
+	//  - assetsdir/default/$FILE
+	//  - compiled in asset default/$FILE
+
+	// The asset map contains compressed assets, so create a couple of gzip compressed assets here.
+	buf := new(bytes.Buffer)
+	gw := gzip.NewWriter(buf)
+	gw.Write([]byte("default"))
+	gw.Close()
+	def := buf.Bytes()
+
+	buf = new(bytes.Buffer)
+	gw = gzip.NewWriter(buf)
+	gw.Write([]byte("foo"))
+	gw.Close()
+	foo := buf.Bytes()
+
+	e := embeddedStatic{
+		theme:    "foo",
+		mut:      sync.NewRWMutex(),
+		assetDir: "testdata",
+		assets: map[string][]byte{
+			"foo/a":     foo, // overridden in foo/a
+			"foo/b":     foo,
+			"default/a": def, // overridden in default/a (but foo/a takes precedence)
+			"default/b": def, // overridden in default/b (but foo/b takes precedence)
+			"default/c": def,
+		},
+	}
+
+	s := httptest.NewServer(e)
+	defer s.Close()
+
+	// assetsdir/foo/a exists, overrides compiled in
+	expectURLToContain(t, s.URL+"/a", "overridden-foo")
+
+	// foo/b is compiled in, default/b is overriden, return compiled in
+	expectURLToContain(t, s.URL+"/b", "foo")
+
+	// only exists as compiled in default/c so use that
+	expectURLToContain(t, s.URL+"/c", "default")
+
+	// only exists as overriden default/d so use that
+	expectURLToContain(t, s.URL+"/d", "overridden-default")
+}
+
+func expectURLToContain(t *testing.T, url, exp string) {
+	res, err := http.Get(url)
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	if res.StatusCode != 200 {
+		t.Errorf("Got %s instead of 200 OK", res.Status)
+		return
+	}
+
+	data, err := ioutil.ReadAll(res.Body)
+	res.Body.Close()
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	if string(data) != exp {
+		t.Errorf("Got %q instead of %q on %q", data, exp, url)
+		return
+	}
+}
+
+func TestDirNames(t *testing.T) {
+	names := dirNames("testdata")
+	expected := []string{"default", "foo", "testfolder"}
+	if diff, equal := messagediff.PrettyDiff(expected, names); !equal {
+		t.Errorf("Unexpected dirNames return: %#v\n%s", names, diff)
+	}
+}

cmd/syncthing/testdata/default/a

@@ -0,0 +1 @@
+overridden-default

cmd/syncthing/testdata/default/b

@@ -0,0 +1 @@
+overridden-default

cmd/syncthing/testdata/default/d

@@ -0,0 +1 @@
+overridden-default

cmd/syncthing/testdata/foo/a

@@ -0,0 +1 @@
+overridden-foo