Fix various segfaults when running go test manually (#8448)

* failing to find i18n shouldn't segfault

The server was trying to handle the fact that it couldn't find the i18n
directory, by emitting a translated log message...

* fix utils.FindDir

The attempts to find the directory in the parent or grandparent directory
don't work if the current working directory was inside `enterprise`, with
`enterprise` itself being a symlink as per the usual developer setup.

Recurse to the root of the filesystem, cleaning the path along the way
to work around this limitation (and allow tests to be run from an
arbitrarily deep nesting level.)

Fix corresponding usages to employ filepath.Join.

* failing to find html templates shouldn't segfault

* fail fast if the test user cannot be created

* rework utils.FindDir to retain backwards compatibility

api/file_test.go

@@ -10,6 +10,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 	"time"
@@ -870,7 +871,7 @@ func TestGetInfoForFilename(t *testing.T) {
 
 func readTestFile(name string) ([]byte, error) {
 	path, _ := utils.FindDir("tests")
-	file, err := os.Open(path + "/" + name)
+	file, err := os.Open(filepath.Join(path, name))
 	if err != nil {
 		return nil, err
 	}

api/user_test.go

@@ -12,6 +12,7 @@ import (
 	"mime/multipart"
 	"net/http"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 	"time"
@@ -778,7 +779,7 @@ func TestUserUploadProfileImage(t *testing.T) {
 		}
 
 		path, _ := utils.FindDir("tests")
-		file, err := os.Open(path + "/test.png")
+		file, err := os.Open(filepath.Join(path, "test.png"))
 		if err != nil {
 			t.Fatal(err)
 		}

api4/apitestlib.go

@@ -11,6 +11,7 @@ import (
 	"net"
 	"net/http"
 	"os"
+	"path/filepath"
 	"reflect"
 	"strconv"
 	"strings"
@@ -302,7 +303,11 @@ func (me *TestHelper) CreateUserWithClient(client *model.Client4) *model.User {
 	}
 
 	utils.DisableDebugLogForTest()
-	ruser, _ := client.CreateUser(user)
+	ruser, response := client.CreateUser(user)
+	if response.Error != nil {
+		panic(response.Error)
+	}
+
 	ruser.Password = "Password1"
 	store.Must(me.App.Srv.Store.User().VerifyEmail(ruser.Id))
 	utils.EnableDebugLogForTest()
@@ -675,7 +680,7 @@ func CheckInternalErrorStatus(t *testing.T, resp *model.Response) {
 
 func readTestFile(name string) ([]byte, error) {
 	path, _ := utils.FindDir("tests")
-	file, err := os.Open(path + "/" + name)
+	file, err := os.Open(filepath.Join(path, name))
 	if err != nil {
 		return nil, err
 	}

api4/oauth.go

@@ -6,6 +6,7 @@ package api4
 import (
 	"net/http"
 	"net/url"
+	"path/filepath"
 	"strings"
 
 	l4g "github.com/alecthomas/log4go"
@@ -375,7 +376,7 @@ func authorizeOAuthPage(c *Context, w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Cache-Control", "no-cache, max-age=31556926, public")
 
 	staticDir, _ := utils.FindDir(model.CLIENT_DIR)
-	http.ServeFile(w, r, staticDir+"root.html")
+	http.ServeFile(w, r, filepath.Join(staticDir, "root.html"))
 }
 
 func getAccessToken(c *Context, w http.ResponseWriter, r *http.Request) {

api4/plugin_test.go

@@ -8,6 +8,7 @@ import (
 	"encoding/json"
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/mattermost/mattermost-server/model"
@@ -53,7 +54,7 @@ func TestPlugin(t *testing.T) {
 	}()
 
 	path, _ := utils.FindDir("tests")
-	file, err := os.Open(path + "/testplugin.tar.gz")
+	file, err := os.Open(filepath.Join(path, "testplugin.tar.gz"))
 	if err != nil {
 		t.Fatal(err)
 	}

app/app.go

@@ -439,9 +439,13 @@ func (a *App) WaitForGoroutines() {
 }
 
 func (a *App) HTMLTemplates() *template.Template {
+	if a.htmlTemplateWatcher != nil {
 		return a.htmlTemplateWatcher.Templates()
 	}
 
+	return nil
+}
+
 func (a *App) HTTPClient(trustURLs bool) *http.Client {
 	insecure := a.Config().ServiceSettings.EnableInsecureOutgoingConnections != nil && *a.Config().ServiceSettings.EnableInsecureOutgoingConnections
 

app/auto_posts.go

@@ -7,6 +7,7 @@ import (
 	"bytes"
 	"io"
 	"os"
+	"path/filepath"
 
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/utils"
@@ -43,7 +44,7 @@ func (cfg *AutoPostCreator) UploadTestFile() ([]string, bool) {
 	filename := cfg.ImageFilenames[utils.RandIntFromRange(utils.Range{Begin: 0, End: len(cfg.ImageFilenames) - 1})]
 
 	path, _ := utils.FindDir("web/static/images")
-	file, err := os.Open(path + "/" + filename)
+	file, err := os.Open(filepath.Join(path, filename))
 	if err != nil {
 		return nil, false
 	}

app/import_test.go

@@ -4,6 +4,7 @@
 package app
 
 import (
+	"path/filepath"
 	"runtime/debug"
 	"strings"
 	"testing"
@@ -370,7 +371,7 @@ func TestImportValidateUserImportData(t *testing.T) {
 	// Test a valid User with all fields populated.
 	testsDir, _ := utils.FindDir("tests")
 	data = UserImportData{
-		ProfileImage: ptrStr(testsDir + "test.png"),
+		ProfileImage: ptrStr(filepath.Join(testsDir, "test.png")),
 		Username:     ptrStr("bob"),
 		Email:        ptrStr("bob@example.com"),
 		AuthService:  ptrStr("ldap"),
@@ -1487,7 +1488,7 @@ func TestImportImportUser(t *testing.T) {
 	username := model.NewId()
 	testsDir, _ := utils.FindDir("tests")
 	data = UserImportData{
-		ProfileImage: ptrStr(testsDir + "test.png"),
+		ProfileImage: ptrStr(filepath.Join(testsDir, "test.png")),
 		Username:     &username,
 		Email:        ptrStr(model.NewId() + "@example.com"),
 		Nickname:     ptrStr(model.NewId()),
@@ -1543,7 +1544,7 @@ func TestImportImportUser(t *testing.T) {
 
 	// Alter all the fields of that user.
 	data.Email = ptrStr(model.NewId() + "@example.com")
-	data.ProfileImage = ptrStr(testsDir + "testgif.gif")
+	data.ProfileImage = ptrStr(filepath.Join(testsDir, "testgif.gif"))
 	data.AuthService = ptrStr("ldap")
 	data.AuthData = &username
 	data.Nickname = ptrStr(model.NewId())

app/saml.go

@@ -8,7 +8,6 @@ import (
 	"mime/multipart"
 	"net/http"
 	"os"
-
 	"path/filepath"
 
 	"github.com/mattermost/mattermost-server/model"
@@ -42,7 +41,7 @@ func WriteSamlFile(fileData *multipart.FileHeader) *model.AppError {
 	defer file.Close()
 
 	configDir, _ := utils.FindDir("config")
-	out, err := os.Create(configDir + filename)
+	out, err := os.Create(filepath.Join(configDir, filename))
 	if err != nil {
 		return model.NewAppError("AddSamlCertificate", "api.admin.add_certificate.saving.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}

app/user.go

@@ -18,6 +18,7 @@ import (
 	"io/ioutil"
 	"mime/multipart"
 	"net/http"
+	"path/filepath"
 	"strconv"
 	"strings"
 
@@ -717,7 +718,7 @@ func CreateProfileImage(username string, userId string, initialFont string) ([]b
 	initial := string(strings.ToUpper(username)[0])
 
 	fontDir, _ := utils.FindDir("fonts")
-	fontBytes, err := ioutil.ReadFile(fontDir + initialFont)
+	fontBytes, err := ioutil.ReadFile(filepath.Join(fontDir, initialFont))
 	if err != nil {
 		return nil, model.NewAppError("CreateProfileImage", "api.user.create_profile_image.default_font.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}

utils/config.go

@@ -9,7 +9,6 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
-	"path"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -51,21 +50,17 @@ func FindConfigFile(fileName string) (path string) {
 	return ""
 }
 
+// FindDir looks for the given directory in nearby ancestors, falling back to `./` if not found.
 func FindDir(dir string) (string, bool) {
-	fileName := "."
+	for _, parent := range []string{".", "..", "../.."} {
-	found := false
+		foundDir, err := filepath.Abs(filepath.Join(parent, dir))
-	if _, err := os.Stat("./" + dir + "/"); err == nil {
+		if err != nil {
-		fileName, _ = filepath.Abs("./" + dir + "/")
+			continue
-		found = true
+		} else if _, err := os.Stat(foundDir); err == nil {
-	} else if _, err := os.Stat("../" + dir + "/"); err == nil {
+			return foundDir, true
-		fileName, _ = filepath.Abs("../" + dir + "/")
-		found = true
-	} else if _, err := os.Stat("../../" + dir + "/"); err == nil {
-		fileName, _ = filepath.Abs("../../" + dir + "/")
-		found = true
 		}
-
+	}
-	return fileName + "/", found
+	return "./", false
 }
 
 func DisableDebugLogForTest() {
@@ -136,11 +131,10 @@ func ConfigureLog(s *model.LogSettings) {
 
 func GetLogFileLocation(fileLocation string) string {
 	if fileLocation == "" {
-		logDir, _ := FindDir("logs")
+		fileLocation, _ = FindDir("logs")
-		return logDir + LOG_FILENAME
-	} else {
-		return path.Join(fileLocation, LOG_FILENAME)
 	}
+
+	return filepath.Join(fileLocation, LOG_FILENAME)
 }
 
 func SaveConfig(fileName string, config *model.Config) *model.AppError {

utils/html.go

@@ -5,8 +5,10 @@ package utils
 
 import (
 	"bytes"
+	"errors"
 	"html/template"
 	"io"
+	"path/filepath"
 	"reflect"
 	"sync/atomic"
 
@@ -39,7 +41,7 @@ func NewHTMLTemplateWatcher(directory string) (*HTMLTemplateWatcher, error) {
 		return nil, err
 	}
 
-	if htmlTemplates, err := template.ParseGlob(templatesDir + "*.html"); err != nil {
+	if htmlTemplates, err := template.ParseGlob(filepath.Join(templatesDir, "*.html")); err != nil {
 		return nil, err
 	} else {
 		ret.templates.Store(htmlTemplates)
@@ -56,7 +58,7 @@ func NewHTMLTemplateWatcher(directory string) (*HTMLTemplateWatcher, error) {
 			case event := <-watcher.Events:
 				if event.Op&fsnotify.Write == fsnotify.Write {
 					l4g.Info("Re-parsing templates because of modified file %v", event.Name)
-					if htmlTemplates, err := template.ParseGlob(templatesDir + "*.html"); err != nil {
+					if htmlTemplates, err := template.ParseGlob(filepath.Join(templatesDir, "*.html")); err != nil {
 						l4g.Error("Failed to parse templates %v", err)
 					} else {
 						ret.templates.Store(htmlTemplates)
@@ -103,6 +105,10 @@ func (t *HTMLTemplate) Render() string {
 }
 
 func (t *HTMLTemplate) RenderToWriter(w io.Writer) error {
+	if t.Templates == nil {
+		return errors.New("no html templates")
+	}
+
 	if err := t.Templates.ExecuteTemplate(w, t.TemplateName, t); err != nil {
 		l4g.Error(T("api.api.render.error"), t.TemplateName, err)
 		return err

utils/i18n.go

@@ -23,13 +23,15 @@ var settings model.LocalizationSettings
 // this functions loads translations from filesystem
 // and assign english while loading server config
 func TranslationsPreInit() error {
+	// Set T even if we fail to load the translations. Lots of shutdown handling code will
+	// segfault trying to handle the error, and the untranslated IDs are strictly better.
+	T = TfuncWithFallback("en")
+	TDefault = TfuncWithFallback("en")
+
 	if err := InitTranslationsWithDir("i18n"); err != nil {
 		return err
 	}
 
-	T = TfuncWithFallback("en")
-	TDefault = TfuncWithFallback("en")
-
 	return nil
 }
 
@@ -51,9 +53,9 @@ func InitTranslationsWithDir(dir string) error {
 	for _, f := range files {
 		if filepath.Ext(f.Name()) == ".json" {
 			filename := f.Name()
-			locales[strings.Split(filename, ".")[0]] = i18nDirectory + filename
+			locales[strings.Split(filename, ".")[0]] = filepath.Join(i18nDirectory, filename)
 
-			if err := i18n.LoadTranslationFile(i18nDirectory + filename); err != nil {
+			if err := i18n.LoadTranslationFile(filepath.Join(i18nDirectory, filename)); err != nil {
 				return err
 			}
 		}

utils/license.go

@@ -12,6 +12,7 @@ import (
 	"encoding/pem"
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"strconv"
 	"strings"
 
@@ -114,7 +115,7 @@ func GetLicenseFileFromDisk(fileName string) []byte {
 func GetLicenseFileLocation(fileLocation string) string {
 	if fileLocation == "" {
 		configDir, _ := FindDir("config")
-		return configDir + "mattermost.mattermost-license"
+		return filepath.Join(configDir, "mattermost.mattermost-license")
 	} else {
 		return fileLocation
 	}

web/web.go

@@ -5,6 +5,7 @@ package web
 
 import (
 	"net/http"
+	"path/filepath"
 	"strings"
 
 	"github.com/NYTimes/gziphandler"
@@ -102,5 +103,5 @@ func root(c *api.Context, w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Cache-Control", "no-cache, max-age=31556926, public")
 
 	staticDir, _ := utils.FindDir(model.CLIENT_DIR)
-	http.ServeFile(w, r, staticDir+"root.html")
+	http.ServeFile(w, r, filepath.Join(staticDir, "root.html"))
 }