From 99a54ba67ef9969ef8bc8c7e9cd94dd7298686c4 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Mon, 3 Aug 2020 17:49:12 -0700 Subject: [PATCH] tornado: Fix ID lists leaked to the events API. Apparently, `update_message` events unexpectedly contained what were intended to be internal data structures about which users were mentioned in a given message. The bug has been present and accumulating new data structures for years. Fixing this should improve the performance of handling update_message events as well as cleaning up this API's interface. This was discovered by our automated API documentation schema checking tooling detecting these unexpected elements in these event definitions; that same logic should prevent future bugs like this from being introduced in the future. --- zerver/lib/event_schema.py | 11 ++--------- zerver/tornado/event_queue.py | 19 +++++++++++-------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/zerver/lib/event_schema.py b/zerver/lib/event_schema.py index 96e684811c..c3b6efcac1 100644 --- a/zerver/lib/event_schema.py +++ b/zerver/lib/event_schema.py @@ -5,7 +5,7 @@ It will contain schemas (aka validators) for Zulip events. Right now it's only intended to be used by test code. """ -from typing import Any, Dict, Sequence, Set, Tuple, Union +from typing import Any, Callable, Dict, List, Sequence, Set, Tuple, Union from zerver.lib.topic import ORIG_TOPIC, TOPIC_LINKS, TOPIC_NAME from zerver.lib.validator import ( @@ -693,20 +693,13 @@ update_message_required_fields = [ ("message_id", check_int), ] -update_message_content_fields = [ +update_message_content_fields: List[Tuple[str, Callable[[str, object], object]]] = [ ("content", check_string), ("is_me_message", check_bool), - ("mention_user_ids", check_list(check_int)), ("orig_content", check_string), ("orig_rendered_content", check_string), - ("presence_idle_user_ids", check_list(check_int)), ("prev_rendered_content_version", check_int), - ("prior_mention_user_ids", check_list(check_int)), - ("push_notify_user_ids", check_list(check_int)), ("rendered_content", check_string), - ("stream_email_user_ids", check_list(check_int)), - ("stream_push_user_ids", check_list(check_int)), - ("wildcard_mention_user_ids", check_list(check_int)), ] update_message_topic_fields = [ diff --git a/zerver/tornado/event_queue.py b/zerver/tornado/event_queue.py index a880996775..d972d34ff8 100644 --- a/zerver/tornado/event_queue.py +++ b/zerver/tornado/event_queue.py @@ -1007,15 +1007,18 @@ def process_deletion_event(event: Mapping[str, Any], users: Iterable[int]) -> No del compatibility_event['message_ids'] client.add_event(compatibility_event) -def process_message_update_event(event_template: Mapping[str, Any], +def process_message_update_event(orig_event: Mapping[str, Any], users: Iterable[Mapping[str, Any]]) -> None: - prior_mention_user_ids = set(event_template.get('prior_mention_user_ids', [])) - mention_user_ids = set(event_template.get('mention_user_ids', [])) - presence_idle_user_ids = set(event_template.get('presence_idle_user_ids', [])) - stream_push_user_ids = set(event_template.get('stream_push_user_ids', [])) - stream_email_user_ids = set(event_template.get('stream_email_user_ids', [])) - wildcard_mention_user_ids = set(event_template.get('wildcard_mention_user_ids', [])) - push_notify_user_ids = set(event_template.get('push_notify_user_ids', [])) + # Extract the parameters passed via the event object that don't + # belong in the actual events. + event_template = dict(orig_event) + prior_mention_user_ids = set(event_template.pop('prior_mention_user_ids', [])) + mention_user_ids = set(event_template.pop('mention_user_ids', [])) + presence_idle_user_ids = set(event_template.pop('presence_idle_user_ids', [])) + stream_push_user_ids = set(event_template.pop('stream_push_user_ids', [])) + stream_email_user_ids = set(event_template.pop('stream_email_user_ids', [])) + wildcard_mention_user_ids = set(event_template.pop('wildcard_mention_user_ids', [])) + push_notify_user_ids = set(event_template.pop('push_notify_user_ids', [])) stream_name = event_template.get('stream_name') message_id = event_template['message_id']