feat(extensions): Split messages on extended messages

v0.0.2 of toxext_extended_messages brought in a user configurable max
message size. This changeset implements the minimum work required for
qTox to work sanely under the new API.

* Hardcode a max message size for all friends
* If a friend negotiates a max message size below the hardcoded value
  pretend they do not have the extension
* Move splitMessage out of Core to MessageProcessor
* Updates to allow for extended messages to be split

src/core/core.cpp

@@ -1094,7 +1094,7 @@ bool Core::sendMessageWithType(uint32_t friendId, const QString& message, Tox_Me
                                ReceiptNum& receipt)
 {
     int size = message.toUtf8().size();
-    auto maxSize = static_cast<int>(tox_max_message_length());
+    auto maxSize = static_cast<int>(getMaxMessageSize());
     if (size > maxSize) {
         assert(false);
         qCritical() << "Core::sendMessageWithType called with message of size:" << size
@@ -1140,7 +1140,7 @@ void Core::sendGroupMessageWithType(int groupId, const QString& message, Tox_Mes
     QMutexLocker ml{&coreLoopLock};
 
     int size = message.toUtf8().size();
-    auto maxSize = static_cast<int>(tox_max_message_length());
+    auto maxSize = static_cast<int>(getMaxMessageSize());
     if (size > maxSize) {
         qCritical() << "Core::sendMessageWithType called with message of size:" << size
                     << "when max is:" << maxSize << ". Ignoring.";
@@ -1762,11 +1762,8 @@ QString Core::getFriendUsername(uint32_t friendnumber) const
     return ToxString(nameBuf.data(), nameSize).getQString();
 }
 
-QStringList Core::splitMessage(const QString& message)
+uint64_t Core::getMaxMessageSize() const
 {
-    QStringList splittedMsgs;
-    QByteArray ba_message{message.toUtf8()};
-
     /*
      * TODO: Remove this hack; the reported max message length we receive from c-toxcore
      * as of 08-02-2019 is inaccurate, causing us to generate too large messages when splitting
@@ -1777,33 +1774,7 @@ QStringList Core::splitMessage(const QString& message)
      *
      * (uint32_t tox_max_message_length(void); declared in tox.h, unable to see explicit definition)
      */
-    const auto maxLen = static_cast<int>(tox_max_message_length()) - 50;
+    return tox_max_message_length() - 50;
-
-    while (ba_message.size() > maxLen) {
-        int splitPos = ba_message.lastIndexOf('\n', maxLen - 1);
-
-        if (splitPos <= 0) {
-            splitPos = ba_message.lastIndexOf(' ', maxLen - 1);
-        }
-
-        if (splitPos <= 0) {
-            constexpr uint8_t firstOfMultiByteMask = 0xC0;
-            constexpr uint8_t multiByteMask = 0x80;
-            splitPos = maxLen;
-            // don't split a utf8 character
-            if ((ba_message[splitPos] & multiByteMask) == multiByteMask) {
-                while ((ba_message[splitPos] & firstOfMultiByteMask) != firstOfMultiByteMask) {
-                    --splitPos;
-                }
-            }
-            --splitPos;
-        }
-        splittedMsgs.append(QString{ba_message.left(splitPos + 1)});
-        ba_message = ba_message.mid(splitPos + 1);
-    }
-
-    splittedMsgs.append(QString{ba_message});
-    return splittedMsgs;
 }
 
 QString Core::getPeerName(const ToxPk& id) const

src/core/core.h

@@ -85,7 +85,7 @@ public:
     ~Core();
 
     static const QString TOX_EXT;
-    static QStringList splitMessage(const QString& message);
+    uint64_t getMaxMessageSize() const;
     QString getPeerName(const ToxPk& id) const;
     QVector<uint32_t> getFriendList() const;
     GroupId getGroupPersistentId(uint32_t groupNumber) const override;

src/core/coreext.cpp

@@ -98,16 +98,34 @@ uint64_t CoreExt::Packet::addExtendedMessage(QString message)
         return UINT64_MAX;
     }
 
-    ToxString toxString(message);
+    int size = message.toUtf8().size();
-    Tox_Extension_Messages_Error err;
+    enum Tox_Extension_Messages_Error err;
+    auto maxSize = static_cast<int>(tox_extension_messages_get_max_sending_size(
+        toxExtMessages,
+        friendId,
+        &err));
 
-    return tox_extension_messages_append(
+    if (size > maxSize) {
+        assert(false);
+        qCritical() << "addExtendedMessage called with message of size:" << size
+                    << "when max is:" << maxSize << ". Ignoring.";
+        return false;
+    }
+
+    ToxString toxString(message);
+    const auto receipt = tox_extension_messages_append(
         toxExtMessages,
         packetList,
         toxString.data(),
         toxString.size(),
         friendId,
         &err);
+
+    if (err != TOX_EXTENSION_MESSAGES_SUCCESS) {
+        qWarning() << "Error sending extension message";
+    }
+
+    return receipt;
 }
 
 bool CoreExt::Packet::send()
@@ -122,6 +140,11 @@ bool CoreExt::Packet::send()
     return ret == TOXEXT_SUCCESS;
 }
 
+uint64_t CoreExt::getMaxExtendedMessageSize()
+{
+    return TOX_EXTENSION_MESSAGES_DEFAULT_MAX_RECEIVING_MESSAGE_SIZE;
+}
+
 void CoreExt::onFriendStatusChanged(uint32_t friendId, Status::Status status)
 {
     const auto prevStatusIt = currentStatuses.find(friendId);
@@ -154,6 +177,15 @@ void CoreExt::onExtendedMessageReceipt(uint32_t friendId, uint64_t receiptId, vo
 void CoreExt::onExtendedMessageNegotiation(uint32_t friendId, bool compatible, uint64_t maxMessageSize, void* userData)
 {
     auto coreExt = static_cast<CoreExt*>(userData);
+
+    // HACK: handling configurable max message size per-friend is not trivial.
+    // For now the upper layers just assume that the max size for extended
+    // messages is the same for all friends. If a friend has a max message size
+    // lower than this value we just pretend they do not have the extension since
+    // we will not split correctly for this friend.
+    if (maxMessageSize < coreExt->getMaxExtendedMessageSize())
+        compatible = false;
+
     emit coreExt->extendedMessageSupport(friendId, compatible);
 }
 

src/core/coreext.h

@@ -117,6 +117,8 @@ public:
 
     std::unique_ptr<ICoreExtPacket> getPacket(uint32_t friendId) override;
 
+    uint64_t getMaxExtendedMessageSize();
+
 signals:
     void extendedMessageReceived(uint32_t friendId, const QString& message);
     void extendedReceiptReceived(uint32_t friendId, uint64_t receiptId);

src/model/chathistory.cpp

@@ -441,7 +441,7 @@ void ChatHistory::dispatchUnsentMessages(IMessageDispatcher& messageDispatcher)
             // up having to split more than when we added the message to history we'll
             // just associate the last dispatched id with the history message
             ? messageDispatcher.sendMessage(isAction, messageContent).second
-            : messageDispatcher.sendExtendedMessage(messageContent, requiredExtensions);
+            : messageDispatcher.sendExtendedMessage(messageContent, requiredExtensions).second;
 
         handleDispatchedMessage(dispatchId, message.id);
 

src/model/friendmessagedispatcher.cpp

@@ -55,19 +55,22 @@ FriendMessageDispatcher::sendMessage(bool isAction, const QString& content)
 /**
  * @see IMessageDispatcher::sendExtendedMessage
  */
-DispatchedMessageId FriendMessageDispatcher::sendExtendedMessage(const QString& content, ExtensionSet extensions)
+std::pair<DispatchedMessageId, DispatchedMessageId>
+FriendMessageDispatcher::sendExtendedMessage(const QString& content, ExtensionSet extensions)
 {
-    auto messageId = nextMessageId++;
+    const auto firstId = nextMessageId;
+    auto lastId = nextMessageId;
 
-    auto messages = processor.processOutgoingMessage(false, content, extensions);
+    for (const auto& message : processor.processOutgoingMessage(false, content, extensions)) {
-    assert(messages.size() == 1);
+        auto messageId = nextMessageId++;
+        lastId = messageId;
 
-    auto onOfflineMsgComplete = getCompletionFn(messageId);
+        auto onOfflineMsgComplete = getCompletionFn(messageId);
-    sendProcessedMessage(messages[0], onOfflineMsgComplete);
+        sendProcessedMessage(message, onOfflineMsgComplete);
 
-    emit this->messageSent(messageId, messages[0]);
+        emit this->messageSent(messageId, message);
-
+    }
-    return messageId;
+    return std::make_pair(firstId, lastId);
 }
 
 /**
@@ -150,7 +153,8 @@ void FriendMessageDispatcher::sendExtendedProcessedMessage(Message const& messag
 
     auto receipt = ExtendedReceiptNum();
 
-    auto packet = coreExtPacketAllocator.getPacket(f.getId());
+    const auto friendId = f.getId();
+    auto packet = coreExtPacketAllocator.getPacket(friendId);
 
     if (message.extensionSet[ExtensionType::messages]) {
         receipt.get() = packet->addExtendedMessage(message.content);

src/model/friendmessagedispatcher.h

@@ -41,7 +41,7 @@ public:
     std::pair<DispatchedMessageId, DispatchedMessageId> sendMessage(bool isAction,
                                                                     const QString& content) override;
 
-    DispatchedMessageId sendExtendedMessage(const QString& content, ExtensionSet extensions) override;
+    std::pair<DispatchedMessageId, DispatchedMessageId> sendExtendedMessage(const QString& content, ExtensionSet extensions) override;
     void onMessageReceived(bool isAction, const QString& content);
     void onReceiptReceived(ReceiptNum receipt);
     void onExtMessageReceived(const QString& message);

src/model/groupmessagedispatcher.cpp

@@ -65,14 +65,15 @@ GroupMessageDispatcher::sendMessage(bool isAction, QString const& content)
     return std::make_pair(firstMessageId, lastMessageId);
 }
 
-DispatchedMessageId GroupMessageDispatcher::sendExtendedMessage(const QString& content, ExtensionSet extensions)
+std::pair<DispatchedMessageId, DispatchedMessageId>
+GroupMessageDispatcher::sendExtendedMessage(const QString& content, ExtensionSet extensions)
 {
     // Stub this api to immediately fail
     auto messageId = nextMessageId++;
     auto messages = processor.processOutgoingMessage(false, content, ExtensionSet());
     emit this->messageSent(messageId, messages[0]);
     emit this->messageBroken(messageId, BrokenMessageReason::unsupportedExtensions);
-    return messageId;
+    return {messageId, messageId};
 }
 
 /**

src/model/groupmessagedispatcher.h

@@ -43,7 +43,8 @@ public:
     std::pair<DispatchedMessageId, DispatchedMessageId> sendMessage(bool isAction,
                                                                     QString const& content) override;
 
-    DispatchedMessageId sendExtendedMessage(const QString& content, ExtensionSet extensions) override;
+    std::pair<DispatchedMessageId, DispatchedMessageId> sendExtendedMessage(const QString& content,
+                            ExtensionSet extensions) override;
     void onMessageReceived(ToxPk const& sender, bool isAction, QString const& content);
 
 private:

src/model/imessagedispatcher.h

@@ -54,7 +54,7 @@ public:
      * @note If the provided extensions are not supported the message will be flagged
      *       as broken
      */
-    virtual DispatchedMessageId
+    virtual std::pair<DispatchedMessageId, DispatchedMessageId>
     sendExtendedMessage(const QString& content, ExtensionSet extensions) = 0;
 
 signals:

src/model/message.cpp

@@ -23,6 +23,38 @@
 
 #include <cassert>
 
+namespace {
+    QStringList splitMessage(const QString& message, uint64_t maxLength)
+    {
+        QStringList splittedMsgs;
+        QByteArray ba_message{message.toUtf8()};
+        while (static_cast<uint64_t>(ba_message.size()) > maxLength) {
+            int splitPos = ba_message.lastIndexOf('\n', maxLength - 1);
+
+            if (splitPos <= 0) {
+                splitPos = ba_message.lastIndexOf(' ', maxLength - 1);
+            }
+
+            if (splitPos <= 0) {
+                constexpr uint8_t firstOfMultiByteMask = 0xC0;
+                constexpr uint8_t multiByteMask = 0x80;
+                splitPos = maxLength;
+                // don't split a utf8 character
+                if ((ba_message[splitPos] & multiByteMask) == multiByteMask) {
+                    while ((ba_message[splitPos] & firstOfMultiByteMask) != firstOfMultiByteMask) {
+                        --splitPos;
+                    }
+                }
+                --splitPos;
+            }
+            splittedMsgs.append(QString{ba_message.left(splitPos + 1)});
+            ba_message = ba_message.mid(splitPos + 1);
+        }
+
+        splittedMsgs.append(QString{ba_message});
+        return splittedMsgs;
+    }
+}
 void MessageProcessor::SharedParams::onUserNameSet(const QString& username)
 {
     QString sanename = username;
@@ -55,10 +87,11 @@ std::vector<Message> MessageProcessor::processOutgoingMessage(bool isAction, QSt
 {
     std::vector<Message> ret;
 
-    const auto needsSplit = !extensions[ExtensionType::messages] || isAction;
+    const auto maxSendingSize = extensions[ExtensionType::messages]
-    const auto splitMsgs = needsSplit
+        ? sharedParams.getMaxExtendedMessageSize()
-        ? Core::splitMessage(content)
+        : sharedParams.getMaxCoreMessageSize();
-        : QStringList({content});
+
+    const auto splitMsgs = splitMessage(content, maxSendingSize);
 
     ret.reserve(splitMsgs.size());
 
@@ -91,9 +124,9 @@ Message MessageProcessor::processIncomingCoreMessage(bool isAction, QString cons
     ret.timestamp = timestamp;
 
     if (detectingMentions) {
-        auto nameMention = sharedParams.GetNameMention();
+        auto nameMention = sharedParams.getNameMention();
-        auto sanitizedNameMention = sharedParams.GetSanitizedNameMention();
+        auto sanitizedNameMention = sharedParams.getSanitizedNameMention();
-        auto pubKeyMention = sharedParams.GetPublicKeyMention();
+        auto pubKeyMention = sharedParams.getPublicKeyMention();
 
         for (auto const& mention : {nameMention, sanitizedNameMention, pubKeyMention}) {
             auto matchIt = mention.globalMatch(ret.content);

src/model/message.h

@@ -71,22 +71,39 @@ public:
     {
 
     public:
-        QRegularExpression GetNameMention() const
+        SharedParams(uint64_t maxCoreMessageSize_, uint64_t maxExtendedMessageSize_)
+            : maxCoreMessageSize(maxCoreMessageSize_)
+            , maxExtendedMessageSize(maxExtendedMessageSize_)
+        {}
+
+        QRegularExpression getNameMention() const
         {
             return nameMention;
         }
-        QRegularExpression GetSanitizedNameMention() const
+        QRegularExpression getSanitizedNameMention() const
         {
             return sanitizedNameMention;
         }
-        QRegularExpression GetPublicKeyMention() const
+        QRegularExpression getPublicKeyMention() const
         {
             return pubKeyMention;
         }
         void onUserNameSet(const QString& username);
         void setPublicKey(const QString& pk);
 
+        uint64_t getMaxCoreMessageSize() const
+        {
+            return maxCoreMessageSize;
+        }
+
+        uint64_t getMaxExtendedMessageSize() const
+        {
+            return maxExtendedMessageSize;
+        }
+
     private:
+        uint64_t maxCoreMessageSize;
+        uint64_t maxExtendedMessageSize;
         QRegularExpression nameMention;
         QRegularExpression sanitizedNameMention;
         QRegularExpression pubKeyMention;

src/widget/widget.cpp

@@ -248,6 +248,9 @@ void Widget::init()
     ui->searchContactFilterBox->setMenu(filterMenu);
 
     core = &profile.getCore();
+    auto coreExt = core->getExt();
+
+    sharedMessageProcessorParams.reset(new MessageProcessor::SharedParams(core->getMaxMessageSize(), coreExt->getMaxExtendedMessageSize()));
 
     contactListWidget = new FriendListWidget(*core, this, settings.getGroupchatPosition());
     connect(contactListWidget, &FriendListWidget::searchCircle, this, &Widget::searchCircle);
@@ -707,7 +710,7 @@ void Widget::onCoreChanged(Core& core)
     connect(this, &Widget::friendRequestAccepted, &core, &Core::acceptFriendRequest);
     connect(this, &Widget::changeGroupTitle, &core, &Core::changeGroupTitle);
 
-    sharedMessageProcessorParams.setPublicKey(core.getSelfPublicKey().toString());
+    sharedMessageProcessorParams->setPublicKey(core.getSelfPublicKey().toString());
 }
 
 void Widget::onConnected()
@@ -999,7 +1002,7 @@ void Widget::setUsername(const QString& username)
             Qt::convertFromPlainText(username, Qt::WhiteSpaceNormal)); // for overlength names
     }
 
-    sharedMessageProcessorParams.onUserNameSet(username);
+    sharedMessageProcessorParams->onUserNameSet(username);
 }
 
 void Widget::onStatusMessageChanged(const QString& newStatusMessage)
@@ -1151,7 +1154,7 @@ void Widget::addFriend(uint32_t friendId, const ToxPk& friendPk)
     connectFriendWidget(*widget);
     auto history = profile.getHistory();
 
-    auto messageProcessor = MessageProcessor(sharedMessageProcessorParams);
+    auto messageProcessor = MessageProcessor(*sharedMessageProcessorParams);
     auto friendMessageDispatcher =
         std::make_shared<FriendMessageDispatcher>(*newfriend, std::move(messageProcessor), *core, *core->getExt());
 
@@ -2127,7 +2130,7 @@ Group* Widget::createGroup(uint32_t groupnumber, const GroupId& groupId)
 
     const auto compact = settings.getCompactLayout();
     auto widget = new GroupWidget(chatroom, compact);
-    auto messageProcessor = MessageProcessor(sharedMessageProcessorParams);
+    auto messageProcessor = MessageProcessor(*sharedMessageProcessorParams);
     auto messageDispatcher =
         std::make_shared<GroupMessageDispatcher>(*newgroup, std::move(messageProcessor), *core,
                                                  *core, settings);

src/widget/widget.h

@@ -364,7 +364,7 @@ private:
     Core* core = nullptr;
 
 
-    MessageProcessor::SharedParams sharedMessageProcessorParams;
+    std::unique_ptr<MessageProcessor::SharedParams> sharedMessageProcessorParams;
 #if DESKTOP_NOTIFICATIONS
     std::unique_ptr<NotificationGenerator> notificationGenerator;
     DesktopNotify notifier;

test/model/friendmessagedispatcher_test.cpp

@@ -28,6 +28,8 @@
 #include <set>
 #include <deque>
 
+static constexpr uint64_t testMaxExtendedMessageSize = 10 * 1024 * 1024;
+
 
 class MockCoreExtPacket : public ICoreExtPacket
 {
@@ -165,7 +167,8 @@ void TestFriendMessageDispatcher::init()
     messageSender = std::unique_ptr<MockFriendMessageSender>(new MockFriendMessageSender());
     coreExtPacketAllocator = std::unique_ptr<MockCoreExtPacketAllocator>(new MockCoreExtPacketAllocator());
     sharedProcessorParams =
-        std::unique_ptr<MessageProcessor::SharedParams>(new MessageProcessor::SharedParams());
+        std::unique_ptr<MessageProcessor::SharedParams>(new MessageProcessor::SharedParams(tox_max_message_length(), testMaxExtendedMessageSize));
+
     messageProcessor = std::unique_ptr<MessageProcessor>(new MessageProcessor(*sharedProcessorParams));
     friendMessageDispatcher = std::unique_ptr<FriendMessageDispatcher>(
         new FriendMessageDispatcher(*f, *messageProcessor, *messageSender, *coreExtPacketAllocator));
@@ -377,7 +380,7 @@ void TestFriendMessageDispatcher::testActionMessagesSplitWithExtensions()
 
     auto reallyLongMessage = QString("a");
 
-    for (int i = 0; i < 9999; ++i) {
+    for (uint64_t i = 0; i < testMaxExtendedMessageSize + 50; ++i) {
         reallyLongMessage += i;
     }
 

test/model/groupmessagedispatcher_test.cpp

@@ -131,7 +131,7 @@ void TestGroupMessageDispatcher::init()
         new Group(0, GroupId(), "TestGroup", false, "me", *groupQuery, *coreIdHandler));
     messageSender = std::unique_ptr<MockGroupMessageSender>(new MockGroupMessageSender());
     sharedProcessorParams =
-        std::unique_ptr<MessageProcessor::SharedParams>(new MessageProcessor::SharedParams());
+        std::unique_ptr<MessageProcessor::SharedParams>(new MessageProcessor::SharedParams(tox_max_message_length(), 10 * 1024 * 1024));
     messageProcessor = std::unique_ptr<MessageProcessor>(new MessageProcessor(*sharedProcessorParams));
     groupMessageDispatcher = std::unique_ptr<GroupMessageDispatcher>(
         new GroupMessageDispatcher(*g, *messageProcessor, *coreIdHandler, *messageSender,

test/model/messageprocessor_test.cpp

@@ -53,7 +53,7 @@ private slots:
  */
 void TestMessageProcessor::testSelfMention()
 {
-    MessageProcessor::SharedParams sharedParams;
+    MessageProcessor::SharedParams sharedParams(tox_max_message_length(), 10 * 1024 * 1024);;
     const QLatin1String testUserName{"MyUserName"};
     const QLatin1String testToxPk{
         "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF"};
@@ -124,7 +124,7 @@ void TestMessageProcessor::testSelfMention()
  */
 void TestMessageProcessor::testOutgoingMessage()
 {
-    auto sharedParams = MessageProcessor::SharedParams();
+    auto sharedParams = MessageProcessor::SharedParams(tox_max_message_length(), 10 * 1024 * 1024);
     auto messageProcessor = MessageProcessor(sharedParams);
 
     QString testStr;
@@ -152,7 +152,7 @@ void TestMessageProcessor::testOutgoingMessage()
 void TestMessageProcessor::testIncomingMessage()
 {
     // Nothing too special happening on the incoming side if we aren't looking for self mentions
-    auto sharedParams = MessageProcessor::SharedParams();
+    auto sharedParams = MessageProcessor::SharedParams(tox_max_message_length(), 10 * 1024 * 1024);
     auto messageProcessor = MessageProcessor(sharedParams);
     auto message = messageProcessor.processIncomingCoreMessage(false, "test");