From f8f154236d3dd2b83604941c5c6b72638eec22da Mon Sep 17 00:00:00 2001 From: Ferran Recio Date: Fri, 20 Nov 2020 16:20:46 +0100 Subject: [PATCH] MDL-67782 message: fix messages max length --- lang/en/message.php | 1 + message/classes/api.php | 5 ++ message/classes/helper.php | 3 +- message/externallib.php | 14 ++++ ..._view_conversation_footer_content.mustache | 1 + message/tests/externallib_test.php | 67 +++++++++++++++++++ 6 files changed, 90 insertions(+), 1 deletion(-) diff --git a/lang/en/message.php b/lang/en/message.php index 77a5b181ab2..a2ec609a1c9 100644 --- a/lang/en/message.php +++ b/lang/en/message.php @@ -69,6 +69,7 @@ $string['emailtagline'] = 'This is a copy of a message sent to you at "{$a->site $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 index 69d6a07e0c4..af3a7ec63e8 100644 --- 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 index 286b36c3113..5074c0e9472 100644 --- a/message/classes/helper.php +++ b/message/classes/helper.php @@ -622,7 +622,8 @@ class helper { '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 index 52be6aca820..90bf611ae60 100644 --- a/message/externallib.php +++ b/message/externallib.php @@ -88,6 +88,14 @@ class core_message_external extends external_api { '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 @@ class core_message_external extends external_api { $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 index 1098c0306f4..4c8f49dc6b6 100644 --- 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}}" >
diff --git a/message/tests/externallib_test.php b/message/tests/externallib_test.php index 942460693a4..ed73c9db7f1 100644 --- a/message/tests/externallib_test.php +++ b/message/tests/externallib_test.php @@ -155,6 +155,41 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $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 @@ class core_message_externallib_testcase extends externallib_advanced_testcase { $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. */ -- 2.17.1