From 334cf9d84a11a382be93fb6ffd577031e1a33f94 Mon Sep 17 00:00:00 2001 From: Claudio Costa Date: Thu, 17 Sep 2020 20:16:59 +0200 Subject: [PATCH] [MM-28131] Fix desanitization of DataSourceReplicas and DataSourceSearchReplicas (#15389) * Fix desanitization of DataSourceReplicas and DataSourceSearchReplicas * Fix test case Co-authored-by: Mattermod --- api4/config.go | 2 +- api4/config_test.go | 6 ++---- config/migrate_test.go | 6 ++++++ config/utils.go | 18 ++++++++++++------ config/utils_test.go | 4 ++-- 5 files changed, 23 insertions(+), 13 deletions(-) diff --git a/api4/config.go b/api4/config.go index be6e24daf3..52a0df9e12 100644 --- a/api4/config.go +++ b/api4/config.go @@ -210,7 +210,7 @@ func patchConfig(c *Context, w http.ResponseWriter, r *http.Request) { } appCfg := c.App.Config() - if *appCfg.ServiceSettings.SiteURL != "" && (cfg.ServiceSettings.SiteURL == nil || *cfg.ServiceSettings.SiteURL == "") { + if *appCfg.ServiceSettings.SiteURL != "" && cfg.ServiceSettings.SiteURL != nil && *cfg.ServiceSettings.SiteURL == "" { c.Err = model.NewAppError("patchConfig", "api.config.update_config.clear_siteurl.app_error", nil, "", http.StatusBadRequest) return } diff --git a/api4/config_test.go b/api4/config_test.go index d7d12e4a55..75d71b2cb0 100644 --- a/api4/config_test.go +++ b/api4/config_test.go @@ -670,11 +670,9 @@ func TestPatchConfig(t *testing.T) { CheckNoError(t, resp) require.Equal(t, nonEmptyURL, *cfg.ServiceSettings.SiteURL) - // Check that sending an empty config returns an error. + // Check that sending an empty config returns no error. _, resp = th.SystemAdminClient.PatchConfig(&model.Config{}) - require.NotNil(t, resp.Error) - CheckBadRequestStatus(t, resp) - assert.Equal(t, "api.config.update_config.clear_siteurl.app_error", resp.Error.Id) + CheckNoError(t, resp) }) } diff --git a/config/migrate_test.go b/config/migrate_test.go index 974cffa14a..033959e808 100644 --- a/config/migrate_test.go +++ b/config/migrate_test.go @@ -58,6 +58,12 @@ func TestMigrate(t *testing.T) { files[3], files[4], } + cfg.SqlSettings.DataSourceReplicas = []string{ + "mysql://mmuser:password@tcp(replicahost:3306)/mattermost", + } + cfg.SqlSettings.DataSourceSearchReplicas = []string{ + "mysql://mmuser:password@tcp(searchreplicahost:3306)/mattermost", + } _, err := source.Set(cfg) require.NoError(t, err) diff --git a/config/utils.go b/config/utils.go index 117c06d02e..33debde72a 100644 --- a/config/utils.go +++ b/config/utils.go @@ -52,14 +52,20 @@ func desanitize(actual, target *model.Config) { *target.ElasticsearchSettings.Password = *actual.ElasticsearchSettings.Password } - target.SqlSettings.DataSourceReplicas = make([]string, len(actual.SqlSettings.DataSourceReplicas)) - for i := range target.SqlSettings.DataSourceReplicas { - target.SqlSettings.DataSourceReplicas[i] = actual.SqlSettings.DataSourceReplicas[i] + if len(target.SqlSettings.DataSourceReplicas) == len(actual.SqlSettings.DataSourceReplicas) { + for i, value := range target.SqlSettings.DataSourceReplicas { + if value == model.FAKE_SETTING { + target.SqlSettings.DataSourceReplicas[i] = actual.SqlSettings.DataSourceReplicas[i] + } + } } - target.SqlSettings.DataSourceSearchReplicas = make([]string, len(actual.SqlSettings.DataSourceSearchReplicas)) - for i := range target.SqlSettings.DataSourceSearchReplicas { - target.SqlSettings.DataSourceSearchReplicas[i] = actual.SqlSettings.DataSourceSearchReplicas[i] + if len(target.SqlSettings.DataSourceSearchReplicas) == len(actual.SqlSettings.DataSourceSearchReplicas) { + for i, value := range target.SqlSettings.DataSourceSearchReplicas { + if value == model.FAKE_SETTING { + target.SqlSettings.DataSourceSearchReplicas[i] = actual.SqlSettings.DataSourceSearchReplicas[i] + } + } } if *target.MessageExportSettings.GlobalRelaySettings.SmtpPassword == model.FAKE_SETTING { diff --git a/config/utils_test.go b/config/utils_test.go index 50b1b766dd..fbb5667bec 100644 --- a/config/utils_test.go +++ b/config/utils_test.go @@ -49,8 +49,8 @@ func TestDesanitize(t *testing.T) { target.SqlSettings.DataSource = sToP(model.FAKE_SETTING) target.SqlSettings.AtRestEncryptKey = sToP(model.FAKE_SETTING) target.ElasticsearchSettings.Password = sToP(model.FAKE_SETTING) - target.SqlSettings.DataSourceReplicas = append(target.SqlSettings.DataSourceReplicas, "old_replica0") - target.SqlSettings.DataSourceSearchReplicas = append(target.SqlSettings.DataSourceReplicas, "old_search_replica0") + target.SqlSettings.DataSourceReplicas = []string{model.FAKE_SETTING, model.FAKE_SETTING} + target.SqlSettings.DataSourceSearchReplicas = []string{model.FAKE_SETTING, model.FAKE_SETTING} actualClone := actual.Clone() desanitize(actual, target)