email_gateway: Disable code block processor for email gateway.

Generally emails are not written with markdown in mind and hence
sometimes render in strange ways. This commit fixes a particular
issue that was causing whitespace before paragraphs to be treated
as code block due to which email content was being rendered in a
box that scrolls in right direction a lot.

Fixes: #7045.
This commit is contained in:
Harshit Bansal
2017-11-03 11:13:17 +00:00
committed by Tim Abbott
parent d7254a0556
commit 65838bb825
5 changed files with 84 additions and 39 deletions

View File

@@ -763,8 +763,8 @@ def send_welcome_bot_response(message):
"Feel free to continue using this space to practice your new messaging "
"skills. Or, try clicking on some of the stream names to your left!")
def render_incoming_message(message, content, user_ids, realm, mention_data=None):
# type: (Message, Text, Set[int], Realm, Optional[bugdown.MentionData]) -> Text
def render_incoming_message(message, content, user_ids, realm, mention_data=None, email_gateway=False):
# type: (Message, Text, Set[int], Realm, Optional[bugdown.MentionData], Optional[bool]) -> Text
realm_alert_words = alert_words_in_realm(realm)
try:
rendered_content = render_markdown(
@@ -774,6 +774,7 @@ def render_incoming_message(message, content, user_ids, realm, mention_data=None
realm_alert_words=realm_alert_words,
user_ids=user_ids,
mention_data=mention_data,
email_gateway=email_gateway,
)
except BugdownRenderingException:
raise JsonableError(_('Unable to render message'))
@@ -1008,8 +1009,8 @@ def get_service_bot_events(sender, service_bot_tuples, mentioned_user_ids,
return event_dict
def do_send_messages(messages_maybe_none):
# type: (Sequence[Optional[MutableMapping[str, Any]]]) -> List[int]
def do_send_messages(messages_maybe_none, email_gateway=False):
# type: (Sequence[Optional[MutableMapping[str, Any]]], Optional[bool]) -> List[int]
# Filter out messages which didn't pass internal_prep_message properly
messages = [message for message in messages_maybe_none if message is not None]
@@ -1072,6 +1073,7 @@ def do_send_messages(messages_maybe_none):
message['active_user_ids'],
message['realm'],
mention_data=message['mention_data'],
email_gateway=email_gateway,
)
message['message'].rendered_content = rendered_content
message['message'].rendered_content_version = bugdown_version
@@ -1869,8 +1871,8 @@ def internal_prep_private_message(realm, sender, recipient_user, content):
)
def internal_send_message(realm, sender_email, recipient_type_name, recipients,
topic_name, content):
# type: (Realm, Text, str, Text, Text, Text) -> None
topic_name, content, email_gateway=False):
# type: (Realm, Text, str, Text, Text, Text, Optional[bool]) -> None
msg = internal_prep_message(realm, sender_email, recipient_type_name, recipients,
topic_name, content)
@@ -1878,7 +1880,7 @@ def internal_send_message(realm, sender_email, recipient_type_name, recipients,
if msg is None:
return
do_send_messages([msg])
do_send_messages([msg], email_gateway=email_gateway)
def internal_send_private_message(realm, sender, recipient_user, content):
# type: (Realm, UserProfile, UserProfile, Text) -> None

View File

@@ -1275,7 +1275,8 @@ class Bugdown(markdown.Extension):
# define default configs
self.config = {
"realm_filters": [kwargs['realm_filters'], "Realm-specific filters for realm"],
"realm": [kwargs['realm'], "Realm name"]
"realm": [kwargs['realm'], "Realm name"],
"code_block_processor_disabled": [kwargs['code_block_processor_disabled'], "Disabled for email gateway"]
}
super().__init__(*args, **kwargs)
@@ -1284,6 +1285,9 @@ class Bugdown(markdown.Extension):
# type: (markdown.Markdown, Dict[str, Any]) -> None
del md.preprocessors['reference']
if self.getConfig('code_block_processor_disabled'):
del md.parser.blockprocessors['code']
for k in ('image_link', 'image_reference', 'automail',
'autolink', 'link', 'reference', 'short_reference',
'escape', 'strong_em', 'emphasis', 'emphasis2',
@@ -1421,7 +1425,7 @@ class Bugdown(markdown.Extension):
if k not in ["paragraph"]:
del md.parser.blockprocessors[k]
md_engines = {} # type: Dict[int, markdown.Markdown]
md_engines = {} # type: Dict[Tuple[int, bool], markdown.Markdown]
realm_filter_data = {} # type: Dict[int, List[Tuple[Text, Text, int]]]
class EscapeHtml(markdown.Extension):
@@ -1431,7 +1435,7 @@ class EscapeHtml(markdown.Extension):
del md.inlinePatterns['html']
def make_md_engine(key, opts):
# type: (int, Dict[str, Any]) -> None
# type: (Tuple[int, bool], Dict[str, Any]) -> None
md_engines[key] = markdown.Markdown(
output_format = 'html',
extensions = [
@@ -1444,7 +1448,8 @@ def make_md_engine(key, opts):
fenced_code.makeExtension(),
EscapeHtml(),
Bugdown(realm_filters=opts["realm_filters"][0],
realm=opts["realm"][0])])
realm=opts["realm"][0],
code_block_processor_disabled=opts["code_block_processor_disabled"][0])])
def subject_links(realm_filters_key, subject):
# type: (int, Text) -> List[Text]
@@ -1458,35 +1463,41 @@ def subject_links(realm_filters_key, subject):
matches += [realm_filter[1] % m.groupdict()]
return matches
def make_realm_filters(realm_filters_key, filters):
# type: (int, List[Tuple[Text, Text, int]]) -> None
def make_realm_filters(realm_filters_key, filters, email_gateway):
# type: (int, List[Tuple[Text, Text, int]], bool) -> None
global md_engines, realm_filter_data
if realm_filters_key in md_engines:
del md_engines[realm_filters_key]
md_engine_key = (realm_filters_key, email_gateway)
if md_engine_key in md_engines:
del md_engines[md_engine_key]
realm_filter_data[realm_filters_key] = filters
# Because of how the Markdown config API works, this has confusing
# large number of layers of dicts/arrays :(
make_md_engine(realm_filters_key,
make_md_engine(md_engine_key,
{"realm_filters": [
filters, "Realm-specific filters for realm_filters_key %s" % (realm_filters_key,)],
"realm": [realm_filters_key, "Realm name"]})
"realm": [realm_filters_key, "Realm name"],
"code_block_processor_disabled": [email_gateway, 'Disabled for email mirror']})
def maybe_update_realm_filters(realm_filters_key):
# type: (Optional[int]) -> None
def maybe_update_realm_filters(realm_filters_key, email_gateway):
# type: (Optional[int], bool) -> None
# If realm_filters_key is None, load all filters
if realm_filters_key is None:
all_filters = all_realm_filters()
all_filters[DEFAULT_BUGDOWN_KEY] = []
for realm_filters_key, filters in all_filters.items():
make_realm_filters(realm_filters_key, filters)
make_realm_filters(realm_filters_key, filters, email_gateway)
# Hack to ensure that getConfig("realm") is right for mirrored Zephyrs
make_realm_filters(ZEPHYR_MIRROR_BUGDOWN_KEY, [])
make_realm_filters(ZEPHYR_MIRROR_BUGDOWN_KEY, [], False)
else:
realm_filters = realm_filters_for_realm(realm_filters_key)
if realm_filters_key not in realm_filter_data or realm_filter_data[realm_filters_key] != realm_filters:
# Data has changed, re-load filters
make_realm_filters(realm_filters_key, realm_filters)
if realm_filters_key not in realm_filter_data or \
realm_filter_data[realm_filters_key] != realm_filters or \
(realm_filters_key, email_gateway) not in md_engines:
# Either realm filters data has changed or an email message has been
# forwarded to an realm by the email gateway for which the markdown
# engine specific to the email gateway hasn't been populated.
make_realm_filters(realm_filters_key, realm_filters, email_gateway)
# We want to log Markdown parser failures, but shouldn't log the actual input
# message for privacy reasons. The compromise is to replace all alphanumeric
@@ -1636,8 +1647,9 @@ def get_stream_name_info(realm, stream_names):
return dct
def do_convert(content, message=None, message_realm=None, possible_words=None, sent_by_bot=False, mention_data=None):
# type: (Text, Optional[Message], Optional[Realm], Optional[Set[Text]], Optional[bool], Optional[MentionData]) -> Text
def do_convert(content, message=None, message_realm=None, possible_words=None, sent_by_bot=False,
mention_data=None, email_gateway=False):
# type: (Text, Optional[Message], Optional[Realm], Optional[Set[Text]], Optional[bool], Optional[MentionData], Optional[bool]) -> Text
"""Convert Markdown to HTML, with Zulip-specific settings and hacks."""
# This logic is a bit convoluted, but the overall goal is to support a range of use cases:
# * Nothing is passed in other than content -> just run default options (e.g. for docs)
@@ -1657,15 +1669,19 @@ def do_convert(content, message=None, message_realm=None, possible_words=None, s
# delivered via zephyr_mirror
realm_filters_key = ZEPHYR_MIRROR_BUGDOWN_KEY
maybe_update_realm_filters(realm_filters_key)
maybe_update_realm_filters(realm_filters_key, email_gateway)
md_engine_key = (realm_filters_key, email_gateway)
if realm_filters_key in md_engines:
_md_engine = md_engines[realm_filters_key]
if md_engine_key in md_engines:
_md_engine = md_engines[md_engine_key]
elif realm_filters_key is not None and email_gateway:
maybe_update_realm_filters(realm_filters_key, email_gateway)
_md_engine = md_engines[md_engine_key]
else:
if DEFAULT_BUGDOWN_KEY not in md_engines:
maybe_update_realm_filters(realm_filters_key=None)
maybe_update_realm_filters(realm_filters_key=None, email_gateway=False)
_md_engine = md_engines[DEFAULT_BUGDOWN_KEY]
_md_engine = md_engines[(DEFAULT_BUGDOWN_KEY, email_gateway)]
# Reset the parser; otherwise it will get slower over time.
_md_engine.reset()
@@ -1754,9 +1770,10 @@ def bugdown_stats_finish():
bugdown_total_requests += 1
bugdown_total_time += (time.time() - bugdown_time_start)
def convert(content, message=None, message_realm=None, possible_words=None, sent_by_bot=False, mention_data=None):
# type: (Text, Optional[Message], Optional[Realm], Optional[Set[Text]], Optional[bool], Optional[MentionData]) -> Text
def convert(content, message=None, message_realm=None, possible_words=None, sent_by_bot=False,
mention_data=None, email_gateway=False):
# type: (Text, Optional[Message], Optional[Realm], Optional[Set[Text]], Optional[bool], Optional[MentionData], Optional[bool]) -> Text
bugdown_stats_start()
ret = do_convert(content, message, message_realm, possible_words, sent_by_bot, mention_data)
ret = do_convert(content, message, message_realm, possible_words, sent_by_bot, mention_data, email_gateway)
bugdown_stats_finish()
return ret

View File

@@ -203,7 +203,8 @@ def send_zulip(sender, stream, topic, content):
"stream",
stream.name,
topic[:60],
content[:2000])
content[:2000],
email_gateway=True)
def valid_stream(stream_name, token):
# type: (Text, Text) -> bool

View File

@@ -499,8 +499,9 @@ 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=None, realm_alert_words=None, user_ids=None, mention_data=None):
# type: (Message, Text, Optional[Realm], Optional[RealmAlertWords], Optional[Set[int]], Optional[bugdown.MentionData]) -> Text
def render_markdown(message, content, realm=None, realm_alert_words=None, user_ids=None,
mention_data=None, email_gateway=False):
# type: (Message, Text, Optional[Realm], Optional[RealmAlertWords], Optional[Set[int]], Optional[bugdown.MentionData], Optional[bool]) -> Text
"""Return HTML for given markdown. Bugdown may add properties to the
message object such as `mentions_user_ids`, `mentions_user_group_ids`, and
`mentions_wildcard`. These are only on this Django object and are not
@@ -543,6 +544,7 @@ def render_markdown(message, content, realm=None, realm_alert_words=None, user_i
possible_words=possible_words,
sent_by_bot=sent_by_bot,
mention_data=mention_data,
email_gateway=email_gateway
)
if message is not None:

View File

@@ -275,7 +275,9 @@ class BugdownTest(ZulipTestCase):
realm = Realm.objects.create(string_id='file_links_test')
bugdown.make_md_engine(
realm.id,
{'realm_filters': [[], u'file_links_test.example.com'], 'realm': [u'file_links_test.example.com', 'Realm name']})
{'realm_filters': [[], u'file_links_test.example.com'],
'realm': [u'file_links_test.example.com', 'Realm name'],
'code_block_processor_disabled': [False, 'Disabled for email gateway']})
converted = bugdown.convert(msg, message_realm=realm)
self.assertEqual(converted, '<p>Check out this file file:///Volumes/myserver/Users/Shared/pi.py</p>')
@@ -659,7 +661,7 @@ class BugdownTest(ZulipTestCase):
realm_filter.save()
bugdown.realm_filter_data = {}
bugdown.maybe_update_realm_filters(None)
bugdown.maybe_update_realm_filters(None, False)
all_filters = bugdown.realm_filter_data
zulip_filters = all_filters[realm.id]
self.assertEqual(len(zulip_filters), 1)
@@ -1146,6 +1148,27 @@ class BugdownTest(ZulipTestCase):
'javascript://example.com/invalidURL',
)
def test_disabled_code_block_processor(self):
# type: () -> None
msg = "Hello,\n\n" + \
" I am writing this message to test something. I am writing this message to test something."
converted = bugdown_convert(msg)
expected_output = '<p>Hello,</p>\n' + \
'<div class="codehilite"><pre><span></span>I am writing this message to test something. I am writing this message to test something.\n' + \
'</pre></div>'
self.assertEqual(converted, expected_output)
realm = Realm.objects.create(string_id='code_block_processor_test')
bugdown.make_md_engine(
realm.id,
{'realm_filters': [[], u'file_links_test.example.com'],
'realm': [u'file_links_test.example.com', 'Realm name'],
'code_block_processor_disabled': [True, 'Disabled for email gateway']})
converted = bugdown.convert(msg, message_realm=realm, email_gateway=True)
expected_output = '<p>Hello,</p>\n' + \
'<p>I am writing this message to test something. I am writing this message to test something.</p>'
self.assertEqual(converted, expected_output)
class BugdownApiTests(ZulipTestCase):
def test_render_message_api(self):
# type: () -> None