From 32bc5854464ffe45da92559018230059dc35cd0e Mon Sep 17 00:00:00 2001 From: Shota Gvinepadze Date: Fri, 20 Sep 2019 17:48:19 +0400 Subject: [PATCH] [MM-18121] Fix config set panic (#12083) * Fix config set panic * Change error message * Add a test case * Update error string --- cmd/mattermost/commands/config.go | 17 +++++++++++------ cmd/mattermost/commands/config_test.go | 6 ++++++ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/cmd/mattermost/commands/config.go b/cmd/mattermost/commands/config.go index 494e45a433..81ed6d7236 100644 --- a/cmd/mattermost/commands/config.go +++ b/cmd/mattermost/commands/config.go @@ -21,6 +21,8 @@ import ( "github.com/mattermost/viper" ) +const noSettingsNamed = "unable to find a setting named: %s" + var ConfigCmd = &cobra.Command{ Use: "config", Short: "Configuration", @@ -296,7 +298,7 @@ func updateConfigValue(configSetting string, newVal []string, oldConfig, newConf func UpdateMap(configMap map[string]interface{}, configSettings []string, newVal []string) error { res, ok := configMap[configSettings[0]] if !ok { - return fmt.Errorf("unable to find a setting with that name %s", configSettings[0]) + return fmt.Errorf(noSettingsNamed, configSettings[0]) } value := reflect.ValueOf(res) @@ -308,6 +310,9 @@ func UpdateMap(configMap map[string]interface{}, configSettings []string, newVal if len(configSettings) == 1 { return errors.New("unable to set multiple settings at once") } + if value.Len() == 0 { + return fmt.Errorf(noSettingsNamed, configSettings[1]) + } return UpdateMap(res.(map[string]interface{}), configSettings[1:], newVal) case reflect.Int: @@ -319,7 +324,7 @@ func UpdateMap(configMap map[string]interface{}, configSettings []string, newVal configMap[configSettings[0]] = val return nil } - return fmt.Errorf("unable to find a setting with that name %s", configSettings[0]) + return fmt.Errorf(noSettingsNamed, configSettings[0]) case reflect.Int64: if len(configSettings) == 1 { @@ -330,7 +335,7 @@ func UpdateMap(configMap map[string]interface{}, configSettings []string, newVal configMap[configSettings[0]] = int64(val) return nil } - return fmt.Errorf("unable to find a setting with that name %s", configSettings[0]) + return fmt.Errorf(noSettingsNamed, configSettings[0]) case reflect.Bool: if len(configSettings) == 1 { @@ -341,21 +346,21 @@ func UpdateMap(configMap map[string]interface{}, configSettings []string, newVal configMap[configSettings[0]] = val return nil } - return fmt.Errorf("unable to find a setting with that name %s", configSettings[0]) + return fmt.Errorf(noSettingsNamed, configSettings[0]) case reflect.String: if len(configSettings) == 1 { configMap[configSettings[0]] = newVal[0] return nil } - return fmt.Errorf("unable to find a setting with that name %s", configSettings[0]) + return fmt.Errorf(noSettingsNamed, configSettings[0]) case reflect.Slice: if len(configSettings) == 1 { configMap[configSettings[0]] = newVal return nil } - return fmt.Errorf("unable to find a setting with that name %s", configSettings[0]) + return fmt.Errorf(noSettingsNamed, configSettings[0]) default: return errors.New("type not supported yet") diff --git a/cmd/mattermost/commands/config_test.go b/cmd/mattermost/commands/config_test.go index d5cb45ccc5..607f4f4a70 100644 --- a/cmd/mattermost/commands/config_test.go +++ b/cmd/mattermost/commands/config_test.go @@ -141,6 +141,12 @@ func TestConfigSet(t *testing.T) { assert.NotContains(t, string(output), "invalid-key") }) + t.Run("Error when the parameter of an unknown plugin is set", func(t *testing.T) { + output, err := th.RunCommandWithOutput(t, "config", "set", "PluginSettings.Plugins.someplugin", "true") + assert.Error(t, err) + assert.NotContains(t, string(output), "panic") + }) + t.Run("Error when the wrong locale is set", func(t *testing.T) { th.CheckCommand(t, "config", "set", "LocalizationSettings.DefaultServerLocale", "es") assert.Error(t, th.RunCommand(t, "config", "set", "LocalizationSettings.DefaultServerLocale", "invalid-key"))