5 files changed, 22 insertions, 31 deletions
diff --git a/cmd/mattermost/commands/server.go b/cmd/mattermost/commands/server.go
index 299005b6a..20ebfade6 100644
--- a/cmd/mattermost/commands/server.go
+++ b/cmd/mattermost/commands/server.go
@@ -6,7 +6,6 @@ package commands
 import (
 	"fmt"
 	"net"
-	"net/http"
 	"net/url"
 	"os"
 	"os/signal"
@@ -74,6 +73,11 @@ func runServer(configFileLocation string, disableConfigWatch bool, usedPlatform
 	if usedPlatform {
 		mlog.Error("The platform binary has been deprecated, please switch to using the mattermost binary.")
 	}
+
+	if _, err := url.ParseRequestURI(*a.Config().ServiceSettings.SiteURL); err != nil {
+		mlog.Error("SiteURL must be set. Some features will operate incorrectly if the SiteURL is not set. See documentation for details: http://about.mattermost.com/default-site-url")
+	}
+
 	mlog.Info(fmt.Sprintf("Current version is %v (%v/%v/%v/%v)", model.CurrentVersion, model.BuildNumber, model.BuildDate, model.BuildHash, model.BuildHashEnterprise))
 	mlog.Info(fmt.Sprintf("Enterprise Enabled: %v", model.BuildEnterpriseReady))
 	mlog.Info(fmt.Sprintf("Current working directory is %v", pwd))
@@ -131,19 +135,7 @@ func runServer(configFileLocation string, disableConfigWatch bool, usedPlatform
 
 	// Enable developer settings if this is a "dev" build
 	if model.BuildNumber == "dev" {
-		a.UpdateConfig(func(cfg *model.Config) {
-			*cfg.ServiceSettings.EnableDeveloper = true
-			if *cfg.ServiceSettings.SiteURL == "" {
-				*cfg.ServiceSettings.SiteURL = "http://localhost:8065"
-			}
-		})
-	}
-
-	// SiteURL should be set at this point. Either by a user or by the dev mode above
-	// This is here instead of in config.IsValid because there are many tests that make the assumption
-	// that the default config is valid. Which it is not.
-	if _, err := url.ParseRequestURI(*a.Config().ServiceSettings.SiteURL); err != nil {
-		return model.NewAppError("Config.IsValid", "model.config.is_valid.site_url.app_error", nil, "", http.StatusBadRequest)
+		a.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableDeveloper = true })
 	}
 
 	resetStatuses(a)
diff --git a/cmd/mattermost/commands/server_test.go b/cmd/mattermost/commands/server_test.go
index a0c7c6948..0f825e316 100644
--- a/cmd/mattermost/commands/server_test.go
+++ b/cmd/mattermost/commands/server_test.go
@@ -11,7 +11,6 @@ import (
 	"testing"
 
 	"github.com/mattermost/mattermost-server/jobs"
-	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/utils"
 	"github.com/stretchr/testify/require"
 )
@@ -21,7 +20,6 @@ type ServerTestHelper struct {
 	disableConfigWatch bool
 	interruptChan      chan os.Signal
 	originalInterval   int
-	oldBuildNumber     string
 }
 
 func SetupServerTest() *ServerTestHelper {
@@ -43,20 +41,14 @@ func SetupServerTest() *ServerTestHelper {
 		interruptChan:      interruptChan,
 		originalInterval:   originalInterval,
 	}
-
-	// Run in dev mode so SiteURL gets set
-	th.oldBuildNumber = model.BuildNumber
-	model.BuildNumber = "dev"
-
 	return th
 }
 
 func (th *ServerTestHelper) TearDownServerTest() {
 	jobs.DEFAULT_WATCHER_POLLING_INTERVAL = th.originalInterval
-	model.BuildNumber = th.oldBuildNumber
 }
 
-func TestRunServerSiteURL(t *testing.T) {
+func TestRunServerSuccess(t *testing.T) {
 	th := SetupServerTest()
 	defer th.TearDownServerTest()
 
diff --git a/i18n/en.json b/i18n/en.json
index 1f4d55476..e2f67d0ce 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -5260,7 +5260,7 @@
   },
   {
     "id": "model.config.is_valid.site_url.app_error",
-    "translation": "Site URL must be set, a valid URL, and start with http:// or https://"
+    "translation": "Site URL must be a valid URL and start with http:// or https://"
   },
   {
     "id": "model.config.is_valid.site_url_email_batching.app_error",
diff --git a/model/config.go b/model/config.go
index ba3a02d33..d5a8ad38e 100644
--- a/model/config.go
+++ b/model/config.go
@@ -1900,10 +1900,18 @@ func (o *Config) SetDefaults() {
 }
 
 func (o *Config) IsValid() *AppError {
+	if len(*o.ServiceSettings.SiteURL) == 0 && *o.EmailSettings.EnableEmailBatching {
+		return NewAppError("Config.IsValid", "model.config.is_valid.site_url_email_batching.app_error", nil, "", http.StatusBadRequest)
+	}
+
 	if *o.ClusterSettings.Enable && *o.EmailSettings.EnableEmailBatching {
 		return NewAppError("Config.IsValid", "model.config.is_valid.cluster_email_batching.app_error", nil, "", http.StatusBadRequest)
 	}
 
+	if len(*o.ServiceSettings.SiteURL) == 0 && *o.ServiceSettings.AllowCookiesForSubdomains {
+		return NewAppError("Config.IsValid", "Allowing cookies for subdomains requires SiteURL to be set.", nil, "", http.StatusBadRequest)
+	}
+
 	if err := o.TeamSettings.isValid(); err != nil {
 		return err
 	}
@@ -2209,6 +2217,12 @@ func (ss *ServiceSettings) isValid() *AppError {
 		return NewAppError("Config.IsValid", "model.config.is_valid.login_attempts.app_error", nil, "", http.StatusBadRequest)
 	}
 
+	if len(*ss.SiteURL) != 0 {
+		if _, err := url.ParseRequestURI(*ss.SiteURL); err != nil {
+			return NewAppError("Config.IsValid", "model.config.is_valid.site_url.app_error", nil, "", http.StatusBadRequest)
+		}
+	}
+
 	if len(*ss.WebsocketURL) != 0 {
 		if _, err := url.ParseRequestURI(*ss.WebsocketURL); err != nil {
 			return NewAppError("Config.IsValid", "model.config.is_valid.websocket_url.app_error", nil, "", http.StatusBadRequest)
diff --git a/model/config_test.go b/model/config_test.go
index e39ecef3b..b7533145b 100644
--- a/model/config_test.go
+++ b/model/config_test.go
@@ -82,13 +82,6 @@ func TestConfigDefaultFileSettingsS3SSE(t *testing.T) {
 	}
 }
 
-func TestConfigDefaultSiteURL(t *testing.T) {
-	c1 := Config{}
-	c1.SetDefaults()
-
-	assert.Equal(t, "", *c1.ServiceSettings.SiteURL, "SiteURL should be empty by default.")
-}
-
 func TestConfigDefaultServiceSettingsExperimentalGroupUnreadChannels(t *testing.T) {
 	c1 := Config{}
 	c1.SetDefaults()