From a054f57af68637ad8b96290b3e13b2a6d8a8a359 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Fri, 18 Dec 2020 17:36:50 -0800 Subject: [PATCH] message: Bundle message stripping, validation, and truncation. We always want to do these at the same time. Previously, message editing did too much stripping (fixes #16837) and failed to check for NUL bytes. Signed-off-by: Anders Kaseorg --- zerver/lib/actions.py | 10 ++-------- zerver/lib/email_mirror.py | 7 +++---- zerver/lib/message.py | 7 ++++++- zerver/tests/test_drafts.py | 2 +- zerver/views/drafts.py | 6 ++---- zerver/views/message_edit.py | 7 +++---- 6 files changed, 17 insertions(+), 22 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 719d4a2979..81f5a86244 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -100,8 +100,8 @@ from zerver.lib.message import ( MessageDict, access_message, get_last_message_id, + normalize_body, render_markdown, - truncate_body, truncate_topic, update_first_visible_message_id, wildcard_mention_allowed, @@ -2333,13 +2333,7 @@ def check_message(sender: UserProfile, client: Client, addressee: Addressee, """ stream = None - message_content = message_content_raw.rstrip() - if len(message_content) == 0: - raise JsonableError(_("Message must not be empty")) - if '\x00' in message_content: - raise JsonableError(_("Message must not contain null bytes")) - - message_content = truncate_body(message_content) + message_content = normalize_body(message_content_raw) if realm is None: realm = sender.realm diff --git a/zerver/lib/email_mirror.py b/zerver/lib/email_mirror.py index 82d14ec2c8..8716e5ee84 100644 --- a/zerver/lib/email_mirror.py +++ b/zerver/lib/email_mirror.py @@ -21,7 +21,7 @@ from zerver.lib.email_mirror_helpers import ( ) from zerver.lib.email_notifications import convert_html_to_markdown from zerver.lib.exceptions import RateLimited -from zerver.lib.message import truncate_body, truncate_topic +from zerver.lib.message import normalize_body, truncate_topic from zerver.lib.queue import queue_json_publish from zerver.lib.rate_limiter import RateLimitedObject from zerver.lib.send_email import FromAddress @@ -161,8 +161,7 @@ def construct_zulip_body(message: EmailMessage, realm: Realm, show_sender: bool= if not body.endswith('\n'): body += '\n' body += extract_and_upload_attachments(message, realm) - body = body.strip() - if not body: + if not body.rstrip(): body = '(No email body)' if show_sender: @@ -182,7 +181,7 @@ def send_zulip(sender: UserProfile, stream: Stream, topic: str, content: str) -> sender, stream, truncate_topic(topic), - truncate_body(content), + normalize_body(content), email_gateway=True) def get_message_part_by_type(message: EmailMessage, content_type: str) -> Optional[str]: diff --git a/zerver/lib/message.py b/zerver/lib/message.py index a31c089cd9..2b0a191202 100644 --- a/zerver/lib/message.py +++ b/zerver/lib/message.py @@ -89,7 +89,12 @@ def truncate_content(content: str, max_length: int, truncation_message: str) -> content = content[:max_length - len(truncation_message)] + truncation_message return content -def truncate_body(body: str) -> str: +def normalize_body(body: str) -> str: + body = body.rstrip() + if len(body) == 0: + raise JsonableError(_("Message must not be empty")) + if '\x00' in body: + raise JsonableError(_("Message must not contain null bytes")) return truncate_content(body, MAX_MESSAGE_LENGTH, "\n[message truncated]") def truncate_topic(topic: str) -> str: diff --git a/zerver/tests/test_drafts.py b/zerver/tests/test_drafts.py index e9dd65c953..e6b4fdf65d 100644 --- a/zerver/tests/test_drafts.py +++ b/zerver/tests/test_drafts.py @@ -264,7 +264,7 @@ class DraftCreationTests(ZulipTestCase): }] self.create_and_check_drafts_for_error( draft_dicts, - "Content must not contain null bytes" + "Message must not contain null bytes" ) draft_dicts = [{ diff --git a/zerver/views/drafts.py b/zerver/views/drafts.py index a663a54235..3969c73e5e 100644 --- a/zerver/views/drafts.py +++ b/zerver/views/drafts.py @@ -8,7 +8,7 @@ from django.utils.translation import ugettext as _ from zerver.lib.actions import recipient_for_user_profiles from zerver.lib.addressee import get_user_profiles_by_ids from zerver.lib.exceptions import JsonableError -from zerver.lib.message import truncate_body, truncate_topic +from zerver.lib.message import normalize_body, truncate_topic from zerver.lib.request import REQ, has_request_variables from zerver.lib.response import json_error, json_success from zerver.lib.streams import access_stream_by_id @@ -48,9 +48,7 @@ def further_validated_draft_dict(draft_dict: Dict[str, Any], validated" draft dict. It will have a slightly different set of keys the values for which can be used to directly create a Draft object. """ - content = truncate_body(draft_dict["content"]) - if "\x00" in content: - raise JsonableError(_("Content must not contain null bytes")) + content = normalize_body(draft_dict["content"]) timestamp = draft_dict.get("timestamp", time.time()) timestamp = round(timestamp, 6) diff --git a/zerver/views/message_edit.py b/zerver/views/message_edit.py index 8153bc9244..cd26942737 100644 --- a/zerver/views/message_edit.py +++ b/zerver/views/message_edit.py @@ -17,7 +17,7 @@ from zerver.lib.actions import ( from zerver.lib.exceptions import JsonableError from zerver.lib.html_diff import highlight_html_differences from zerver.lib.markdown import MentionData -from zerver.lib.message import access_message, truncate_body +from zerver.lib.message import access_message, normalize_body from zerver.lib.queue import queue_json_publish from zerver.lib.response import json_error, json_success from zerver.lib.streams import get_stream_by_id @@ -161,10 +161,9 @@ def update_message_backend(request: HttpRequest, user_profile: UserMessage, mention_user_ids: Set[int] = set() mention_data: Optional[MentionData] = None if content is not None: - content = content.strip() - if content == "": + if content.rstrip() == "": content = "(deleted)" - content = truncate_body(content) + content = normalize_body(content) mention_data = MentionData( realm_id=user_profile.realm.id,