From 7454680be5b8561b60e630dbbd2452a66753f44c Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Wed, 21 Jul 2021 13:15:29 +0530 Subject: [PATCH] Cleanup model/utils.go (#17945) * Cleanup model/utils.go - Removed some functions which were unsued. (The unused linter was somehow failing to catch this). - Moved some functions inside other packages. - Removed two duplicate instances of the same function - RemoveDuplicateStrings and UniqueStrings. - Moved some regexes to global vars to prevent compilation every time. ```release-note NONE ``` * fix lint ```release-note NONE ``` * bring back unused exception ```release-note NONE ``` * fix tests ```release-note NONE ``` * Address review comments ```release-note NONE ``` --- api4/post_test.go | 13 +-- api4/role.go | 2 +- api4/role_test.go | 9 +- app/import_validators.go | 50 +++++++--- app/plugin_signature.go | 9 +- model/config.go | 80 +++++++++++++-- model/role.go | 12 ++- model/team.go | 2 +- model/user.go | 24 ----- model/utils.go | 206 +++++++-------------------------------- model/utils_test.go | 6 +- 11 files changed, 182 insertions(+), 231 deletions(-) diff --git a/api4/post_test.go b/api4/post_test.go index 268a9a0d73..f210290e05 100644 --- a/api4/post_test.go +++ b/api4/post_test.go @@ -847,21 +847,22 @@ func TestPatchPost(t *testing.T) { th.App.Srv().SetLicense(model.NewTestLicense()) - fileIds := make([]string, 3) + fileIDs := make([]string, 3) data, err := testutils.ReadTestFile("test.png") require.NoError(t, err) - for i := 0; i < len(fileIds); i++ { + for i := 0; i < len(fileIDs); i++ { fileResp, resp := Client.UploadFile(data, channel.Id, "test.png") CheckNoError(t, resp) - fileIds[i] = fileResp.FileInfos[0].Id + fileIDs[i] = fileResp.FileInfos[0].Id } + sort.Strings(fileIDs) post := &model.Post{ ChannelId: channel.Id, IsPinned: true, Message: "#hashtag a message", Props: model.StringInterface{"channel_header": "old_header"}, - FileIds: fileIds[0:2], + FileIds: fileIDs[0:2], HasReactions: true, } post, _ = Client.CreatePost(post) @@ -873,7 +874,7 @@ func TestPatchPost(t *testing.T) { patch.IsPinned = model.NewBool(false) patch.Message = model.NewString("#otherhashtag other message") patch.Props = &model.StringInterface{"channel_header": "new_header"} - patchFileIds := model.StringArray(fileIds) // one extra file + patchFileIds := model.StringArray(fileIDs) // one extra file patch.FileIds = &patchFileIds patch.HasReactions = model.NewBool(false) @@ -885,7 +886,7 @@ func TestPatchPost(t *testing.T) { assert.Equal(t, "#otherhashtag other message", rpost.Message, "Message did not update properly") assert.Equal(t, *patch.Props, rpost.GetProps(), "Props did not update properly") assert.Equal(t, "#otherhashtag", rpost.Hashtags, "Message did not update properly") - assert.Equal(t, model.StringArray(fileIds[0:2]), rpost.FileIds, "FileIds should not update") + assert.Equal(t, model.StringArray(fileIDs[0:2]), rpost.FileIds, "FileIds should not update") assert.False(t, rpost.HasReactions, "HasReactions did not update properly") }) diff --git a/api4/role.go b/api4/role.go index 25dd6ab0dc..85e83342f9 100644 --- a/api4/role.go +++ b/api4/role.go @@ -137,7 +137,7 @@ func patchRole(c *Context, w http.ResponseWriter, r *http.Request) { } } - *patch.Permissions = model.UniqueStrings(*patch.Permissions) + *patch.Permissions = model.RemoveDuplicateStrings(*patch.Permissions) } if c.App.Srv().License() != nil && isGuest && !*c.App.Srv().License().Features.GuestAccountsPermissions { diff --git a/api4/role_test.go b/api4/role_test.go index ba95a48a3a..4215f6f5a7 100644 --- a/api4/role_test.go +++ b/api4/role_test.go @@ -5,6 +5,7 @@ package api4 import ( "context" + "sort" "strings" "testing" @@ -223,7 +224,9 @@ func TestPatchRole(t *testing.T) { assert.Equal(t, received.Name, role.Name) assert.Equal(t, received.DisplayName, role.DisplayName) assert.Equal(t, received.Description, role.Description) - assert.EqualValues(t, received.Permissions, []string{"manage_system", "create_public_channel", "manage_incoming_webhooks", "manage_outgoing_webhooks"}) + perms := []string{"manage_system", "create_public_channel", "manage_incoming_webhooks", "manage_outgoing_webhooks"} + sort.Strings(perms) + assert.EqualValues(t, received.Permissions, perms) assert.Equal(t, received.SchemeManaged, role.SchemeManaged) // Check a no-op patch succeeds. @@ -252,7 +255,9 @@ func TestPatchRole(t *testing.T) { assert.Equal(t, received.Name, role.Name) assert.Equal(t, received.DisplayName, role.DisplayName) assert.Equal(t, received.Description, role.Description) - assert.EqualValues(t, received.Permissions, []string{"manage_system", "manage_incoming_webhooks", "manage_outgoing_webhooks"}) + perms := []string{"manage_system", "manage_incoming_webhooks", "manage_outgoing_webhooks"} + sort.Strings(perms) + assert.EqualValues(t, received.Permissions, perms) assert.Equal(t, received.SchemeManaged, role.SchemeManaged) t.Run("Check guest permissions editing without E20 license", func(t *testing.T) { diff --git a/app/import_validators.go b/app/import_validators.go index cc42e906bc..0d1412f984 100644 --- a/app/import_validators.go +++ b/app/import_validators.go @@ -259,48 +259,48 @@ func validateUserImportData(data *UserImportData) *model.AppError { } if data.NotifyProps != nil { - if data.NotifyProps.Desktop != nil && !model.IsValidUserNotifyLevel(*data.NotifyProps.Desktop) { + if data.NotifyProps.Desktop != nil && !isValidUserNotifyLevel(*data.NotifyProps.Desktop) { return model.NewAppError("BulkImport", "app.import.validate_user_import_data.notify_props_desktop_invalid.error", nil, "", http.StatusBadRequest) } - if data.NotifyProps.DesktopSound != nil && !model.IsValidTrueOrFalseString(*data.NotifyProps.DesktopSound) { + if data.NotifyProps.DesktopSound != nil && !isValidTrueOrFalseString(*data.NotifyProps.DesktopSound) { return model.NewAppError("BulkImport", "app.import.validate_user_import_data.notify_props_desktop_sound_invalid.error", nil, "", http.StatusBadRequest) } - if data.NotifyProps.Email != nil && !model.IsValidTrueOrFalseString(*data.NotifyProps.Email) { + if data.NotifyProps.Email != nil && !isValidTrueOrFalseString(*data.NotifyProps.Email) { return model.NewAppError("BulkImport", "app.import.validate_user_import_data.notify_props_email_invalid.error", nil, "", http.StatusBadRequest) } - if data.NotifyProps.Mobile != nil && !model.IsValidUserNotifyLevel(*data.NotifyProps.Mobile) { + if data.NotifyProps.Mobile != nil && !isValidUserNotifyLevel(*data.NotifyProps.Mobile) { return model.NewAppError("BulkImport", "app.import.validate_user_import_data.notify_props_mobile_invalid.error", nil, "", http.StatusBadRequest) } - if data.NotifyProps.MobilePushStatus != nil && !model.IsValidPushStatusNotifyLevel(*data.NotifyProps.MobilePushStatus) { + if data.NotifyProps.MobilePushStatus != nil && !isValidPushStatusNotifyLevel(*data.NotifyProps.MobilePushStatus) { return model.NewAppError("BulkImport", "app.import.validate_user_import_data.notify_props_mobile_push_status_invalid.error", nil, "", http.StatusBadRequest) } - if data.NotifyProps.ChannelTrigger != nil && !model.IsValidTrueOrFalseString(*data.NotifyProps.ChannelTrigger) { + if data.NotifyProps.ChannelTrigger != nil && !isValidTrueOrFalseString(*data.NotifyProps.ChannelTrigger) { return model.NewAppError("BulkImport", "app.import.validate_user_import_data.notify_props_channel_trigger_invalid.error", nil, "", http.StatusBadRequest) } - if data.NotifyProps.CommentsTrigger != nil && !model.IsValidCommentsNotifyLevel(*data.NotifyProps.CommentsTrigger) { + if data.NotifyProps.CommentsTrigger != nil && !isValidCommentsNotifyLevel(*data.NotifyProps.CommentsTrigger) { return model.NewAppError("BulkImport", "app.import.validate_user_import_data.notify_props_comments_trigger_invalid.error", nil, "", http.StatusBadRequest) } } - if data.UseMarkdownPreview != nil && !model.IsValidTrueOrFalseString(*data.UseMarkdownPreview) { + if data.UseMarkdownPreview != nil && !isValidTrueOrFalseString(*data.UseMarkdownPreview) { return model.NewAppError("BulkImport", "app.import.validate_user_import_data.advanced_props_feature_markdown_preview.error", nil, "", http.StatusBadRequest) } - if data.UseFormatting != nil && !model.IsValidTrueOrFalseString(*data.UseFormatting) { + if data.UseFormatting != nil && !isValidTrueOrFalseString(*data.UseFormatting) { return model.NewAppError("BulkImport", "app.import.validate_user_import_data.advanced_props_formatting.error", nil, "", http.StatusBadRequest) } - if data.ShowUnreadSection != nil && !model.IsValidTrueOrFalseString(*data.ShowUnreadSection) { + if data.ShowUnreadSection != nil && !isValidTrueOrFalseString(*data.ShowUnreadSection) { return model.NewAppError("BulkImport", "app.import.validate_user_import_data.advanced_props_show_unread_section.error", nil, "", http.StatusBadRequest) } - if data.EmailInterval != nil && !model.IsValidEmailBatchingInterval(*data.EmailInterval) { + if data.EmailInterval != nil && !isValidEmailBatchingInterval(*data.EmailInterval) { return model.NewAppError("BulkImport", "app.import.validate_user_import_data.advanced_props_email_interval.error", nil, "", http.StatusBadRequest) } @@ -579,3 +579,31 @@ func validateEmojiImportData(data *EmojiImportData) *model.AppError { return nil } + +func isValidTrueOrFalseString(value string) bool { + return value == "true" || value == "false" +} + +func isValidUserNotifyLevel(notifyLevel string) bool { + return notifyLevel == model.ChannelNotifyAll || + notifyLevel == model.ChannelNotifyMention || + notifyLevel == model.ChannelNotifyNone +} + +func isValidPushStatusNotifyLevel(notifyLevel string) bool { + return notifyLevel == model.StatusOnline || + notifyLevel == model.StatusAway || + notifyLevel == model.StatusOffline +} + +func isValidCommentsNotifyLevel(notifyLevel string) bool { + return notifyLevel == model.CommentsNotifyAny || + notifyLevel == model.CommentsNotifyRoot || + notifyLevel == model.CommentsNotifyNever +} + +func isValidEmailBatchingInterval(emailInterval string) bool { + return emailInterval == model.PreferenceEmailIntervalImmediately || + emailInterval == model.PreferenceEmailIntervalFifteen || + emailInterval == model.PreferenceEmailIntervalHour +} diff --git a/app/plugin_signature.go b/app/plugin_signature.go index 3dcc4234e9..06ae69f3e0 100644 --- a/app/plugin_signature.go +++ b/app/plugin_signature.go @@ -43,7 +43,7 @@ func (s *Server) getPublicKey(name string) ([]byte, *model.AppError) { // AddPublicKey will add plugin public key to the config. Overwrites the previous file func (a *App) AddPublicKey(name string, key io.Reader) *model.AppError { - if model.IsSamlFile(&a.Config().SamlSettings, name) { + if isSamlFile(&a.Config().SamlSettings, name) { return model.NewAppError("AddPublicKey", "app.plugin.modify_saml.app_error", nil, "", http.StatusInternalServerError) } data, err := ioutil.ReadAll(key) @@ -66,7 +66,7 @@ func (a *App) AddPublicKey(name string, key io.Reader) *model.AppError { // DeletePublicKey will delete plugin public key from the config. func (a *App) DeletePublicKey(name string) *model.AppError { - if model.IsSamlFile(&a.Config().SamlSettings, name) { + if isSamlFile(&a.Config().SamlSettings, name) { return model.NewAppError("AddPublicKey", "app.plugin.modify_saml.app_error", nil, "", http.StatusInternalServerError) } filename := filepath.Base(name) @@ -144,3 +144,8 @@ func decodeIfArmored(reader io.Reader) (io.Reader, error) { } return block.Body, nil } + +// isSamlFile checks if filename is a SAML file. +func isSamlFile(saml *model.SamlSettings, filename string) bool { + return filename == *saml.PublicCertificateFile || filename == *saml.PrivateKeyFile || filename == *saml.IdpCertificateFile +} diff --git a/model/config.go b/model/config.go index 92fd7eda4c..d32d4fbc9b 100644 --- a/model/config.go +++ b/model/config.go @@ -1768,7 +1768,7 @@ type SupportSettings struct { } func (s *SupportSettings) SetDefaults() { - if !IsSafeLink(s.TermsOfServiceLink) { + if !isSafeLink(s.TermsOfServiceLink) { *s.TermsOfServiceLink = SupportSettingsDefaultTermsOfServiceLink } @@ -1776,7 +1776,7 @@ func (s *SupportSettings) SetDefaults() { s.TermsOfServiceLink = NewString(SupportSettingsDefaultTermsOfServiceLink) } - if !IsSafeLink(s.PrivacyPolicyLink) { + if !isSafeLink(s.PrivacyPolicyLink) { *s.PrivacyPolicyLink = "" } @@ -1784,7 +1784,7 @@ func (s *SupportSettings) SetDefaults() { s.PrivacyPolicyLink = NewString(SupportSettingsDefaultPrivacyPolicyLink) } - if !IsSafeLink(s.AboutLink) { + if !isSafeLink(s.AboutLink) { *s.AboutLink = "" } @@ -1792,7 +1792,7 @@ func (s *SupportSettings) SetDefaults() { s.AboutLink = NewString(SupportSettingsDefaultAboutLink) } - if !IsSafeLink(s.HelpLink) { + if !isSafeLink(s.HelpLink) { *s.HelpLink = "" } @@ -1800,7 +1800,7 @@ func (s *SupportSettings) SetDefaults() { s.HelpLink = NewString(SupportSettingsDefaultHelpLink) } - if !IsSafeLink(s.ReportAProblemLink) { + if !isSafeLink(s.ReportAProblemLink) { *s.ReportAProblemLink = "" } @@ -3669,7 +3669,7 @@ func (s *ServiceSettings) isValid() *AppError { if host == "" { isValidHost = true } else { - isValidHost = (net.ParseIP(host) != nil) || IsDomainName(host) + isValidHost = (net.ParseIP(host) != nil) || isDomainName(host) } portInt, err := strconv.Atoi(port) if err != nil || !isValidHost || portInt < 0 || portInt > math.MaxUint16 { @@ -3974,3 +3974,71 @@ func isTagPresent(tag string, tags []string) bool { return false } + +// Copied from https://golang.org/src/net/dnsclient.go#L119 +func isDomainName(s string) bool { + // See RFC 1035, RFC 3696. + // Presentation format has dots before every label except the first, and the + // terminal empty label is optional here because we assume fully-qualified + // (absolute) input. We must therefore reserve space for the first and last + // labels' length octets in wire format, where they are necessary and the + // maximum total length is 255. + // So our _effective_ maximum is 253, but 254 is not rejected if the last + // character is a dot. + l := len(s) + if l == 0 || l > 254 || l == 254 && s[l-1] != '.' { + return false + } + + last := byte('.') + ok := false // Ok once we've seen a letter. + partlen := 0 + for i := 0; i < len(s); i++ { + c := s[i] + switch { + default: + return false + case 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' || c == '_': + ok = true + partlen++ + case '0' <= c && c <= '9': + // fine + partlen++ + case c == '-': + // Byte before dash cannot be dot. + if last == '.' { + return false + } + partlen++ + case c == '.': + // Byte before dot cannot be dot, dash. + if last == '.' || last == '-' { + return false + } + if partlen > 63 || partlen == 0 { + return false + } + partlen = 0 + } + last = c + } + if last == '-' || partlen > 63 { + return false + } + + return ok +} + +func isSafeLink(link *string) bool { + if link != nil { + if IsValidHttpUrl(*link) { + return true + } else if strings.HasPrefix(*link, "/") { + return true + } else { + return false + } + } + + return true +} diff --git a/model/role.go b/model/role.go index 99bc623a20..db16126051 100644 --- a/model/role.go +++ b/model/role.go @@ -443,8 +443,8 @@ func (r *Role) Patch(patch *RolePatch) { func (r *Role) MergeChannelHigherScopedPermissions(higherScopedPermissions *RolePermissions) { mergedPermissions := []string{} - higherScopedPermissionsMap := AsStringBoolMap(higherScopedPermissions.Permissions) - rolePermissionsMap := AsStringBoolMap(r.Permissions) + higherScopedPermissionsMap := asStringBoolMap(higherScopedPermissions.Permissions) + rolePermissionsMap := asStringBoolMap(r.Permissions) for _, cp := range AllPermissions { if cp.Scope != PermissionScopeChannel { @@ -950,3 +950,11 @@ func AddAncillaryPermissions(permissions []string) []string { } return permissions } + +func asStringBoolMap(list []string) map[string]bool { + listMap := make(map[string]bool, len(list)) + for _, p := range list { + listMap[p] = true + } + return listMap +} diff --git a/model/team.go b/model/team.go index a6de6ce8f9..e38a140758 100644 --- a/model/team.go +++ b/model/team.go @@ -235,7 +235,7 @@ func IsReservedTeamName(s string) bool { } func IsValidTeamName(s string) bool { - if !IsValidAlphaNum(s) { + if !isValidAlphaNum(s) { return false } diff --git a/model/user.go b/model/user.go index ba48d6638c..ca9b228d9e 100644 --- a/model/user.go +++ b/model/user.go @@ -931,30 +931,6 @@ func CleanUsername(username string) string { return s } -func IsValidUserNotifyLevel(notifyLevel string) bool { - return notifyLevel == ChannelNotifyAll || - notifyLevel == ChannelNotifyMention || - notifyLevel == ChannelNotifyNone -} - -func IsValidPushStatusNotifyLevel(notifyLevel string) bool { - return notifyLevel == StatusOnline || - notifyLevel == StatusAway || - notifyLevel == StatusOffline -} - -func IsValidCommentsNotifyLevel(notifyLevel string) bool { - return notifyLevel == CommentsNotifyAny || - notifyLevel == CommentsNotifyRoot || - notifyLevel == CommentsNotifyNever -} - -func IsValidEmailBatchingInterval(emailInterval string) bool { - return emailInterval == PreferenceEmailIntervalImmediately || - emailInterval == PreferenceEmailIntervalFifteen || - emailInterval == PreferenceEmailIntervalHour -} - func IsValidLocale(locale string) bool { if locale != "" { if len(locale) > UserLocaleMaxLength { diff --git a/model/utils.go b/model/utils.go index a5f90b14cb..bbf54e75cb 100644 --- a/model/utils.go +++ b/model/utils.go @@ -16,7 +16,7 @@ import ( "net/mail" "net/url" "regexp" - "strconv" + "sort" "strings" "sync" "time" @@ -227,7 +227,7 @@ func GetEndOfDayMillis(thisTime time.Time, timeZoneOffset int) int64 { } func CopyStringMap(originalMap map[string]string) map[string]string { - copyMap := make(map[string]string) + copyMap := make(map[string]string, len(originalMap)) for k, v := range originalMap { copyMap[k] = v } @@ -315,21 +315,6 @@ func StringInterfaceFromJson(data io.Reader) map[string]interface{} { return objmap } -func StringToJson(s string) string { - b, _ := json.Marshal(s) - return string(b) -} - -func StringFromJson(data io.Reader) string { - decoder := json.NewDecoder(data) - - var s string - if err := decoder.Decode(&s); err != nil { - return "" - } - return s -} - // ToJson serializes an arbitrary data type to JSON, discarding the error. func ToJson(v interface{}) []byte { b, _ := json.Marshal(v) @@ -372,12 +357,12 @@ func GetServerIpAddress(iface string) string { return "" } -func IsLower(s string) bool { +func isLower(s string) bool { return strings.ToLower(s) == s } func IsValidEmail(email string) bool { - if !IsLower(email) { + if !isLower(email) { return false } @@ -422,25 +407,25 @@ func IsValidChannelIdentifier(s string) bool { return true } -func IsValidAlphaNum(s string) bool { - validAlphaNum := regexp.MustCompile(`^[a-z0-9]+([a-z\-0-9]+|(__)?)[a-z0-9]+$`) +var ( + validAlphaNum = regexp.MustCompile(`^[a-z0-9]+([a-z\-0-9]+|(__)?)[a-z0-9]+$`) + validAlphaNumHyphenUnderscore = regexp.MustCompile(`^[a-z0-9]+([a-z\-\_0-9]+|(__)?)[a-z0-9]+$`) + validSimpleAlphaNumHyphenUnderscore = regexp.MustCompile(`^[a-zA-Z0-9\-_]+$`) + validSimpleAlphaNumHyphenUnderscorePlus = regexp.MustCompile(`^[a-zA-Z0-9+_-]+$`) +) +func isValidAlphaNum(s string) bool { return validAlphaNum.MatchString(s) } func IsValidAlphaNumHyphenUnderscore(s string, withFormat bool) bool { if withFormat { - validAlphaNumHyphenUnderscore := regexp.MustCompile(`^[a-z0-9]+([a-z\-\_0-9]+|(__)?)[a-z0-9]+$`) return validAlphaNumHyphenUnderscore.MatchString(s) } - - validSimpleAlphaNumHyphenUnderscore := regexp.MustCompile(`^[a-zA-Z0-9\-_]+$`) return validSimpleAlphaNumHyphenUnderscore.MatchString(s) } func IsValidAlphaNumHyphenUnderscorePlus(s string) bool { - - validSimpleAlphaNumHyphenUnderscorePlus := regexp.MustCompile(`^[a-zA-Z0-9+_-]+$`) return validSimpleAlphaNumHyphenUnderscorePlus.MatchString(s) } @@ -455,10 +440,12 @@ func Etag(parts ...interface{}) string { return etag } -var validHashtag = regexp.MustCompile(`^(#\pL[\pL\d\-_.]*[\pL\d])$`) -var puncStart = regexp.MustCompile(`^[^\pL\d\s#]+`) -var hashtagStart = regexp.MustCompile(`^#{2,}`) -var puncEnd = regexp.MustCompile(`[^\pL\d\s]+$`) +var ( + validHashtag = regexp.MustCompile(`^(#\pL[\pL\d\-_.]*[\pL\d])$`) + puncStart = regexp.MustCompile(`^[^\pL\d\s#]+`) + hashtagStart = regexp.MustCompile(`^#{2,}`) + puncEnd = regexp.MustCompile(`[^\pL\d\s]+$`) +) func ParseHashtags(text string) (string, string) { words := strings.Fields(text) @@ -511,56 +498,6 @@ func IsValidHttpUrl(rawUrl string) bool { return true } -func IsValidTurnOrStunServer(rawUri string) bool { - if strings.Index(rawUri, "turn:") != 0 && strings.Index(rawUri, "stun:") != 0 { - return false - } - - if _, err := url.ParseRequestURI(rawUri); err != nil { - return false - } - - return true -} - -func IsSafeLink(link *string) bool { - if link != nil { - if IsValidHttpUrl(*link) { - return true - } else if strings.HasPrefix(*link, "/") { - return true - } else { - return false - } - } - - return true -} - -func IsValidWebsocketUrl(rawUrl string) bool { - if strings.Index(rawUrl, "ws://") != 0 && strings.Index(rawUrl, "wss://") != 0 { - return false - } - - if _, err := url.ParseRequestURI(rawUrl); err != nil { - return false - } - - return true -} - -func IsValidTrueOrFalseString(value string) bool { - return value == "true" || value == "false" -} - -func IsValidNumberString(value string) bool { - if _, err := strconv.Atoi(value); err != nil { - return false - } - - return true -} - func IsValidId(value string) bool { if len(value) != 26 { return false @@ -575,73 +512,24 @@ func IsValidId(value string) bool { return true } -// Copied from https://golang.org/src/net/dnsclient.go#L119 -func IsDomainName(s string) bool { - // See RFC 1035, RFC 3696. - // Presentation format has dots before every label except the first, and the - // terminal empty label is optional here because we assume fully-qualified - // (absolute) input. We must therefore reserve space for the first and last - // labels' length octets in wire format, where they are necessary and the - // maximum total length is 255. - // So our _effective_ maximum is 253, but 254 is not rejected if the last - // character is a dot. - l := len(s) - if l == 0 || l > 254 || l == 254 && s[l-1] != '.' { - return false - } - - last := byte('.') - ok := false // Ok once we've seen a letter. - partlen := 0 - for i := 0; i < len(s); i++ { - c := s[i] - switch { - default: - return false - case 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' || c == '_': - ok = true - partlen++ - case '0' <= c && c <= '9': - // fine - partlen++ - case c == '-': - // Byte before dash cannot be dot. - if last == '.' { - return false - } - partlen++ - case c == '.': - // Byte before dot cannot be dot, dash. - if last == '.' || last == '-' { - return false - } - if partlen > 63 || partlen == 0 { - return false - } - partlen = 0 - } - last = c - } - if last == '-' || partlen > 63 { - return false - } - - return ok -} - +// RemoveDuplicateStrings does an in-place removal of duplicate strings +// from the input slice. The original slice gets modified. func RemoveDuplicateStrings(in []string) []string { - out := []string{} - seen := make(map[string]bool, len(in)) - - for _, item := range in { - if !seen[item] { - out = append(out, item) - - seen[item] = true - } + // In-place de-dup. + // Copied from https://github.com/golang/go/wiki/SliceTricks#in-place-deduplicate-comparable + if len(in) == 0 { + return in } - - return out + sort.Strings(in) + j := 0 + for i := 1; i < len(in); i++ { + if in[j] == in[i] { + continue + } + j++ + in[j] = in[i] + } + return in[:j+1] } func GetPreferredTimezone(timezone StringMap) string { @@ -652,19 +540,6 @@ func GetPreferredTimezone(timezone StringMap) string { return timezone["manualTimezone"] } -// IsSamlFile checks if filename is a SAML file. -func IsSamlFile(saml *SamlSettings, filename string) bool { - return filename == *saml.PublicCertificateFile || filename == *saml.PrivateKeyFile || filename == *saml.IdpCertificateFile -} - -func AsStringBoolMap(list []string) map[string]bool { - listMap := map[string]bool{} - for _, p := range list { - listMap[p] = true - } - return listMap -} - // SanitizeUnicode will remove undesirable Unicode characters from a string. func SanitizeUnicode(s string) string { return strings.Map(filterBlocklist, s) @@ -709,18 +584,3 @@ func filterBlocklist(r rune) rune { return r } - -// UniqueStrings returns a unique subset of the string slice provided. -func UniqueStrings(input []string) []string { - u := make([]string, 0, len(input)) - m := make(map[string]bool) - - for _, val := range input { - if _, ok := m[val]; !ok { - m[val] = true - u = append(u, val) - } - } - - return u -} diff --git a/model/utils_test.go b/model/utils_test.go index acb466d610..2e811a0fc3 100644 --- a/model/utils_test.go +++ b/model/utils_test.go @@ -338,7 +338,7 @@ func TestIsValidAlphaNum(t *testing.T) { } for _, tc := range cases { - actual := IsValidAlphaNum(tc.Input) + actual := isValidAlphaNum(tc.Input) require.Equalf(t, actual, tc.Result, "case: %v\tshould returned: %#v", tc, tc.Result) } } @@ -945,7 +945,7 @@ func TestIsValidHttpUrl(t *testing.T) { } } -func TestUniqueStrings(t *testing.T) { +func TestRemoveDuplicateStrings(t *testing.T) { cases := []struct { Input []string Result []string @@ -973,7 +973,7 @@ func TestUniqueStrings(t *testing.T) { } for _, tc := range cases { - actual := UniqueStrings(tc.Input) + actual := RemoveDuplicateStrings(tc.Input) require.Equalf(t, actual, tc.Result, "case: %v\tshould returned: %#v", tc, tc.Result) } }