From 3d5cf06237535774f98e41dd78ebf462b446032c Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Tue, 27 May 2025 10:03:12 +0530 Subject: [PATCH] [AI assisted] MM-62927: add comprehensive whitespace validation tests for FileSettings paths (#31087) https://mattermost.atlassian.net/browse/MM-62927 ```release-note NONE ``` Co-authored-by: Claude Co-authored-by: Mattermost Build --- server/i18n/en.json | 4 ++ server/public/model/config.go | 17 ++++++ server/public/model/config_test.go | 88 ++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+) diff --git a/server/i18n/en.json b/server/i18n/en.json index dd3713e705..02105487ec 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -9152,6 +9152,10 @@ "id": "model.config.is_valid.directory.app_error", "translation": "Invalid Local Storage Directory. Must be a non-empty string." }, + { + "id": "model.config.is_valid.directory_whitespace.app_error", + "translation": "Leading or trailing whitespace detected for {{.Setting}}. Found \"{{.Value}}\"." + }, { "id": "model.config.is_valid.display.custom_url_schemes.app_error", "translation": "The custom URL scheme {{.Scheme}} is invalid. Custom URL schemes must start with a letter and contain only letters, numbers, plus (+), period (.) and hyphen (-)." diff --git a/server/public/model/config.go b/server/public/model/config.go index 8e201ceb5f..638eaa7ffc 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -4129,6 +4129,11 @@ func (s *FileSettings) isValid() *AppError { return NewAppError("Config.IsValid", "model.config.is_valid.directory.app_error", nil, "", http.StatusBadRequest) } + // Check for leading/trailing whitespace in directory path + if strings.TrimSpace(*s.Directory) != *s.Directory { + return NewAppError("Config.IsValid", "model.config.is_valid.directory_whitespace.app_error", map[string]any{"Setting": "FileSettings.Directory", "Value": *s.Directory}, "", http.StatusBadRequest) + } + if *s.MaxImageDecoderConcurrency < -1 || *s.MaxImageDecoderConcurrency == 0 { return NewAppError("Config.IsValid", "model.config.is_valid.image_decoder_concurrency.app_error", map[string]any{"Value": *s.MaxImageDecoderConcurrency}, "", http.StatusBadRequest) } @@ -4141,10 +4146,22 @@ func (s *FileSettings) isValid() *AppError { return NewAppError("Config.IsValid", "model.config.is_valid.storage_class.app_error", map[string]any{"Value": *s.AmazonS3StorageClass}, "", http.StatusBadRequest) } + if strings.TrimSpace(*s.AmazonS3PathPrefix) != *s.AmazonS3PathPrefix { + return NewAppError("Config.IsValid", "model.config.is_valid.directory_whitespace.app_error", map[string]any{"Setting": "FileSettings.AmazonS3PathPrefix", "Value": *s.AmazonS3PathPrefix}, "", http.StatusBadRequest) + } + if *s.ExportAmazonS3StorageClass != "" && !slices.Contains([]string{StorageClassStandard, StorageClassReducedRedundancy, StorageClassStandardIA, StorageClassOnezoneIA, StorageClassIntelligentTiering, StorageClassGlacier, StorageClassDeepArchive, StorageClassOutposts, StorageClassGlacierIR, StorageClassSnow, StorageClassExpressOnezone}, *s.ExportAmazonS3StorageClass) { return NewAppError("Config.IsValid", "model.config.is_valid.storage_class.app_error", map[string]any{"Value": *s.ExportAmazonS3StorageClass}, "", http.StatusBadRequest) } + if strings.TrimSpace(*s.ExportAmazonS3PathPrefix) != *s.ExportAmazonS3PathPrefix { + return NewAppError("Config.IsValid", "model.config.is_valid.directory_whitespace.app_error", map[string]any{"Setting": "FileSettings.ExportAmazonS3PathPrefix", "Value": *s.ExportAmazonS3PathPrefix}, "", http.StatusBadRequest) + } + + if strings.TrimSpace(*s.ExportDirectory) != *s.ExportDirectory { + return NewAppError("Config.IsValid", "model.config.is_valid.directory_whitespace.app_error", map[string]any{"Setting": "FileSettings.ExportDirectory", "Value": *s.ExportDirectory}, "", http.StatusBadRequest) + } + return nil } diff --git a/server/public/model/config_test.go b/server/public/model/config_test.go index cabc731aa6..1edd4e5759 100644 --- a/server/public/model/config_test.go +++ b/server/public/model/config_test.go @@ -207,6 +207,94 @@ func TestConfigDefaultFileSettingsS3SSE(t *testing.T) { require.False(t, *c1.FileSettings.AmazonS3SSE) } +func TestFileSettingsDirectoryWhitespaceValidation(t *testing.T) { + // Define Unicode whitespace characters to test + unicodeWhitespaces := []struct { + name string + char string + }{ + {"Regular Space", " "}, + {"Standard Space (U+0020)", "\u0020"}, + {"No-Break Space (U+00A0)", "\u00A0"}, + {"En Space (U+2002)", "\u2002"}, + } + + // Define all FileSettings path fields to test + pathSettings := []struct { + name string + validValue string + configSetter func(*Config, *string) + }{ + { + "Directory", + "/path/to/directory", + func(cfg *Config, value *string) { cfg.FileSettings.Directory = value }, + }, + { + "AmazonS3PathPrefix", + "files/", + func(cfg *Config, value *string) { cfg.FileSettings.AmazonS3PathPrefix = value }, + }, + { + "ExportAmazonS3PathPrefix", + "exports/", + func(cfg *Config, value *string) { cfg.FileSettings.ExportAmazonS3PathPrefix = value }, + }, + { + "ExportDirectory", + "/path/to/exports", + func(cfg *Config, value *string) { cfg.FileSettings.ExportDirectory = value }, + }, + } + + // Test valid paths first + for _, setting := range pathSettings { + t.Run(fmt.Sprintf("Valid %s", setting.name), func(t *testing.T) { + cfg := &Config{} + cfg.SetDefaults() + setting.configSetter(cfg, NewPointer(setting.validValue)) + + err := cfg.FileSettings.isValid() + require.Nil(t, err, "Expected no error but got: %v", err) + }) + } + + // Test path with space in the middle (should be valid) + t.Run("Directory with space in the middle (valid)", func(t *testing.T) { + cfg := &Config{} + cfg.SetDefaults() + cfg.FileSettings.Directory = NewPointer("/path/to/my directory") + + err := cfg.FileSettings.isValid() + require.Nil(t, err, "Expected no error but got: %v", err) + }) + + // Test all combinations of settings, whitespace characters, and positions + for _, setting := range pathSettings { + for _, ws := range unicodeWhitespaces { + for _, position := range []string{"leading", "trailing"} { + t.Run(fmt.Sprintf("%s with %s %s whitespace", setting.name, position, ws.name), func(t *testing.T) { + cfg := &Config{} + cfg.SetDefaults() + + var testValue string + if position == "leading" { + testValue = ws.char + setting.validValue + } else { + testValue = setting.validValue + ws.char + } + + setting.configSetter(cfg, NewPointer(testValue)) + + err := cfg.FileSettings.isValid() + require.NotNil(t, err, "Expected an error but got none") + assert.Equal(t, "model.config.is_valid.directory_whitespace.app_error", err.Id) + }) + } + } + } +} + func TestConfigDefaultSignatureAlgorithm(t *testing.T) { c1 := Config{} c1.SetDefaults()