From 3796292913b436560db3f55a8645303be9d2d1f4 Mon Sep 17 00:00:00 2001
From: Harshit Bansal <harshitbansal2015@gmail.com>
Date: Sun, 30 Jul 2017 19:07:59 +0000
Subject: [PATCH] markdown: Fix the rendering of realm filters.

A realm filter should match only after the start of a line, whitespace
or opening delimiters. But markdown was not configured to respect those
rules which was causing some weird rendering behavior. This commit fixes
the regex used for matching realm filters. On the backend we are using
regex with negative lookbehind to perform matches but since javascript
regex don't support lookbehind we are using a workaround on the frontend
using `contains_backend_only_syntax()` function which detects if a realm
filter can be rendered correctly by backend only and if so it stops the
message from getting echoed locally.

Fixes: #5154.
---
 frontend_tests/node_tests/markdown.js | 19 ++++++++++++++++++-
 static/js/markdown.js                 | 20 +++++++++++++++++++-
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/frontend_tests/node_tests/markdown.js b/frontend_tests/node_tests/markdown.js
index 883e1525f6..808e0ffd21 100644
--- a/frontend_tests/node_tests/markdown.js
+++ b/frontend_tests/node_tests/markdown.js
@@ -229,8 +229,15 @@ var bugdown_data = JSON.parse(fs.readFileSync(path.join(__dirname, '../../zerver
          expected: '<p><img alt=":poop:" class="emoji" src="/static/generated/emoji/images/emoji/unicode/1f4a9.png" title="poop"></p>'},
         {input: '\u{1f937}',
          expected: '<p>\u{1f937}</p>' },
+        // Test only those realm filters which don't return True for
+        // `contains_backend_only_syntax()`. Those which return True
+        // are tested separately.
         {input: 'This is a realm filter #1234 with text after it',
          expected: '<p>This is a realm filter <a href="https://trac.zulip.net/ticket/1234" target="_blank" title="https://trac.zulip.net/ticket/1234">#1234</a> with text after it</p>'},
+        {input: '#1234is not a realm filter.',
+         expected: '<p>#1234is not a realm filter.</p>'},
+        {input: 'A pattern written as #1234is not a realm filter.',
+         expected: '<p>A pattern written as #1234is not a realm filter.</p>'},
         {input: 'This is a realm filter with ZGROUP_123:45 groups',
          expected: '<p>This is a realm filter with <a href="https://zone_45.zulip.net/ticket/123" target="_blank" title="https://zone_45.zulip.net/ticket/123">ZGROUP_123:45</a> groups</p>'},
         {input: 'This is an !avatar(cordelia@zulip.com) of Cordelia Lear',
@@ -335,12 +342,22 @@ var bugdown_data = JSON.parse(fs.readFileSync(path.join(__dirname, '../../zerver
     assert(message.flags.indexOf('mentioned') === -1);
 }());
 
+(function test_backend_only_realm_filters() {
+    var backend_only_realm_filters = [
+        'Here is the PR-#123.',
+        'Function abc() was introduced in (PR)#123.',
+    ];
+    backend_only_realm_filters.forEach(function (content) {
+        assert.equal(markdown.contains_backend_only_syntax(content), true);
+    });
+}());
+
 (function test_python_to_js_filter() {
     // The only way to reach python_to_js_filter is indirectly, hence the call
     // to set_realm_filters.
     markdown.set_realm_filters([[ '/a(?im)a/g'], [ '/a(?L)a/g' ]]);
     var actual_value = (marked.InlineLexer.rules.zulip.realm_filters);
-    var expected_value = [ /\/aa\/g/gim, /\/aa\/g/g ];
+    var expected_value = [ /\/aa\/g(?![\w])/gim, /\/aa\/g(?![\w])/g ];
     assert.deepEqual(actual_value, expected_value);
 }());
 
diff --git a/static/js/markdown.js b/static/js/markdown.js
index 95ef63eb67..5e18ab5784 100644
--- a/static/js/markdown.js
+++ b/static/js/markdown.js
@@ -30,7 +30,16 @@ exports.contains_backend_only_syntax = function (content) {
     var markedup = _.find(backend_only_markdown_re, function (re) {
         return re.test(content);
     });
-    return markedup !== undefined;
+
+    // If a realm filter doesn't start with some specified characters
+    // then don't render it locally. It is workaround for the fact that
+    // javascript regex doesn't support lookbehind.
+    var false_filter_match = _.find(realm_filter_list, function (re) {
+        var pattern = /(?:[^\s'"\(,:<])/.source + re[0].source + /(?![\w])/.source;
+        var regex = new RegExp(pattern);
+        return regex.test(content);
+    });
+    return markedup !== undefined || false_filter_match !== undefined;
 };
 
 function push_uniquely(lst, elem) {
@@ -221,6 +230,15 @@ function python_to_js_filter(pattern, url) {
         });
         pattern = pattern.replace(inline_flag_re, "");
     }
+    // Ideally we should have been checking that realm filters
+    // begin with certain characters but since there is no
+    // support for negative lookbehind in javascript, we check
+    // for this condition in `contains_backend_only_syntax()`
+    // function. If the condition is satisfied then the message
+    // is rendered locally, otherwise, we return false there and
+    // message is rendered on the backend which has proper support
+    // for negative lookbehind.
+    pattern = pattern + /(?![\w])/.source;
     return [new RegExp(pattern, js_flags), url];
 }