MM-55966 - Update ArrayFromJSON to use LimitedReader (#25510)

* update ArrayFromJSON to use LimitedReader * update for bad merge * fix lint errors * update test code * update unit tests * update unit tests * fix unit tests * use consts, other cleanup * add non sorting duplicate check * set config to default value, then config setting if available * fix lint errors * fixes and debugs * fix log test * remove setting from Client, add unlimited Parser to client * a couple more fixes * another fix * rename some variables * remove superflous call * check for valid MaximumPayloadSize * update language file * fix for e2e-tests * update util function to return error * lint fix * update config property name to include unit * fix for unit test * add new config to telemetry * call function to create LimitedReader * Deprecate old function, use new function name * return new AppError on failed parse * return new AppError on failed parse * return new AppError on failed parse * add constant for i18n valid constants * Update server/public/model/utils_test.go Co-authored-by: Miguel de la Cruz <mgdelacroix@gmail.com> * Apply suggestions from code review Co-authored-by: Miguel de la Cruz <mgdelacroix@gmail.com> * update error variable, remove unnecessary check * Update function names * fix errors from merge * update unit test to create unique ids --------- Co-authored-by: Mattermost Build <build@mattermost.com> Co-authored-by: Miguel de la Cruz <mgdelacroix@gmail.com>
2024-01-09 10:04:16 -07:00
--- a/server/public/model/client4.go
+++ b/server/public/model/client4.go
@@ -103,6 +103,15 @@ func (c *Client4) boolString(value bool) string {
 	return "false"
 }

+func (c *Client4) ArrayFromJSON(data io.Reader) []string {
+	var objmap []string
+	json.NewDecoder(data).Decode(&objmap)
+	if objmap == nil {
+		return make([]string, 0)
+	}
+	return objmap
+}
+
 func closeBody(r *http.Response) {
 	if r.Body != nil {
 		_, _ = io.Copy(io.Discard, r.Body)
@@ -3136,7 +3145,7 @@ func (c *Client4) GetChannelMembersTimezones(ctx context.Context, channelId stri
 		return nil, BuildResponse(r), err
 	}
 	defer closeBody(r)
-	return ArrayFromJSON(r.Body), BuildResponse(r), nil
+	return c.ArrayFromJSON(r.Body), BuildResponse(r), nil
 }

 // GetPinnedPosts gets a list of pinned posts.
@@ -5818,7 +5827,7 @@ func (c *Client4) GetLogs(ctx context.Context, page, perPage int) ([]string, *Re
 		return nil, BuildResponse(r), err
 	}
 	defer closeBody(r)
-	return ArrayFromJSON(r.Body), BuildResponse(r), nil
+	return c.ArrayFromJSON(r.Body), BuildResponse(r), nil
 }

 // PostLog is a convenience Web Service call so clients can log messages into
@@ -7922,7 +7931,7 @@ func (c *Client4) GetSidebarCategoryOrderForTeamForUser(ctx context.Context, use
 		return nil, BuildResponse(r), err
 	}
 	defer closeBody(r)
-	return ArrayFromJSON(r.Body), BuildResponse(r), nil
+	return c.ArrayFromJSON(r.Body), BuildResponse(r), nil
 }

 func (c *Client4) UpdateSidebarCategoryOrderForTeamForUser(ctx context.Context, userID, teamID string, order []string) ([]string, *Response, error) {
@@ -7936,7 +7945,7 @@ func (c *Client4) UpdateSidebarCategoryOrderForTeamForUser(ctx context.Context,
 		return nil, BuildResponse(r), err
 	}
 	defer closeBody(r)
-	return ArrayFromJSON(r.Body), BuildResponse(r), nil
+	return c.ArrayFromJSON(r.Body), BuildResponse(r), nil
 }

 func (c *Client4) GetSidebarCategoryForTeamForUser(ctx context.Context, userID, teamID, categoryID, etag string) (*SidebarCategoryWithChannels, *Response, error) {
@@ -8396,7 +8405,7 @@ func (c *Client4) ListImports(ctx context.Context) ([]string, *Response, error)
 		return nil, BuildResponse(r), err
 	}
 	defer closeBody(r)
-	return ArrayFromJSON(r.Body), BuildResponse(r), nil
+	return c.ArrayFromJSON(r.Body), BuildResponse(r), nil
 }

 func (c *Client4) ListExports(ctx context.Context) ([]string, *Response, error) {
@@ -8405,7 +8414,7 @@ func (c *Client4) ListExports(ctx context.Context) ([]string, *Response, error)
 		return nil, BuildResponse(r), err
 	}
 	defer closeBody(r)
-	return ArrayFromJSON(r.Body), BuildResponse(r), nil
+	return c.ArrayFromJSON(r.Body), BuildResponse(r), nil
 }

 func (c *Client4) DeleteExport(ctx context.Context, name string) (*Response, error) {
--- a/server/public/model/config.go
+++ b/server/public/model/config.go
@@ -402,6 +402,7 @@ type ServiceSettings struct {
 	AllowSyncedDrafts                                 *bool   `access:"site_posts"`
 	UniqueEmojiReactionLimitPerPost                   *int    `access:"site_posts"`
 	RefreshPostStatsRunTime                           *string `access:"site_users_and_teams"`
+	MaximumPayloadSizeBytes                           *int64  `access:"environment_file_storage,write_restrictable,cloud_restrictable"`
 }

 var MattermostGiphySdkKey string
@@ -908,6 +909,10 @@ func (s *ServiceSettings) SetDefaults(isUpdate bool) {
 	if s.RefreshPostStatsRunTime == nil {
 		s.RefreshPostStatsRunTime = NewString("00:00")
 	}
+
+	if s.MaximumPayloadSizeBytes == nil {
+		s.MaximumPayloadSizeBytes = NewInt64(100000)
+	}
 }

 type ClusterSettings struct {
@@ -3978,6 +3983,10 @@ func (s *ServiceSettings) isValid() *AppError {
 		}
 	}

+	if *s.MaximumPayloadSizeBytes <= 0 {
+		return NewAppError("Config.IsValid", "model.config.is_valid.max_payload_size.app_error", nil, "", http.StatusBadRequest)
+	}
+
 	if *s.ReadTimeout <= 0 {
 		return NewAppError("Config.IsValid", "model.config.is_valid.read_timeout.app_error", nil, "", http.StatusBadRequest)
 	}
--- a/server/public/model/utils.go
+++ b/server/public/model/utils.go
@@ -29,13 +29,14 @@ import (
 )

 const (
-	LowercaseLetters = "abcdefghijklmnopqrstuvwxyz"
-	UppercaseLetters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
-	NUMBERS          = "0123456789"
-	SYMBOLS          = " !\"\\#$%&'()*+,-./:;<=>?@[]^_`|~"
-	BinaryParamKey   = "MM_BINARY_PARAMETERS"
-	NoTranslation    = "<untranslated>"
-	maxPropSizeBytes = 1024 * 1024
+	LowercaseLetters  = "abcdefghijklmnopqrstuvwxyz"
+	UppercaseLetters  = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+	NUMBERS           = "0123456789"
+	SYMBOLS           = " !\"\\#$%&'()*+,-./:;<=>?@[]^_`|~"
+	BinaryParamKey    = "MM_BINARY_PARAMETERS"
+	NoTranslation     = "<untranslated>"
+	maxPropSizeBytes  = 1024 * 1024
+	PayloadParseError = "api.payload.parse.error"
 )

 var ErrMaxPropSizeExceeded = fmt.Errorf("max prop size of %d exceeded", maxPropSizeBytes)
@@ -484,17 +485,41 @@ func ArrayToJSON(objmap []string) string {
 	return string(b)
 }

+// Deprecated: ArrayFromJSON is deprecated,
+// use SortedArrayFromJSON or NonSortedArrayFromJSON instead
 func ArrayFromJSON(data io.Reader) []string {
 	var objmap []string
-
 	json.NewDecoder(data).Decode(&objmap)
 	if objmap == nil {
 		return make([]string, 0)
 	}
-
 	return objmap
 }

+func SortedArrayFromJSON(data io.Reader, maxBytes int64) ([]string, error) {
+	var obj []string
+	lr := io.LimitReader(data, maxBytes)
+	err := json.NewDecoder(lr).Decode(&obj)
+	if err != nil || obj == nil {
+		return nil, err
+	}
+
+	// Remove duplicate IDs as it can bring a significant load to the database.
+	return RemoveDuplicateStrings(obj), nil
+}
+
+func NonSortedArrayFromJSON(data io.Reader, maxBytes int64) ([]string, error) {
+	var obj []string
+	lr := io.LimitReader(data, maxBytes)
+	err := json.NewDecoder(lr).Decode(&obj)
+	if err != nil || obj == nil {
+		return nil, err
+	}
+
+	// Remove duplicate IDs, but don't sort.
+	return RemoveDuplicateStringsNonSort(obj), nil
+}
+
 func ArrayFromInterface(data any) []string {
 	stringArray := []string{}

@@ -737,6 +762,20 @@ func RemoveDuplicateStrings(in []string) []string {
 	return in[:j+1]
 }

+// RemoveDuplicateStringsNonSort does a removal of duplicate
+// strings using a map.
+func RemoveDuplicateStringsNonSort(in []string) []string {
+	allKeys := make(map[string]bool)
+	list := []string{}
+	for _, item := range in {
+		if _, value := allKeys[item]; !value {
+			allKeys[item] = true
+			list = append(list, item)
+		}
+	}
+	return list
+}
+
 func GetPreferredTimezone(timezone StringMap) string {
 	if timezone["useAutomaticTimezone"] == "true" {
 		return timezone["automaticTimezone"]
--- a/server/public/model/utils_test.go
+++ b/server/public/model/utils_test.go
@@ -5,8 +5,10 @@ package model

 import (
 	"bytes"
+	"encoding/json"
 	"errors"
 	"fmt"
+	"io"
 	"net/http"
 	"reflect"
 	"strings"
@@ -200,6 +202,88 @@ func TestMapJson(t *testing.T) {
 	require.LessOrEqual(t, len(rm2), 0, "make should be invalid")
 }

+func TestSortedArrayFromJSON(t *testing.T) {
+	t.Run("Successful parse", func(t *testing.T) {
+		ids := []string{NewId(), NewId(), NewId()}
+		b, _ := json.Marshal(ids)
+		a, err := SortedArrayFromJSON(bytes.NewReader(b), 1000)
+		require.NoError(t, err)
+		require.ElementsMatch(t, ids, a)
+	})
+
+	t.Run("Empty Array", func(t *testing.T) {
+		ids := []string{}
+		b, _ := json.Marshal(ids)
+		a, err := SortedArrayFromJSON(bytes.NewReader(b), 1000)
+		require.NoError(t, err)
+		require.Empty(t, a)
+	})
+
+	t.Run("Error too large", func(t *testing.T) {
+		var ids []string
+		for i := 0; i <= 100; i++ {
+			ids = append(ids, NewId())
+		}
+		b, _ := json.Marshal(ids)
+		_, err := SortedArrayFromJSON(bytes.NewReader(b), 1000)
+		require.Error(t, err)
+		require.ErrorIs(t, err, io.ErrUnexpectedEOF)
+	})
+
+	t.Run("Duplicate keys, returns one", func(t *testing.T) {
+		var ids []string
+		id := NewId()
+		for i := 0; i < 10; i++ {
+			ids = append(ids, id)
+		}
+		b, _ := json.Marshal(ids)
+		a, err := SortedArrayFromJSON(bytes.NewReader(b), 26000)
+		require.NoError(t, err)
+		require.Len(t, a, 1)
+	})
+}
+
+func TestNonSortedArrayFromJSON(t *testing.T) {
+	t.Run("Successful parse", func(t *testing.T) {
+		ids := []string{NewId(), NewId(), NewId()}
+		b, _ := json.Marshal(ids)
+		a, err := NonSortedArrayFromJSON(bytes.NewReader(b), 1000)
+		require.NoError(t, err)
+		require.Equal(t, ids, a)
+	})
+
+	t.Run("Empty Array", func(t *testing.T) {
+		ids := []string{}
+		b, _ := json.Marshal(ids)
+		a, err := NonSortedArrayFromJSON(bytes.NewReader(b), 1000)
+		require.NoError(t, err)
+		require.Empty(t, a)
+	})
+
+	t.Run("Error too large", func(t *testing.T) {
+		var ids []string
+		for i := 0; i <= 100; i++ {
+			ids = append(ids, NewId())
+		}
+		b, _ := json.Marshal(ids)
+		_, err := NonSortedArrayFromJSON(bytes.NewReader(b), 1000)
+		require.Error(t, err)
+		require.ErrorIs(t, err, io.ErrUnexpectedEOF)
+	})
+
+	t.Run("Duplicate keys, returns one", func(t *testing.T) {
+		var ids []string
+		id := NewId()
+		for i := 0; i <= 10; i++ {
+			ids = append(ids, id)
+		}
+		b, _ := json.Marshal(ids)
+		a, err := NonSortedArrayFromJSON(bytes.NewReader(b), 26000)
+		require.NoError(t, err)
+		require.Len(t, a, 1)
+	})
+}
+
 func TestIsValidEmail(t *testing.T) {
 	for _, testCase := range []struct {
 		Input    string