Skip to content

Commit

Permalink
raise max size of chat messages in dev channels to 1MB (#12668)
Browse files Browse the repository at this point in the history
* disable message length checks for non-chat convos

* use bigger length for dev

* even bigger

* nvm

* test
  • Loading branch information
mmaxim committed Jul 3, 2018
1 parent 1ad0267 commit af55437
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 11 deletions.
5 changes: 3 additions & 2 deletions go/chat/msgchecker/boxed_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ func boxedFieldLengthChecker(descriptibleItemName string, actualLength int, maxL
}

func checkMessageBoxedLength(msg chat1.MessageBoxed) error {
textMsgLength := getBoxedMaxTextLength(msg.ClientHeader.Conv.TopicType)
switch msg.GetMessageType() {
case chat1.MessageType_ATTACHMENT,
chat1.MessageType_DELETE,
Expand All @@ -36,9 +37,9 @@ func checkMessageBoxedLength(msg chat1.MessageBoxed) error {
chat1.MessageType_ATTACHMENTUPLOADED:
return nil
case chat1.MessageType_TEXT:
return boxedFieldLengthChecker("TEXT message", len(msg.BodyCiphertext.E), BoxedTextMessageBodyMaxLength)
return boxedFieldLengthChecker("TEXT message", len(msg.BodyCiphertext.E), textMsgLength)
case chat1.MessageType_EDIT:
return boxedFieldLengthChecker("EDIT message", len(msg.BodyCiphertext.E), BoxedTextMessageBodyMaxLength)
return boxedFieldLengthChecker("EDIT message", len(msg.BodyCiphertext.E), textMsgLength)
case chat1.MessageType_REACTION:
return boxedFieldLengthChecker("REACTION message", len(msg.BodyCiphertext.E), BoxedReactionMessageBodyMaxLength)
case chat1.MessageType_HEADLINE:
Expand Down
28 changes: 28 additions & 0 deletions go/chat/msgchecker/constants.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
package msgchecker

import "github.com/keybase/client/go/protocol/chat1"

const (
TextMessageMaxLength = 10000
DevTextMessageMaxLength = 1000000
ReactionMessageMaxLength = 50
HeadlineMaxLength = 280
TopicMaxLength = 20
)

const (
BoxedTextMessageBodyMaxLength = 11000
DevBoxedTextMessageBodyMaxLength = 1100000
BoxedEditMessageBodyMaxLength = 11000
BoxedReactionMessageBodyMaxLength = 150
BoxedHeadlineMessageBodyMaxLength = 380
Expand All @@ -18,3 +22,27 @@ const (
BoxedSystemMessageBodyMaxLength = 5000
BoxedDeleteHistoryMessageBodyMaxLength = 200
)

func getMaxTextLength(topicType chat1.TopicType) (textMsgLength int) {
switch topicType {
case chat1.TopicType_CHAT:
textMsgLength = TextMessageMaxLength
case chat1.TopicType_DEV, chat1.TopicType_KBFSFILEEDIT:
textMsgLength = DevTextMessageMaxLength
default:
textMsgLength = TextMessageMaxLength
}
return textMsgLength
}

func getBoxedMaxTextLength(topicType chat1.TopicType) (textMsgLength int) {
switch topicType {
case chat1.TopicType_CHAT:
textMsgLength = BoxedTextMessageBodyMaxLength
case chat1.TopicType_DEV, chat1.TopicType_KBFSFILEEDIT:
textMsgLength = DevBoxedTextMessageBodyMaxLength
default:
textMsgLength = BoxedTextMessageBodyMaxLength
}
return textMsgLength
}
13 changes: 9 additions & 4 deletions go/chat/msgchecker/plaintext_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ func checkMessagePlaintextLength(msg chat1.MessagePlaintext) error {
if err != nil {
return err
}

textMsgLength := getMaxTextLength(msg.ClientHeader.Conv.TopicType)
switch mtype {
case chat1.MessageType_ATTACHMENT,
chat1.MessageType_DELETE,
Expand All @@ -82,13 +84,16 @@ func checkMessagePlaintextLength(msg chat1.MessagePlaintext) error {
chat1.MessageType_DELETEHISTORY:
return nil
case chat1.MessageType_TEXT:
return plaintextFieldLengthChecker("message", len(msg.MessageBody.Text().Body), TextMessageMaxLength)
return plaintextFieldLengthChecker("message", len(msg.MessageBody.Text().Body), textMsgLength)
case chat1.MessageType_EDIT:
return plaintextFieldLengthChecker("message edit", len(msg.MessageBody.Edit().Body), TextMessageMaxLength)
return plaintextFieldLengthChecker("message edit", len(msg.MessageBody.Edit().Body),
textMsgLength)
case chat1.MessageType_REACTION:
return plaintextFieldLengthChecker("message reaction", len(msg.MessageBody.Reaction().Body), ReactionMessageMaxLength)
return plaintextFieldLengthChecker("message reaction", len(msg.MessageBody.Reaction().Body),
ReactionMessageMaxLength)
case chat1.MessageType_HEADLINE:
return plaintextFieldLengthChecker("headline", len(msg.MessageBody.Headline().Headline), HeadlineMaxLength)
return plaintextFieldLengthChecker("headline", len(msg.MessageBody.Headline().Headline),
HeadlineMaxLength)
case chat1.MessageType_METADATA:
topicNameRes := validateTopicName(msg.MessageBody.Metadata().ConversationTitle)
if validateTopicNameResOK != topicNameRes {
Expand Down
10 changes: 5 additions & 5 deletions go/chat/sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,11 +361,6 @@ func (s *BlockingSender) Prepare(ctx context.Context, plaintext chat1.MessagePla
return nil, nil, nil, chat1.ChannelMention_NONE, nil, fmt.Errorf("cannot send message without type")
}

// Make sure it is a proper length
if err := msgchecker.CheckMessagePlaintext(plaintext); err != nil {
return nil, nil, nil, chat1.ChannelMention_NONE, nil, err
}

msg, err := s.addSenderToMessage(plaintext)
if err != nil {
return nil, nil, nil, chat1.ChannelMention_NONE, nil, err
Expand All @@ -380,6 +375,11 @@ func (s *BlockingSender) Prepare(ctx context.Context, plaintext chat1.MessagePla
}
}

// Make sure it is a proper length
if err := msgchecker.CheckMessagePlaintext(plaintext); err != nil {
return nil, nil, nil, chat1.ChannelMention_NONE, nil, err
}

// Make sure our delete message gets everything it should
var pendingAssetDeletes []chat1.Asset
if conv != nil {
Expand Down
10 changes: 10 additions & 0 deletions go/chat/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1271,6 +1271,7 @@ func TestChatSrvPostLocalLengthLimit(t *testing.T) {
users := ctc.users()

var created chat1.ConversationInfoLocal
var dev chat1.ConversationInfoLocal
switch mt {
case chat1.ConversationMembersType_TEAM:
firstConv := mustCreateConversationForTest(t, ctc, users[0], chat1.TopicType_CHAT,
Expand All @@ -1290,12 +1291,21 @@ func TestChatSrvPostLocalLengthLimit(t *testing.T) {
created = mustCreateConversationForTest(t, ctc, users[0], chat1.TopicType_CHAT,
mt, ctc.as(t, users[1]).user())
}
dev = mustCreateConversationForTest(t, ctc, users[0], chat1.TopicType_DEV,
mt, ctc.as(t, users[1]).user())

maxTextBody := strings.Repeat(".", msgchecker.TextMessageMaxLength)
_, err := postLocalForTest(t, ctc, users[0], created, chat1.NewMessageBodyWithText(chat1.MessageText{Body: maxTextBody}))
require.NoError(t, err)
_, err = postLocalForTest(t, ctc, users[0], created, chat1.NewMessageBodyWithText(chat1.MessageText{Body: maxTextBody + "!"}))
require.Error(t, err)
_, err = postLocalForTest(t, ctc, users[0], dev,
chat1.NewMessageBodyWithText(chat1.MessageText{Body: maxTextBody + "!"}))
require.NoError(t, err)
maxDevTextBody := strings.Repeat(".", msgchecker.DevTextMessageMaxLength)
_, err = postLocalForTest(t, ctc, users[0], dev,
chat1.NewMessageBodyWithText(chat1.MessageText{Body: maxDevTextBody + "!"}))
require.Error(t, err)

maxHeadlineBody := strings.Repeat(".", msgchecker.HeadlineMaxLength)
_, err = postLocalForTest(t, ctc, users[0], created, chat1.NewMessageBodyWithHeadline(chat1.MessageHeadline{Headline: maxHeadlineBody}))
Expand Down

0 comments on commit af55437

Please sign in to comment.