[MM-18625] Make config.DatabaseStore.String() more robust (#12309)

config/database.go

@@ -7,8 +7,6 @@ import (
 	"bytes"
 	"database/sql"
 	"io/ioutil"
-	"net/url"
-	"regexp"
 	"strings"
 
 	"github.com/jmoiron/sqlx"
@@ -23,8 +21,6 @@ import (
 	_ "github.com/lib/pq"
 )
 
-var tcpStripper = regexp.MustCompile(`@tcp\((.*)\)`)
-
 // DatabaseStore is a config store backed by a database.
 type DatabaseStore struct {
 	commonStore
@@ -295,16 +291,7 @@ func (ds *DatabaseStore) RemoveFile(name string) error {
 
 // String returns the path to the database backing the config, masking the password.
 func (ds *DatabaseStore) String() string {
-	// Remove @tcp and the parentheses from the host and parse the rest as a URL
+	return stripPassword(ds.originalDsn, ds.driverName)
-	u, err := url.Parse(tcpStripper.ReplaceAllString(ds.originalDsn, `@$1`))
-	if err != nil {
-		return "(omitted due to error parsing the DSN)"
-	}
-
-	// Strip out the password to avoid leaking in logs.
-	u.User = url.User(u.User.Username())
-
-	return u.String()
 }
 
 // Close cleans up resources associated with the store.

config/utils.go

@@ -140,3 +140,24 @@ func Merge(cfg *model.Config, patch *model.Config, mergeConfig *utils.MergeConfi
 	retCfg := ret.(model.Config)
 	return &retCfg, nil
 }
+
+// stripPassword remove the password from a given DSN
+func stripPassword(dsn, schema string) string {
+	prefix := schema + "://"
+	dsn = strings.TrimPrefix(dsn, prefix)
+
+	i := strings.Index(dsn, ":")
+	j := strings.LastIndex(dsn, "@")
+
+	// Return error if no @ sign is found
+	if j < 0 {
+		return "(omitted due to error parsing the DSN)"
+	}
+
+	// Return back the input if no password is found
+	if i < 0 || i > j {
+		return prefix + dsn
+	}
+
+	return prefix + dsn[:i+1] + dsn[j:]
+}

config/utils_test.go

@@ -142,6 +142,61 @@ func TestFixInvalidLocales(t *testing.T) {
 	assert.Contains(t, *cfg.LocalizationSettings.AvailableLocales, *cfg.LocalizationSettings.DefaultClientLocale, "DefaultClientLocale should have been added to AvailableLocales")
 }
 
+func TestStripPassword(t *testing.T) {
+	for name, test := range map[string]struct {
+		DSN         string
+		Schema      string
+		ExpectedOut string
+	}{
+		"mysql": {
+			DSN:         "mysql://mmuser:password@tcp(localhost:3306)/mattermost?charset=utf8mb4,utf8&readTimeout=30s",
+			Schema:      "mysql",
+			ExpectedOut: "mysql://mmuser:@tcp(localhost:3306)/mattermost?charset=utf8mb4,utf8&readTimeout=30s",
+		},
+		"mysql idempotent": {
+			DSN:         "mysql://mmuser:@tcp(localhost:3306)/mattermost?charset=utf8mb4,utf8&readTimeout=30s",
+			Schema:      "mysql",
+			ExpectedOut: "mysql://mmuser:@tcp(localhost:3306)/mattermost?charset=utf8mb4,utf8&readTimeout=30s",
+		},
+		"mysql: password with : and @": {
+			DSN:         "mysql://mmuser:p:assw@ord@tcp(localhost:3306)/mattermost?charset=utf8mb4,utf8&readTimeout=30s",
+			Schema:      "mysql",
+			ExpectedOut: "mysql://mmuser:@tcp(localhost:3306)/mattermost?charset=utf8mb4,utf8&readTimeout=30s",
+		},
+		"mysql: password with @ and :": {
+			DSN:         "mysql://mmuser:pa@sswo:rd@tcp(localhost:3306)/mattermost?charset=utf8mb4,utf8&readTimeout=30s",
+			Schema:      "mysql",
+			ExpectedOut: "mysql://mmuser:@tcp(localhost:3306)/mattermost?charset=utf8mb4,utf8&readTimeout=30s",
+		},
+		"postgres": {
+			DSN:         "postgres://mmuser:password@localhost:5432/mattermost?sslmode=disable&connect_timeout=10",
+			Schema:      "postgres",
+			ExpectedOut: "postgres://mmuser:@localhost:5432/mattermost?sslmode=disable&connect_timeout=10",
+		},
+		"pipe": {
+			DSN:         "mysql://user@unix(/path/to/socket)/dbname",
+			Schema:      "mysql",
+			ExpectedOut: "mysql://user@unix(/path/to/socket)/dbname",
+		},
+		"malformed without :": {
+			DSN:         "postgres://mmuserpassword@localhost:5432/mattermost?sslmode=disable&connect_timeout=10",
+			Schema:      "postgres",
+			ExpectedOut: "postgres://mmuserpassword@localhost:5432/mattermost?sslmode=disable&connect_timeout=10",
+		},
+		"malformed without @": {
+			DSN:         "postgres://mmuser:passwordlocalhost:5432/mattermost?sslmode=disable&connect_timeout=10",
+			Schema:      "postgres",
+			ExpectedOut: "(omitted due to error parsing the DSN)",
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			out := stripPassword(test.DSN, test.Schema)
+
+			assert.Equal(t, test.ExpectedOut, out)
+		})
+	}
+}
+
 func sToP(s string) *string {
 	return &s
 }