From 1351874528b3dc8c77fe00ed438ae8a8d97c32a9 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Fri, 23 Mar 2018 12:33:50 -0400 Subject: improve error handling around invalid Forward80To443 settings (#8496) * If Forward80To443 is true, but not configured to listen on 443, fail to start the server with an error message. * If Forward80To443 is false and LetsEncrypt is true, fail to start the server with an error message. --- app/server.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'app') diff --git a/app/server.go b/app/server.go index 0c6c25ba5..e89041ebe 100644 --- a/app/server.go +++ b/app/server.go @@ -6,6 +6,7 @@ package app import ( "context" "crypto/tls" + "fmt" "io" "io/ioutil" "net" @@ -149,8 +150,10 @@ func (a *App) StartServer() error { } if *a.Config().ServiceSettings.Forward80To443 { - if host, _, err := net.SplitHostPort(addr); err != nil { + if host, port, err := net.SplitHostPort(addr); err != nil { l4g.Error("Unable to setup forwarding: " + err.Error()) + } else if port != "443" { + return fmt.Errorf(utils.T("api.server.start_server.forward80to443.enabled_but_listening_on_wrong_port"), port) } else { httpListenAddress := net.JoinHostPort(host, "http") @@ -169,6 +172,8 @@ func (a *App) StartServer() error { }() } } + } else if *a.Config().ServiceSettings.UseLetsEncrypt { + return errors.New(utils.T("api.server.start_server.forward80to443.disabled_while_using_lets_encrypt")) } a.Srv.didFinishListen = make(chan struct{}) -- cgit v1.2.3-1-g7c22 From 2bf2b5b851dfc693e5afa9648b5bbb9e6321b7fe Mon Sep 17 00:00:00 2001 From: Chris Date: Fri, 23 Mar 2018 15:04:22 -0500 Subject: send websocket event when users are activated / deactivated (#8500) --- app/user.go | 2 ++ 1 file changed, 2 insertions(+) (limited to 'app') diff --git a/app/user.go b/app/user.go index 8f393038a..e1ba0e79f 100644 --- a/app/user.go +++ b/app/user.go @@ -940,6 +940,8 @@ func (a *App) UpdateActive(user *model.User, active bool) (*model.User, *model.A } } + a.sendUpdatedUserEvent(*ruser, false) + return ruser, nil } } -- cgit v1.2.3-1-g7c22 From 8491ba5740e2d9942b2612ce06aef90bb10ad4c0 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Mon, 26 Mar 2018 17:55:35 -0400 Subject: Relax 4k post message limit (#8478) * MM-9661: rename POST_MESSAGE_MAX_RUNES to \0_v1 * MM-9661: s/4000/POST_MESSAGE_MAX_RUNES_V1/ in tests * MM-9661: introduce POST_MESSAGE_MAX_RUNES_V2 * MM-9661: migrate Postgres Posts.Message column to TEXT from VARCHAR(4000) This is safe to do in a production instance since the underyling type is not changing. We explicitly don't do this automatically for MySQL, but also don't need to since the ORM would have already created a TEXT column for MySQL in that case. * MM-9661: emit MaxPostSize in client config This value remains unconfigurable at this time, but exposes the current limit to the client. The limit remains at 4k in this commit. * MM-9661: introduce and use SqlPostStore.GetMaxPostSize Enforce a byte limitation in the database, and use 1/4 of that value as the rune count limitation (assuming a worst case UTF-8 representation). * move maxPostSizeCached, lastPostsCache and lastPostTimeCache out of the global context and onto the SqlPostStore * address feedback from code review: * ensure sqlstore unit tests are actually being run * move global caches into SqlPostStore * leverage sync.Once to address a race condition * modify upgrade semantics to match new db semantics gorp's behaviour on creating columns with a maximum length on Postgres differs from MySQL: * Postgres * gorp uses TEXT for string columns without a maximum length * gorp uses VARCHAR(N) for string columns with a maximum length of N * MySQL * gorp uses TEXT for string columns with a maximum length >= 256 * gorp uses VARCHAR(N) for string columns with a maximum length of N * gorp defaults to a maximum length of 255, implying VARCHAR(255) So the Message column has been TEXT on MySQL but VARCHAR(4000) on Postgres. With the new, longer limits of 65535, and without changes to gorp, the expected behaviour is TEXT on MySQL and VARCHAR(65535) on Postgres. This commit makes the upgrade semantics match the new database semantics. Ideally, we'd revisit the gorp behaviour at a later time. * allow TestMaxPostSize test cases to actually run in parallel * default maxPostSizeCached to POST_MESSAGE_MAX_RUNES_V1 in case the once initializer panics * fix casting error * MM-9661: skip the schema migration for Postgres It turns out resizing VARCHAR requires a rewrite in some versions of Postgres, but migrating VARCHAR to TEXT does not. Given the increasing complexity, let's defer the migration to the enduser instead. --- app/app.go | 2 +- app/config.go | 8 ++++--- app/config_test.go | 7 ++++-- app/import.go | 29 +++++++++++----------- app/import_test.go | 69 ++++++++++++++++++++++++++++------------------------- app/post.go | 11 +++++++++ app/post_test.go | 59 +++++++++++++++++++++++++++++++++++++++++++++ app/webhook.go | 8 +++---- app/webhook_test.go | 16 +++++++------ 9 files changed, 145 insertions(+), 64 deletions(-) (limited to 'app') diff --git a/app/app.go b/app/app.go index 6329a80d3..cd9fdaa66 100644 --- a/app/app.go +++ b/app/app.go @@ -139,7 +139,7 @@ func New(options ...Option) (outApp *App, outErr error) { message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_CONFIG_CHANGED, "", "", "", nil) - message.Add("config", app.ClientConfigWithNoAccounts()) + message.Add("config", app.ClientConfigWithComputed()) app.Go(func() { app.Publish(message) }) diff --git a/app/config.go b/app/config.go index ccd7236a0..761fe3ec9 100644 --- a/app/config.go +++ b/app/config.go @@ -273,15 +273,17 @@ func (a *App) GetSiteURL() string { return a.siteURL } -// ClientConfigWithNoAccounts gets the configuration in a format suitable for sending to the client. -func (a *App) ClientConfigWithNoAccounts() map[string]string { +// ClientConfigWithComputed gets the configuration in a format suitable for sending to the client. +func (a *App) ClientConfigWithComputed() map[string]string { respCfg := map[string]string{} for k, v := range a.ClientConfig() { respCfg[k] = v } - // NoAccounts is not actually part of the configuration, but is expected by the client. + // These properties are not configurable, but nevertheless represent configuration expected + // by the client. respCfg["NoAccounts"] = strconv.FormatBool(a.IsFirstUserAccount()) + respCfg["MaxPostSize"] = strconv.Itoa(a.MaxPostSize()) return respCfg } diff --git a/app/config_test.go b/app/config_test.go index 051fa8fd8..4fc7df5e2 100644 --- a/app/config_test.go +++ b/app/config_test.go @@ -64,12 +64,15 @@ func TestAsymmetricSigningKey(t *testing.T) { assert.NotEmpty(t, th.App.ClientConfig()["AsymmetricSigningPublicKey"]) } -func TestClientConfigWithNoAccounts(t *testing.T) { +func TestClientConfigWithComputed(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() - config := th.App.ClientConfigWithNoAccounts() + config := th.App.ClientConfigWithComputed() if _, ok := config["NoAccounts"]; !ok { t.Fatal("expected NoAccounts in returned config") } + if _, ok := config["MaxPostSize"]; !ok { + t.Fatal("expected MaxPostSize in returned config") + } } diff --git a/app/import.go b/app/import.go index e2e3aa1b7..23a315be7 100644 --- a/app/import.go +++ b/app/import.go @@ -1086,7 +1086,7 @@ func (a *App) ImportReaction(data *ReactionImportData, post *model.Post, dryRun } func (a *App) ImportReply(data *ReplyImportData, post *model.Post, dryRun bool) *model.AppError { - if err := validateReplyImportData(data, post.CreateAt); err != nil { + if err := validateReplyImportData(data, post.CreateAt, a.MaxPostSize()); err != nil { return err } @@ -1136,7 +1136,7 @@ func (a *App) ImportReply(data *ReplyImportData, post *model.Post, dryRun bool) } func (a *App) ImportPost(data *PostImportData, dryRun bool) *model.AppError { - if err := validatePostImportData(data); err != nil { + if err := validatePostImportData(data, a.MaxPostSize()); err != nil { return err } @@ -1271,14 +1271,14 @@ func validateReactionImportData(data *ReactionImportData, parentCreateAt int64) return nil } -func validateReplyImportData(data *ReplyImportData, parentCreateAt int64) *model.AppError { +func validateReplyImportData(data *ReplyImportData, parentCreateAt int64, maxPostSize int) *model.AppError { if data.User == nil { return model.NewAppError("BulkImport", "app.import.validate_reply_import_data.user_missing.error", nil, "", http.StatusBadRequest) } if data.Message == nil { return model.NewAppError("BulkImport", "app.import.validate_reply_import_data.message_missing.error", nil, "", http.StatusBadRequest) - } else if utf8.RuneCountInString(*data.Message) > model.POST_MESSAGE_MAX_RUNES { + } else if utf8.RuneCountInString(*data.Message) > maxPostSize { return model.NewAppError("BulkImport", "app.import.validate_reply_import_data.message_length.error", nil, "", http.StatusBadRequest) } @@ -1293,7 +1293,7 @@ func validateReplyImportData(data *ReplyImportData, parentCreateAt int64) *model return nil } -func validatePostImportData(data *PostImportData) *model.AppError { +func validatePostImportData(data *PostImportData, maxPostSize int) *model.AppError { if data.Team == nil { return model.NewAppError("BulkImport", "app.import.validate_post_import_data.team_missing.error", nil, "", http.StatusBadRequest) } @@ -1308,7 +1308,7 @@ func validatePostImportData(data *PostImportData) *model.AppError { if data.Message == nil { return model.NewAppError("BulkImport", "app.import.validate_post_import_data.message_missing.error", nil, "", http.StatusBadRequest) - } else if utf8.RuneCountInString(*data.Message) > model.POST_MESSAGE_MAX_RUNES { + } else if utf8.RuneCountInString(*data.Message) > maxPostSize { return model.NewAppError("BulkImport", "app.import.validate_post_import_data.message_length.error", nil, "", http.StatusBadRequest) } @@ -1326,7 +1326,7 @@ func validatePostImportData(data *PostImportData) *model.AppError { if data.Replies != nil { for _, reply := range *data.Replies { - validateReplyImportData(&reply, *data.CreateAt) + validateReplyImportData(&reply, *data.CreateAt, maxPostSize) } } @@ -1446,7 +1446,7 @@ func validateDirectChannelImportData(data *DirectChannelImportData) *model.AppEr } func (a *App) ImportDirectPost(data *DirectPostImportData, dryRun bool) *model.AppError { - if err := validateDirectPostImportData(data); err != nil { + if err := validateDirectPostImportData(data, a.MaxPostSize()); err != nil { return err } @@ -1572,7 +1572,7 @@ func (a *App) ImportDirectPost(data *DirectPostImportData, dryRun bool) *model.A return nil } -func validateDirectPostImportData(data *DirectPostImportData) *model.AppError { +func validateDirectPostImportData(data *DirectPostImportData, maxPostSize int) *model.AppError { if data.ChannelMembers == nil { return model.NewAppError("BulkImport", "app.import.validate_direct_post_import_data.channel_members_required.error", nil, "", http.StatusBadRequest) } @@ -1591,7 +1591,7 @@ func validateDirectPostImportData(data *DirectPostImportData) *model.AppError { if data.Message == nil { return model.NewAppError("BulkImport", "app.import.validate_direct_post_import_data.message_missing.error", nil, "", http.StatusBadRequest) - } else if utf8.RuneCountInString(*data.Message) > model.POST_MESSAGE_MAX_RUNES { + } else if utf8.RuneCountInString(*data.Message) > maxPostSize { return model.NewAppError("BulkImport", "app.import.validate_direct_post_import_data.message_length.error", nil, "", http.StatusBadRequest) } @@ -1624,7 +1624,7 @@ func validateDirectPostImportData(data *DirectPostImportData) *model.AppError { if data.Replies != nil { for _, reply := range *data.Replies { - validateReplyImportData(&reply, *data.CreateAt) + validateReplyImportData(&reply, *data.CreateAt, maxPostSize) } } @@ -1640,12 +1640,13 @@ func validateDirectPostImportData(data *DirectPostImportData) *model.AppError { func (a *App) OldImportPost(post *model.Post) { // Workaround for empty messages, which may be the case if they are webhook posts. firstIteration := true + maxPostSize := a.MaxPostSize() for messageRuneCount := utf8.RuneCountInString(post.Message); messageRuneCount > 0 || firstIteration; messageRuneCount = utf8.RuneCountInString(post.Message) { firstIteration = false var remainder string - if messageRuneCount > model.POST_MESSAGE_MAX_RUNES { - remainder = string(([]rune(post.Message))[model.POST_MESSAGE_MAX_RUNES:]) - post.Message = truncateRunes(post.Message, model.POST_MESSAGE_MAX_RUNES) + if messageRuneCount > maxPostSize { + remainder = string(([]rune(post.Message))[maxPostSize:]) + post.Message = truncateRunes(post.Message, maxPostSize) } else { remainder = "" } diff --git a/app/import_test.go b/app/import_test.go index f294c8731..23213d81b 100644 --- a/app/import_test.go +++ b/app/import_test.go @@ -644,12 +644,13 @@ func TestImportValidateReactionImportData(t *testing.T) { func TestImportValidateReplyImportData(t *testing.T) { // Test with minimum required valid properties. parentCreateAt := model.GetMillis() - 100 + maxPostSize := 10000 data := ReplyImportData{ User: ptrStr("username"), Message: ptrStr("message"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validateReplyImportData(&data, parentCreateAt); err != nil { + if err := validateReplyImportData(&data, parentCreateAt, maxPostSize); err != nil { t.Fatal("Validation failed but should have been valid.") } @@ -658,7 +659,7 @@ func TestImportValidateReplyImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validateReplyImportData(&data, parentCreateAt); err == nil { + if err := validateReplyImportData(&data, parentCreateAt, maxPostSize); err == nil { t.Fatal("Should have failed due to missing required property.") } @@ -666,7 +667,7 @@ func TestImportValidateReplyImportData(t *testing.T) { User: ptrStr("username"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validateReplyImportData(&data, parentCreateAt); err == nil { + if err := validateReplyImportData(&data, parentCreateAt, maxPostSize); err == nil { t.Fatal("Should have failed due to missing required property.") } @@ -674,17 +675,17 @@ func TestImportValidateReplyImportData(t *testing.T) { User: ptrStr("username"), Message: ptrStr("message"), } - if err := validateReplyImportData(&data, parentCreateAt); err == nil { + if err := validateReplyImportData(&data, parentCreateAt, maxPostSize); err == nil { t.Fatal("Should have failed due to missing required property.") } // Test with invalid message. data = ReplyImportData{ User: ptrStr("username"), - Message: ptrStr(strings.Repeat("1234567890", 500)), + Message: ptrStr(strings.Repeat("0", maxPostSize+1)), CreateAt: ptrInt64(model.GetMillis()), } - if err := validateReplyImportData(&data, parentCreateAt); err == nil { + if err := validateReplyImportData(&data, parentCreateAt, maxPostSize); err == nil { t.Fatal("Should have failed due to too long message.") } @@ -694,7 +695,7 @@ func TestImportValidateReplyImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(0), } - if err := validateReplyImportData(&data, parentCreateAt); err == nil { + if err := validateReplyImportData(&data, parentCreateAt, maxPostSize); err == nil { t.Fatal("Should have failed due to 0 create-at value.") } @@ -703,12 +704,13 @@ func TestImportValidateReplyImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(parentCreateAt - 100), } - if err := validateReplyImportData(&data, parentCreateAt); err == nil { + if err := validateReplyImportData(&data, parentCreateAt, maxPostSize); err == nil { t.Fatal("Should have failed due parent with newer create-at value.") } } func TestImportValidatePostImportData(t *testing.T) { + maxPostSize := 10000 // Test with minimum required valid properties. data := PostImportData{ @@ -718,7 +720,7 @@ func TestImportValidatePostImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validatePostImportData(&data); err != nil { + if err := validatePostImportData(&data, maxPostSize); err != nil { t.Fatal("Validation failed but should have been valid.") } @@ -729,7 +731,7 @@ func TestImportValidatePostImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validatePostImportData(&data); err == nil { + if err := validatePostImportData(&data, maxPostSize); err == nil { t.Fatal("Should have failed due to missing required property.") } @@ -739,7 +741,7 @@ func TestImportValidatePostImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validatePostImportData(&data); err == nil { + if err := validatePostImportData(&data, maxPostSize); err == nil { t.Fatal("Should have failed due to missing required property.") } @@ -749,7 +751,7 @@ func TestImportValidatePostImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validatePostImportData(&data); err == nil { + if err := validatePostImportData(&data, maxPostSize); err == nil { t.Fatal("Should have failed due to missing required property.") } @@ -759,7 +761,7 @@ func TestImportValidatePostImportData(t *testing.T) { User: ptrStr("username"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validatePostImportData(&data); err == nil { + if err := validatePostImportData(&data, maxPostSize); err == nil { t.Fatal("Should have failed due to missing required property.") } @@ -769,7 +771,7 @@ func TestImportValidatePostImportData(t *testing.T) { User: ptrStr("username"), Message: ptrStr("message"), } - if err := validatePostImportData(&data); err == nil { + if err := validatePostImportData(&data, maxPostSize); err == nil { t.Fatal("Should have failed due to missing required property.") } @@ -778,10 +780,10 @@ func TestImportValidatePostImportData(t *testing.T) { Team: ptrStr("teamname"), Channel: ptrStr("channelname"), User: ptrStr("username"), - Message: ptrStr(strings.Repeat("1234567890", 500)), + Message: ptrStr(strings.Repeat("0", maxPostSize+1)), CreateAt: ptrInt64(model.GetMillis()), } - if err := validatePostImportData(&data); err == nil { + if err := validatePostImportData(&data, maxPostSize); err == nil { t.Fatal("Should have failed due to too long message.") } @@ -793,7 +795,7 @@ func TestImportValidatePostImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(0), } - if err := validatePostImportData(&data); err == nil { + if err := validatePostImportData(&data, maxPostSize); err == nil { t.Fatal("Should have failed due to 0 create-at value.") } @@ -817,7 +819,7 @@ func TestImportValidatePostImportData(t *testing.T) { Reactions: &reactions, Replies: &replies, } - if err := validatePostImportData(&data); err != nil { + if err := validatePostImportData(&data, maxPostSize); err != nil { t.Fatal("Should have succeeded.") } } @@ -933,6 +935,7 @@ func TestImportValidateDirectChannelImportData(t *testing.T) { } func TestImportValidateDirectPostImportData(t *testing.T) { + maxPostSize := 10000 // Test with minimum required valid properties. data := DirectPostImportData{ @@ -944,7 +947,7 @@ func TestImportValidateDirectPostImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validateDirectPostImportData(&data); err != nil { + if err := validateDirectPostImportData(&data, maxPostSize); err != nil { t.Fatal("Validation failed but should have been valid.") } @@ -954,7 +957,7 @@ func TestImportValidateDirectPostImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validateDirectPostImportData(&data); err == nil { + if err := validateDirectPostImportData(&data, maxPostSize); err == nil { t.Fatal("Should have failed due to missing required property.") } @@ -966,7 +969,7 @@ func TestImportValidateDirectPostImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validateDirectPostImportData(&data); err == nil { + if err := validateDirectPostImportData(&data, maxPostSize); err == nil { t.Fatal("Should have failed due to missing required property.") } @@ -978,7 +981,7 @@ func TestImportValidateDirectPostImportData(t *testing.T) { User: ptrStr("username"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validateDirectPostImportData(&data); err == nil { + if err := validateDirectPostImportData(&data, maxPostSize); err == nil { t.Fatal("Should have failed due to missing required property.") } @@ -990,7 +993,7 @@ func TestImportValidateDirectPostImportData(t *testing.T) { User: ptrStr("username"), Message: ptrStr("message"), } - if err := validateDirectPostImportData(&data); err == nil { + if err := validateDirectPostImportData(&data, maxPostSize); err == nil { t.Fatal("Should have failed due to missing required property.") } @@ -1001,7 +1004,7 @@ func TestImportValidateDirectPostImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validateDirectPostImportData(&data); err == nil { + if err := validateDirectPostImportData(&data, maxPostSize); err == nil { t.Fatal("Should have failed due to unsuitable number of members.") } @@ -1013,7 +1016,7 @@ func TestImportValidateDirectPostImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validateDirectPostImportData(&data); err == nil { + if err := validateDirectPostImportData(&data, maxPostSize); err == nil { t.Fatal("Should have failed due to unsuitable number of members.") } @@ -1034,7 +1037,7 @@ func TestImportValidateDirectPostImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validateDirectPostImportData(&data); err == nil { + if err := validateDirectPostImportData(&data, maxPostSize); err == nil { t.Fatal("Should have failed due to unsuitable number of members.") } @@ -1049,7 +1052,7 @@ func TestImportValidateDirectPostImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validateDirectPostImportData(&data); err != nil { + if err := validateDirectPostImportData(&data, maxPostSize); err != nil { t.Fatal("Validation failed but should have been valid.") } @@ -1060,10 +1063,10 @@ func TestImportValidateDirectPostImportData(t *testing.T) { model.NewId(), }, User: ptrStr("username"), - Message: ptrStr(strings.Repeat("1234567890", 500)), + Message: ptrStr(strings.Repeat("0", maxPostSize+1)), CreateAt: ptrInt64(model.GetMillis()), } - if err := validateDirectPostImportData(&data); err == nil { + if err := validateDirectPostImportData(&data, maxPostSize); err == nil { t.Fatal("Should have failed due to too long message.") } @@ -1077,7 +1080,7 @@ func TestImportValidateDirectPostImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(0), } - if err := validateDirectPostImportData(&data); err == nil { + if err := validateDirectPostImportData(&data, maxPostSize); err == nil { t.Fatal("Should have failed due to 0 create-at value.") } @@ -1097,7 +1100,7 @@ func TestImportValidateDirectPostImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validateDirectPostImportData(&data); err == nil { + if err := validateDirectPostImportData(&data, maxPostSize); err == nil { t.Fatal("Validation should have failed due to non-member flagged.") } @@ -1115,7 +1118,7 @@ func TestImportValidateDirectPostImportData(t *testing.T) { Message: ptrStr("message"), CreateAt: ptrInt64(model.GetMillis()), } - if err := validateDirectPostImportData(&data); err != nil { + if err := validateDirectPostImportData(&data, maxPostSize); err != nil { t.Fatal(err) } @@ -1146,7 +1149,7 @@ func TestImportValidateDirectPostImportData(t *testing.T) { Replies: &replies, } - if err := validateDirectPostImportData(&data); err != nil { + if err := validateDirectPostImportData(&data, maxPostSize); err != nil { t.Fatal(err) } } diff --git a/app/post.go b/app/post.go index 5067777ab..d9445155b 100644 --- a/app/post.go +++ b/app/post.go @@ -965,3 +965,14 @@ func (a *App) ImageProxyRemover() (f func(string) string) { return url } } + +func (a *App) MaxPostSize() int { + maxPostSize := model.POST_MESSAGE_MAX_RUNES_V1 + if result := <-a.Srv.Store.Post().GetMaxPostSize(); result.Err != nil { + l4g.Error(result.Err) + } else { + maxPostSize = result.Data.(int) + } + + return maxPostSize +} diff --git a/app/post_test.go b/app/post_test.go index 2472e40c6..8455656d7 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -9,6 +9,7 @@ import ( "net/http" "net/http/httptest" "strings" + "sync/atomic" "testing" "time" @@ -17,6 +18,8 @@ import ( "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/store" + "github.com/mattermost/mattermost-server/store/storetest" ) func TestUpdatePostEditAt(t *testing.T) { @@ -346,3 +349,59 @@ func TestMakeOpenGraphURLsAbsolute(t *testing.T) { }) } } + +func TestMaxPostSize(t *testing.T) { + t.Parallel() + + testCases := []struct { + Description string + StoreMaxPostSize int + ExpectedMaxPostSize int + ExpectedError *model.AppError + }{ + { + "error fetching max post size", + 0, + model.POST_MESSAGE_MAX_RUNES_V1, + model.NewAppError("TestMaxPostSize", "this is an error", nil, "", http.StatusBadRequest), + }, + { + "4000 rune limit", + 4000, + 4000, + nil, + }, + { + "16383 rune limit", + 16383, + 16383, + nil, + }, + } + + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.Description, func(t *testing.T) { + t.Parallel() + + mockStore := &storetest.Store{} + defer mockStore.AssertExpectations(t) + + mockStore.PostStore.On("GetMaxPostSize").Return( + storetest.NewStoreChannel(store.StoreResult{ + Data: testCase.StoreMaxPostSize, + Err: testCase.ExpectedError, + }), + ) + + app := App{ + Srv: &Server{ + Store: mockStore, + }, + config: atomic.Value{}, + } + + assert.Equal(t, testCase.ExpectedMaxPostSize, app.MaxPostSize()) + }) + } +} diff --git a/app/webhook.go b/app/webhook.go index abfc388b5..5c3e963ce 100644 --- a/app/webhook.go +++ b/app/webhook.go @@ -143,7 +143,7 @@ func (a *App) TriggerWebhook(payload *model.OutgoingWebhookPayload, hook *model. } } -func SplitWebhookPost(post *model.Post) ([]*model.Post, *model.AppError) { +func SplitWebhookPost(post *model.Post, maxPostSize int) ([]*model.Post, *model.AppError) { splits := make([]*model.Post, 0) remainingText := post.Message @@ -159,12 +159,12 @@ func SplitWebhookPost(post *model.Post) ([]*model.Post, *model.AppError) { return nil, model.NewAppError("SplitWebhookPost", "web.incoming_webhook.split_props_length.app_error", map[string]interface{}{"Max": model.POST_PROPS_MAX_USER_RUNES}, "", http.StatusBadRequest) } - for utf8.RuneCountInString(remainingText) > model.POST_MESSAGE_MAX_RUNES { + for utf8.RuneCountInString(remainingText) > maxPostSize { split := base x := 0 for index := range remainingText { x++ - if x > model.POST_MESSAGE_MAX_RUNES { + if x > maxPostSize { split.Message = remainingText[:index] remainingText = remainingText[index:] break @@ -266,7 +266,7 @@ func (a *App) CreateWebhookPost(userId string, channel *model.Channel, text, ove } } - splits, err := SplitWebhookPost(post) + splits, err := SplitWebhookPost(post, a.MaxPostSize()) if err != nil { return nil, err } diff --git a/app/webhook_test.go b/app/webhook_test.go index 4d2bc58fa..8931100ac 100644 --- a/app/webhook_test.go +++ b/app/webhook_test.go @@ -383,23 +383,25 @@ func TestSplitWebhookPost(t *testing.T) { Expected []*model.Post } + maxPostSize := 10000 + for name, tc := range map[string]TestCase{ "LongPost": { Post: &model.Post{ - Message: strings.Repeat("本", model.POST_MESSAGE_MAX_RUNES*3/2), + Message: strings.Repeat("本", maxPostSize*3/2), }, Expected: []*model.Post{ { - Message: strings.Repeat("本", model.POST_MESSAGE_MAX_RUNES), + Message: strings.Repeat("本", maxPostSize), }, { - Message: strings.Repeat("本", model.POST_MESSAGE_MAX_RUNES/2), + Message: strings.Repeat("本", maxPostSize/2), }, }, }, "LongPostAndMultipleAttachments": { Post: &model.Post{ - Message: strings.Repeat("本", model.POST_MESSAGE_MAX_RUNES*3/2), + Message: strings.Repeat("本", maxPostSize*3/2), Props: map[string]interface{}{ "attachments": []*model.SlackAttachment{ &model.SlackAttachment{ @@ -416,10 +418,10 @@ func TestSplitWebhookPost(t *testing.T) { }, Expected: []*model.Post{ { - Message: strings.Repeat("本", model.POST_MESSAGE_MAX_RUNES), + Message: strings.Repeat("本", maxPostSize), }, { - Message: strings.Repeat("本", model.POST_MESSAGE_MAX_RUNES/2), + Message: strings.Repeat("本", maxPostSize/2), Props: map[string]interface{}{ "attachments": []*model.SlackAttachment{ &model.SlackAttachment{ @@ -452,7 +454,7 @@ func TestSplitWebhookPost(t *testing.T) { }, } { t.Run(name, func(t *testing.T) { - splits, err := SplitWebhookPost(tc.Post) + splits, err := SplitWebhookPost(tc.Post, maxPostSize) if tc.Expected == nil { require.NotNil(t, err) } else { -- cgit v1.2.3-1-g7c22