Skip to content

Commit

Permalink
If autolinking would make posts too long, don't
Browse files Browse the repository at this point in the history
Substitutions that increase message length have the potential to grow a
post past the ability of the database to store it. This manifests to
users as their posts inexplicably failing. To avoid this fail state, we
check to see if the post-substitution message is too long and, if so,
don't apply the changes.

Unfortunately, the actual maximum post size is not currently accessible
to plugins. In the meantime, we use the smallest value for which the
server guarantees support, which is exposed through
model.POST_MESSAGE_MAX_RUNES_V1 and is in practice 4000 runes.
  • Loading branch information
elyscape committed Apr 1, 2021
1 parent 681b494 commit bfa2b6c
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 0 deletions.
6 changes: 6 additions & 0 deletions server/autolinkplugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net/http"
"strings"
"sync"
"unicode/utf8"

"github.com/mattermost/mattermost-server/v5/model"
"github.com/mattermost/mattermost-server/v5/plugin"
Expand Down Expand Up @@ -208,6 +209,11 @@ func (p *Plugin) ProcessPost(c *plugin.Context, post *model.Post) (*model.Post,
return nil, ""
}

if runes := utf8.RuneCountInString(postText); runes > model.POST_MESSAGE_MAX_RUNES_V1 {
p.API.LogWarn("Rewritten message would be too long, skipping", "postID", post.Id, "rewrittenLength", runes)
return nil, ""
}

post.Message = postText
}

Expand Down
10 changes: 10 additions & 0 deletions server/autolinkplugin/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"strings"
"testing"

"github.com/mattermost/mattermost-server/v5/model"
Expand Down Expand Up @@ -305,6 +306,9 @@ func TestSpecialCases(t *testing.T) {
Pattern: "(example)",
Template: "test",
Scope: []string{"other-team/town-square"},
}, autolink.Autolink{
Pattern: "(toolong)",
Template: strings.Repeat("1", model.POST_MESSAGE_MAX_RUNES_V1+1),
})
validConfig := Config{
EnableAdminCommand: false,
Expand All @@ -331,6 +335,8 @@ func TestSpecialCases(t *testing.T) {
api.On("GetChannel", mock.AnythingOfType("string")).Return(&testChannel, nil)
api.On("GetTeam", mock.AnythingOfType("string")).Return(&testTeam, nil)

api.On("LogWarn", mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("int"))

testUser := model.User{
IsBot: false,
}
Expand Down Expand Up @@ -454,6 +460,10 @@ func TestSpecialCases(t *testing.T) {
"check out MM-12345 too",
"check out [MM-12345](https://mattermost.atlassian.net/browse/MM-12345) too",
},
{
"toolong",
"toolong",
},
}

for _, tt := range tests {
Expand Down

0 comments on commit bfa2b6c

Please sign in to comment.