MDL-67782 message: fix messages max length

etreby · Jan 12, 2021 · ce87be7 · ce87be7
1 parent b17037b
commit ce87be7
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 1 deletion.
diff --git a/lang/en/message.php b/lang/en/message.php
@@ -69,6 +69,7 @@
 $string['enabled'] = 'Enabled';
 $string['errorcallingprocessor'] = 'Error calling defined output';
 $string['errorconversationdoesnotexist'] = 'Conversation does not exist';
+$string['errormessagetoolong'] = 'The message is longer than the maximum allowed.';
 $string['errortranslatingdefault'] = 'Error translating default setting provided by plugin, using system defaults instead.';
 $string['eventgroupmessagesent'] = 'Group message sent';
 $string['eventnotificationviewed'] = 'Notification viewed';

diff --git a/message/classes/api.php b/message/classes/api.php
@@ -93,6 +93,11 @@ class api {
      */
     const MESSAGE_CONVERSATION_DISABLED = 0;
 
+    /**
+     * The max message length.
+     */
+    const MESSAGE_MAX_LENGTH = 4096;
+
     /**
      * Handles searching for messages in the message area.
      *

diff --git a/message/classes/helper.php b/message/classes/helper.php
@@ -622,7 +622,8 @@ public static function render_messaging_widget(
                 'notification' => $notification
             ],
             'isdrawer' => $isdrawer,
-            'showemojipicker' => !empty($CFG->allowemojipicker)
+            'showemojipicker' => !empty($CFG->allowemojipicker),
+            'messagemaxlength' => api::MESSAGE_MAX_LENGTH,
         ];
 
         if ($sendtouser || $conversationid) {

diff --git a/message/externallib.php b/message/externallib.php
@@ -88,6 +88,14 @@ public static function send_messages_to_conversation(int $conversationid, array
             'messages' => $messages
         ]);
 
+        // Validate messages content before posting them.
+        foreach ($params['messages'] as $message) {
+            // Check message length.
+            if (strlen($message['text']) > \core_message\api::MESSAGE_MAX_LENGTH) {
+                throw new moodle_exception('errormessagetoolong', 'message');
+            }
+        }
+
         $messages = [];
         foreach ($params['messages'] as $message) {
             $createdmessage = \core_message\api::send_message_to_conversation($USER->id, $params['conversationid'], $message['text'],
@@ -187,6 +195,12 @@ public static function send_instant_messages($messages = array()) {
                 $errormessage = get_string('touserdoesntexist', 'message', $message['touserid']);
             }
 
+            // Check message length.
+            if ($success && strlen($message['text']) > \core_message\api::MESSAGE_MAX_LENGTH) {
+                $success = false;
+                $errormessage = get_string('errormessagetoolong', 'message');
+            }
+
             // TODO MDL-31118 performance improvement - edit the function so we can pass an array instead userid
             // Check if the recipient can be messaged by the sender.
             if ($success && !\core_message\api::can_send_message($tousers[$message['touserid']]->id, $USER->id)) {

diff --git a/message/templates/message_drawer_view_conversation_footer_content.mustache b/message/templates/message_drawer_view_conversation_footer_content.mustache
@@ -56,6 +56,7 @@
         aria-label="{{#str}} writeamessage, core_message {{/str}}"
         placeholder="{{#str}} writeamessage, core_message {{/str}}"
         style="resize: none"
+        maxlength="{{messagemaxlength}}"
     ></textarea>
 
     <div class="position-relative d-flex flex-column">

diff --git a/message/tests/externallib_test.php b/message/tests/externallib_test.php
@@ -155,6 +155,41 @@ public function test_send_instant_messages() {
         $this->assertEquals($sentmessage['clientmsgid'], $message1['clientmsgid']);
     }
 
+    /**
+     * Test send_instant_messages with a message text longer than permitted.
+     */
+    public function test_send_instant_messages_long_text() {
+        global $CFG;
+
+        $this->resetAfterTest(true);
+
+        // Transactions used in tests, tell phpunit use alternative reset method.
+        $this->preventResetByRollback();
+
+        $user1 = self::getDataGenerator()->create_user();
+        $user2 = self::getDataGenerator()->create_user();
+
+        $this->setUser($user1);
+
+        // Create test message data.
+        $message1 = [
+            'touserid' => $user2->id,
+            'text' => str_repeat("M", \core_message\api::MESSAGE_MAX_LENGTH + 100),
+            'clientmsgid' => 4,
+        ];
+        $messages = [$message1];
+
+        // Add the user1 as a contact.
+        \core_message\api::add_contact($user1->id, $user2->id);
+
+        $sentmessages = core_message_external::send_instant_messages($messages);
+        $sentmessages = external_api::clean_returnvalue(core_message_external::send_instant_messages_returns(), $sentmessages);
+        $this->assertEquals(
+            get_string('errormessagetoolong', 'message'),
+            array_pop($sentmessages)['errormessage']
+        );
+    }
+
     /**
      * Test send_instant_messages to a user who has blocked you.
      */
@@ -4798,6 +4833,38 @@ public function test_send_messages_to_conversation_non_member() {
         $writtenmessages = core_message_external::send_messages_to_conversation($gc1->id, $messages);
     }
 
+    /**
+     * Test verifying a to long message can not be sent to a conversation.
+     */
+    public function test_send_messages_to_conversation_long_text() {
+        $this->resetAfterTest(true);
+
+        // Get a bunch of conversations, some group, some individual and in different states.
+        list($user1, $user2, $user3, $user4, $ic1, $ic2, $ic3,
+            $gc1, $gc2, $gc3, $gc4, $gc5, $gc6) = $this->create_conversation_test_data();
+
+        // Enrol the users in the same course, so the default privacy controls (course + contacts) can be used.
+        $course1 = $this->getDataGenerator()->create_course();
+        $this->getDataGenerator()->enrol_user($user1->id, $course1->id);
+        $this->getDataGenerator()->enrol_user($user2->id, $course1->id);
+        $this->getDataGenerator()->enrol_user($user3->id, $course1->id);
+        $this->getDataGenerator()->enrol_user($user4->id, $course1->id);
+
+        // The user making the request.
+        $this->setUser($user1);
+
+        // Try to send a message as user1 to a conversation user1 is a a part of.
+        $messages = [
+            [
+                'text' => str_repeat("M", \core_message\api::MESSAGE_MAX_LENGTH + 100),
+                'textformat' => FORMAT_MOODLE
+            ],
+        ];
+
+        $this->expectException(moodle_exception::class);
+        $writtenmessages = core_message_external::send_messages_to_conversation($gc2->id, $messages);
+    }
+
     /**
      * Test getting a conversation that doesn't exist.
      */