From 34a4a1378d3917b827e5d8c1672e2e8088818dad Mon Sep 17 00:00:00 2001 From: Sampriti Panda Date: Thu, 19 Jan 2017 03:49:18 +0530 Subject: [PATCH] bugdown: Use specified realm, not sender realm, for rendering. This changes bugdown to use the realm passed in by the caller (if any) for rendering, fixing a problem where bots such as the notification bot would have their messages rendering using the admin realm's settings, not the settings of the realm their messages are being sent into. Also adds a test for the notification bot case. Fixes #3215. --- zerver/lib/actions.py | 1 + zerver/lib/bugdown/__init__.py | 22 ++++++++++--------- zerver/lib/message.py | 16 ++++++++------ zerver/tests/test_subs.py | 40 ++++++++++++++++++++++++++++++++++ zerver/views/messages.py | 2 +- 5 files changed, 63 insertions(+), 18 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 12fb550042..1560ed9557 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -770,6 +770,7 @@ def render_incoming_message(message, content, message_users, realm): rendered_content = render_markdown( message=message, content=content, + realm=realm, realm_alert_words=realm_alert_words, message_users=message_users, ) diff --git a/zerver/lib/bugdown/__init__.py b/zerver/lib/bugdown/__init__.py index 8c1f328e64..0677e1dba3 100644 --- a/zerver/lib/bugdown/__init__.py +++ b/zerver/lib/bugdown/__init__.py @@ -36,7 +36,7 @@ from zerver.lib.timeout import timeout, TimeoutExpired from zerver.lib.cache import ( cache_with_key, cache_get_many, cache_set_many, NotFoundInCache) from zerver.lib.url_preview import preview as link_preview -from zerver.models import Message +from zerver.models import Message, Realm import zerver.lib.alert_words as alert_words import zerver.lib.mention as mention from zerver.lib.str_utils import force_text, force_str @@ -1290,13 +1290,15 @@ def log_bugdown_error(msg): could cause an infinite exception loop.""" logging.getLogger('').error(msg) -def do_convert(content, realm_filters_key=None, message=None, possible_words=None): - # type: (Text, Optional[int], Optional[Message], Optional[Set[Text]]) -> Optional[Text] +def do_convert(content, realm_filters_key=None, message=None, message_realm=None, possible_words=None): + # type: (Text, Optional[int], Optional[Message], Optional[Realm], Optional[Set[Text]]) -> Optional[Text] """Convert Markdown to HTML, with Zulip-specific settings and hacks.""" from zerver.models import get_active_user_dicts_in_realm, get_active_streams, UserProfile if message: - maybe_update_realm_filters(message.get_realm().id) + if message_realm is None: + message_realm = message.get_realm() + maybe_update_realm_filters(realm_filters_key) if realm_filters_key in md_engines: _md_engine = md_engines[realm_filters_key] @@ -1314,8 +1316,8 @@ def do_convert(content, realm_filters_key=None, message=None, possible_words=Non # Pre-fetch data from the DB that is used in the bugdown thread global db_data if message: - realm_users = get_active_user_dicts_in_realm(message.get_realm()) - realm_streams = get_active_streams(message.get_realm()).values('id', 'name') + realm_users = get_active_user_dicts_in_realm(message_realm) + realm_streams = get_active_streams(message_realm).values('id', 'name') if possible_words is None: possible_words = set() # Set[Text] @@ -1323,7 +1325,7 @@ def do_convert(content, realm_filters_key=None, message=None, possible_words=Non db_data = {'possible_words': possible_words, 'full_names': dict((user['full_name'].lower(), user) for user in realm_users), 'short_names': dict((user['short_name'].lower(), user) for user in realm_users), - 'emoji': message.get_realm().get_emoji(), + 'emoji': message_realm.get_emoji(), 'stream_names': dict((stream['name'], stream) for stream in realm_streams)} try: @@ -1378,9 +1380,9 @@ def bugdown_stats_finish(): bugdown_total_requests += 1 bugdown_total_time += (time.time() - bugdown_time_start) -def convert(content, realm_filters_key=None, message=None, possible_words=None): - # type: (Text, Optional[int], Optional[Message], Optional[Set[Text]]) -> Optional[Text] +def convert(content, realm_filters_key=None, message=None, message_realm=None, possible_words=None): + # type: (Text, Optional[int], Optional[Message], Optional[Realm], Optional[Set[Text]]) -> Optional[Text] bugdown_stats_start() - ret = do_convert(content, realm_filters_key, message, possible_words) + ret = do_convert(content, realm_filters_key, message, message_realm, possible_words) bugdown_stats_finish() return ret diff --git a/zerver/lib/message.py b/zerver/lib/message.py index e6197cb4f3..37f5946324 100644 --- a/zerver/lib/message.py +++ b/zerver/lib/message.py @@ -20,6 +20,7 @@ from zerver.lib.timestamp import datetime_to_timestamp from zerver.models import ( get_display_recipient_by_id, Message, + Realm, Recipient, Stream, UserProfile, @@ -215,7 +216,7 @@ class MessageDict(object): # It's unfortunate that we need to have side effects on the message # in some cases. - rendered_content = render_markdown(message, content, realm_id=sender_realm_id) + rendered_content = render_markdown(message, content, realm=message.get_realm()) message.rendered_content = rendered_content message.rendered_content_version = bugdown.version message.save_rendered_content() @@ -299,8 +300,8 @@ def access_message(user_profile, message_id): # stream in your realm, so return the message, user_message pair return (message, user_message) -def render_markdown(message, content, realm_id=None, realm_alert_words=None, message_users=None): - # type: (Message, Text, Optional[int], Optional[RealmAlertWords], Set[UserProfile]) -> Text +def render_markdown(message, content, realm=None, realm_alert_words=None, message_users=None): + # type: (Message, Text, Optional[Realm], Optional[RealmAlertWords], Set[UserProfile]) -> Text """Return HTML for given markdown. Bugdown may add properties to the message object such as `mentions_user_ids` and `mentions_wildcard`. These are only on this Django object and are not saved in the @@ -319,9 +320,10 @@ def render_markdown(message, content, realm_id=None, realm_alert_words=None, mes message.alert_words = set() message.links_for_preview = set() - if realm_id is None: - realm_id = message.sender.realm_id - realm_filters_key = realm_id + if realm is None: + realm = message.get_realm() + realm_filters_key = realm.id + if message.sending_client.name == "zephyr_mirror" and message.sender.realm.is_zephyr_mirror_realm: # Use slightly customized Markdown processor for content # delivered via zephyr_mirror @@ -335,7 +337,7 @@ def render_markdown(message, content, realm_id=None, realm_alert_words=None, mes # DO MAIN WORK HERE -- call bugdown to convert rendered_content = bugdown.convert(content, realm_filters_key=realm_filters_key, - message=message, + message=message, message_realm=realm, possible_words=possible_words) if message is not None: diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 8c6380f621..abe03ffb9f 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -1298,6 +1298,46 @@ class SubscriptionAPITest(ZulipTestCase): expected_msg = "%s just created a new stream #**%s**." % (invitee_full_name, invite_streams[0]) self.assertEqual(msg.content, expected_msg) + def test_successful_cross_realm_notification(self): + # type: () -> None + """ + Calling POST /json/users/me/subscriptions in a new realm + should notify with a proper new stream link + """ + (realm, _) = do_create_realm("testrealm", "Test Realm") + + notifications_stream = Stream.objects.get(name='announce', realm=realm) + realm.notifications_stream = notifications_stream + realm.save() + + invite_streams = ["cross_stream"] + + user = get_user_profile_by_email("AARON@zulip.com") + user.realm = realm + user.save() + + # Delete the UserProfile from the cache so the realm change will be + # picked up + cache.cache_delete(cache.user_profile_by_email_cache_key(user.email)) + + result = self.common_subscribe_to_streams( + user.email, + invite_streams, + extra_post_data=dict( + announce='true' + ), + ) + self.assert_json_success(result) + + msg = self.get_last_message() + self.assertEqual(msg.recipient.type, Recipient.STREAM) + self.assertEqual(msg.sender_id, + get_user_profile_by_email('notification-bot@zulip.com').id) + stream_id = Stream.objects.latest('id').id + expected_rendered_msg = '

%s just created a new stream #%s.

' % ( + user.full_name, stream_id, invite_streams[0], invite_streams[0]) + self.assertEqual(msg.rendered_content, expected_rendered_msg) + def test_successful_subscriptions_notifies_with_escaping(self): # type: () -> None """ diff --git a/zerver/views/messages.py b/zerver/views/messages.py index 1376c2646b..046f17055b 100644 --- a/zerver/views/messages.py +++ b/zerver/views/messages.py @@ -965,7 +965,7 @@ def render_message_backend(request, user_profile, content=REQ()): message.content = content message.sending_client = request.client - rendered_content = render_markdown(message, content, realm_id=user_profile.realm_id) + rendered_content = render_markdown(message, content, realm=user_profile.realm) return json_success({"rendered": rendered_content}) @authenticated_json_post_view