message-send: Deduplicate check of `settings.MAX_MESSAGE_LENGTH`.

Removes the initial check in `_internal_prep_message` of the length
of the message content because the `check_message` in the try block
will call `normalize_body` on the message content string, which
does a more robust check of the message content (empty string, null
bytes, length). If the message content length exceeds the value of
`settings.MAX_MESSAGE_LENGTH`, then it is truncated based on that
value. Updates associated backend test for these changes.

The removed length check would truncate the message content with a
hard coded value instead of using the value for
`settings.MAX_MESSAGE_LENGTH`.

Also, removes an extraneous comment about removing null bytes. If
there are null bytes in the message content, then `normalize_body`
will raise an error.

Note that the previous check had intentionally reduced any message over
the 10000 character limit to 3900 characters, with the code in
question dating to 2012's 100df7e349.

The 3900 character truncating rule was implemented for incoming emails
with the email gateway, and predated other features to help with
overly long messages (better stripping of email footers via Talon,
introduced in f1f48f305e, and
condensing, introduced in c92d664b44).
While we could preserve that logic if desired, it likely is no longer
a necessary or useful variation from our usual truncation rules.
This commit is contained in:
Lauryn Menard 2023-02-15 16:27:13 +01:00 committed by Tim Abbott
parent 93e78ea72f
commit 06dd6f8254
2 changed files with 3 additions and 6 deletions

View File

@ -1500,10 +1500,6 @@ def _internal_prep_message(
messages together as one database query.
Call do_send_messages with a list of the return values of this method.
"""
# Remove any null bytes from the content
if len(content) > settings.MAX_MESSAGE_LENGTH:
content = content[0:3900] + "\n\n[message was too long and has been truncated]"
# If we have a stream name, and the stream doesn't exist, we
# create it here (though this code path should probably be removed
# eventually, moving that responsibility to the caller). If

View File

@ -2258,14 +2258,15 @@ class InternalPrepTest(ZulipTestCase):
def test_error_handling(self) -> None:
sender = self.example_user("cordelia")
recipient_user = self.example_user("hamlet")
content = "x" * 15000
MAX_MESSAGE_LENGTH = settings.MAX_MESSAGE_LENGTH
content = "x" * (MAX_MESSAGE_LENGTH + 10)
result = internal_prep_private_message(
sender=sender, recipient_user=recipient_user, content=content
)
assert result is not None
message = result.message
self.assertIn("message was too long", message.content)
self.assertIn("message truncated", message.content)
# Simulate sending a message to somebody not in the
# realm of the sender.