Option -gui-address should accept scheme prefixes (fixes #2371)

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,11 +120,13 @@ 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
 	}
 
+	l.Infoln("Starting web GUI on", guiCfg.URL())
+
 	listener := &tlsutil.DowngradingListener{rawListener, tlsCfg}
 	return listener, nil
 }
@@ -202,14 +199,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 +213,7 @@ func (s *apiSvc) Serve() {
 	}
 
 	// Redirect to HTTPS if we are supposed to
-	if guiCfg.UseTLS {
+	if guiCfg.UseTLS() {
 		handler = redirectToHTTPSMiddleware(handler)
 	}
 

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
-		}
-
-		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)
+		l.Fatalln("Cannot start GUI:", err)
+	}
+	cfg.Subscribe(api)
+	mainSvc.Add(api)
 
-		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 {

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 {
@@ -463,8 +524,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

@@ -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])
+		}
+	}
+}