FIX: Do not consider code-blocks when parsing mentions (#23280)

We have the max_mentions_per_chat_message site settings; when a user tries 
to mention more users than allowed, no one gets mentioned.

Chat messages may contain code-blocks with strings that look like mentions:

  def foo
    @bar + @baz
  end

The problem is that the parsing code considers these as real mentions and counts 
them when checking the limit. This commit fixes the problem.

app/assets/javascripts/discourse/app/lib/mentions-parser.js

@@ -0,0 +1,36 @@
+export class MentionsParser {
+  constructor(prettyText) {
+    this.prettyText = prettyText;
+  }
+
+  parse(markdown) {
+    const tokens = this.prettyText.parse(markdown);
+    const mentions = this.#parse(tokens);
+    return [...new Set(mentions)];
+  }
+
+  #parse(tokens) {
+    const mentions = [];
+    let insideMention = false;
+    for (const token of tokens) {
+      if (token.children) {
+        this.#parse(token.children).forEach((mention) =>
+          mentions.push(mention)
+        );
+      } else {
+        if (token.type === "mention_open") {
+          insideMention = true;
+        } else if (insideMention && token.type === "text") {
+          mentions.push(this.#extractMention(token.content));
+          insideMention = false;
+        }
+      }
+    }
+
+    return mentions;
+  }
+
+  #extractMention(mention) {
+    return mention.trim().substring(1);
+  }
+}

app/assets/javascripts/discourse/app/lib/plugin-api.js

@@ -2126,7 +2126,7 @@ class PluginApi {
    * Support for setting a Sidebar panel.
    */
   setSidebarPanel(name) {
-    this._lookupContainer("service:sidebar-state").setPanel(name);
+    this._lookupContainer("service:sidebar-state")?.setPanel(name);
   }
 
   /**
@@ -2134,7 +2134,7 @@ class PluginApi {
    * Set combined sidebar section mode. In this mode, sections from all panels are displayed together.
    */
   setCombinedSidebarMode() {
-    this._lookupContainer("service:sidebar-state").setCombinedMode();
+    this._lookupContainer("service:sidebar-state")?.setCombinedMode();
   }
 
   /**
@@ -2158,7 +2158,7 @@ class PluginApi {
    * Hide sidebar switch panels buttons in separated mode.
    */
   hideSidebarSwitchPanelButtons() {
-    this._lookupContainer("service:sidebar-state").hideSwitchPanelButtons();
+    this._lookupContainer("service:sidebar-state")?.hideSwitchPanelButtons();
   }
 
   /**

app/assets/javascripts/discourse/app/lib/text.js

@@ -9,6 +9,7 @@ import { helperContext } from "discourse-common/lib/helpers";
 import { htmlSafe } from "@ember/template";
 import loadScript from "discourse/lib/load-script";
 import { sanitize as textSanitize } from "pretty-text/sanitizer";
+import { MentionsParser } from "discourse/lib/mentions-parser";
 
 function getOpts(opts) {
   let context = helperContext();
@@ -71,10 +72,17 @@ export function sanitizeAsync(text, options) {
 
 export function parseAsync(md, options = {}, env = {}) {
   return loadMarkdownIt().then(() => {
-    return createPrettyText(options).opts.engine.parse(md, env);
+    return createPrettyText(options).parse(md, env);
   });
 }
 
+export async function parseMentions(markdown, options) {
+  await loadMarkdownIt();
+  const prettyText = createPrettyText(options);
+  const mentionsParser = new MentionsParser(prettyText);
+  return mentionsParser.parse(markdown);
+}
+
 function loadMarkdownIt() {
   return new Promise((resolve) => {
     let markdownItURL = Session.currentProp("markdownItURL");

app/assets/javascripts/discourse/tests/unit/lib/text-test.js

@@ -1,5 +1,10 @@
 import { module, test } from "qunit";
-import { cookAsync, excerpt, parseAsync } from "discourse/lib/text";
+import {
+  cookAsync,
+  excerpt,
+  parseAsync,
+  parseMentions,
+} from "discourse/lib/text";
 
 module("Unit | Utility | text", function () {
   test("parseAsync", async function (assert) {
@@ -35,3 +40,36 @@ module("Unit | Utility | text", function () {
     );
   });
 });
+
+module("Unit | Utility | text | parseMentions", function () {
+  test("parses mentions from markdown", async function (assert) {
+    const markdown = "Hey @user1, @user2, @group1, @group2, @here, @all";
+    const mentions = await parseMentions(markdown);
+    assert.deepEqual(mentions, [
+      "user1",
+      "user2",
+      "group1",
+      "group2",
+      "here",
+      "all",
+    ]);
+  });
+
+  test("ignores duplicated mentions", async function (assert) {
+    const markdown = "Hey @john, @john, @john, @john";
+    const mentions = await parseMentions(markdown);
+    assert.deepEqual(mentions, ["john"]);
+  });
+
+  test("ignores mentions in codeblocks", async function (assert) {
+    const markdown = `Hey
+    \`\`\`
+      def foo
+        @bar = true
+      end
+    \`\`\`
+    `;
+    const mentions = await parseMentions(markdown);
+    assert.equal(mentions.length, 0);
+  });
+});

app/assets/javascripts/pretty-text/addon/pretty-text.js

@@ -125,6 +125,10 @@ export default class {
     return result ? result : "";
   }
 
+  parse(markdown, env = {}) {
+    return this.opts.engine.parse(markdown, env);
+  }
+
   sanitize(html) {
     return this.opts.sanitizer(html).trim();
   }

plugins/chat/assets/javascripts/discourse/models/chat-message.js

@@ -4,7 +4,7 @@ import { TrackedArray, TrackedObject } from "@ember-compat/tracked-built-ins";
 import ChatMessageReaction from "discourse/plugins/chat/discourse/models/chat-message-reaction";
 import Bookmark from "discourse/models/bookmark";
 import I18n from "I18n";
-import { generateCookFunction } from "discourse/lib/text";
+import { generateCookFunction, parseMentions } from "discourse/lib/text";
 import transformAutolinks from "discourse/plugins/chat/discourse/lib/transform-auto-links";
 import { getOwner } from "discourse-common/lib/get-owner";
 import discourseLater from "discourse-common/lib/later";
@@ -170,33 +170,11 @@ export default class ChatMessage {
   }
 
   async cook() {
-    const site = getOwner(this).lookup("service:site");
-
     if (this.isDestroyed || this.isDestroying) {
       return;
     }
-
-    const markdownOptions = {
-      featuresOverride:
-        site.markdown_additional_options?.chat?.limited_pretty_text_features,
-      markdownItRules:
-        site.markdown_additional_options?.chat
-          ?.limited_pretty_text_markdown_rules,
-      hashtagTypesInPriorityOrder:
-        site.hashtag_configurations?.["chat-composer"],
-      hashtagIcons: site.hashtag_icons,
-    };
-
-    if (ChatMessage.cookFunction) {
-      this.cooked = ChatMessage.cookFunction(this.message);
-    } else {
-      const cookFunction = await generateCookFunction(markdownOptions);
-      ChatMessage.cookFunction = (raw) => {
-        return transformAutolinks(cookFunction(raw));
-      };
-
-      this.cooked = ChatMessage.cookFunction(this.message);
-    }
+    await this.#ensureCookFunctionInitialized();
+    this.cooked = ChatMessage.cookFunction(this.message);
   }
 
   get read() {
@@ -263,6 +241,10 @@ export default class ChatMessage {
     this.version++;
   }
 
+  async parseMentions() {
+    return await parseMentions(this.message, this.#markdownOptions);
+  }
+
   toJSONDraft() {
     if (
       this.message?.length === 0 &&
@@ -361,6 +343,31 @@ export default class ChatMessage {
     }
   }
 
+  async #ensureCookFunctionInitialized() {
+    if (ChatMessage.cookFunction) {
+      return;
+    }
+
+    const cookFunction = await generateCookFunction(this.#markdownOptions);
+    ChatMessage.cookFunction = (raw) => {
+      return transformAutolinks(cookFunction(raw));
+    };
+  }
+
+  get #markdownOptions() {
+    const site = getOwner(this).lookup("service:site");
+    return {
+      featuresOverride:
+        site.markdown_additional_options?.chat?.limited_pretty_text_features,
+      markdownItRules:
+        site.markdown_additional_options?.chat
+          ?.limited_pretty_text_markdown_rules,
+      hashtagTypesInPriorityOrder:
+        site.hashtag_configurations?.["chat-composer"],
+      hashtagIcons: site.hashtag_icons,
+    };
+  }
+
   #initChatMessageReactionModel(reactions = []) {
     return reactions.map((reaction) => ChatMessageReaction.create(reaction));
   }

plugins/chat/assets/javascripts/discourse/services/chat-composer-warnings-tracker.js

@@ -2,7 +2,6 @@ import Service, { inject as service } from "@ember/service";
 import { ajax } from "discourse/lib/ajax";
 import discourseDebounce from "discourse-common/lib/debounce";
 import { bind } from "discourse-common/utils/decorators";
-import { mentionRegex } from "pretty-text/mentions";
 import { cancel } from "@ember/runloop";
 import { tracked } from "@glimmer/tracking";
 
@@ -34,11 +33,7 @@ export default class ChatComposerWarningsTracker extends Service {
 
   @bind
   reset() {
-    this.unreachableGroupMentions = [];
-    this.unreachableGroupMentions = [];
-    this.overMembersLimitGroupMentions = [];
-    this.tooManyMentions = false;
-    this.channelWideMentionDisallowed = false;
+    this.#resetMentionStats();
     this.mentionsCount = 0;
     cancel(this.mentionsTimer);
   }
@@ -63,61 +58,42 @@ export default class ChatComposerWarningsTracker extends Service {
       return;
     }
 
-    const mentions = this._extractMentions(currentMessage.message);
-    this.mentionsCount = mentions?.length;
+    currentMessage.parseMentions().then((mentions) => {
+      this.mentionsCount = mentions?.length;
 
-    if (this.mentionsCount > 0) {
-      this.tooManyMentions =
-        this.mentionsCount > this.siteSettings.max_mentions_per_chat_message;
+      if (this.mentionsCount > 0) {
+        this.tooManyMentions =
+          this.mentionsCount > this.siteSettings.max_mentions_per_chat_message;
 
-      if (!this.tooManyMentions) {
-        const newMentions = mentions.filter(
-          (mention) => !(mention in this._mentionWarningsSeen)
-        );
+        if (!this.tooManyMentions) {
+          const newMentions = mentions.filter(
+            (mention) => !(mention in this._mentionWarningsSeen)
+          );
 
-        this.channelWideMentionDisallowed =
-          !currentMessage.channel.allowChannelWideMentions &&
-          (mentions.includes("here") || mentions.includes("all"));
+          this.channelWideMentionDisallowed =
+            !currentMessage.channel.allowChannelWideMentions &&
+            (mentions.includes("here") || mentions.includes("all"));
 
-        if (newMentions?.length > 0) {
-          this._recordNewWarnings(newMentions, mentions);
-        } else {
-          this._rebuildWarnings(mentions);
-        }
-      }
-    } else {
-      this.tooManyMentions = false;
-      this.channelWideMentionDisallowed = false;
-      this.unreachableGroupMentions = [];
-      this.overMembersLimitGroupMentions = [];
-    }
-  }
-
-  _extractMentions(message) {
-    const regex = mentionRegex(this.siteSettings.unicode_usernames);
-    const mentions = [];
-    let mentionsLeft = true;
-
-    while (mentionsLeft) {
-      const matches = message.match(regex);
-
-      if (matches) {
-        const mention = matches[1] || matches[2];
-        mentions.push(mention);
-        message = message.replaceAll(`${mention}`, "");
-
-        if (mentions.length > this.siteSettings.max_mentions_per_chat_message) {
-          mentionsLeft = false;
+          if (newMentions?.length > 0) {
+            this.#recordNewWarnings(newMentions, mentions);
+          } else {
+            this.#rebuildWarnings(mentions);
+          }
         }
       } else {
-        mentionsLeft = false;
+        this.#resetMentionStats();
       }
-    }
-
-    return mentions;
+    });
   }
 
-  _recordNewWarnings(newMentions, mentions) {
+  #resetMentionStats() {
+    this.tooManyMentions = false;
+    this.channelWideMentionDisallowed = false;
+    this.unreachableGroupMentions = [];
+    this.overMembersLimitGroupMentions = [];
+  }
+
+  #recordNewWarnings(newMentions, mentions) {
     ajax("/chat/api/mentions/groups.json", {
       data: { mentions: newMentions },
     })
@@ -135,12 +111,12 @@ export default class ChatComposerWarningsTracker extends Service {
           this._mentionWarningsSeen[warning] = MENTION_RESULT["invalid"];
         });
 
-        this._rebuildWarnings(mentions);
+        this.#rebuildWarnings(mentions);
       })
-      .catch(this._rebuildWarnings(mentions));
+      .catch(this.#rebuildWarnings(mentions));
   }
 
-  _rebuildWarnings(mentions) {
+  #rebuildWarnings(mentions) {
     const newWarnings = mentions.reduce(
       (memo, mention) => {
         if (

plugins/chat/test/javascripts/acceptance/mentions-test.js

@@ -0,0 +1,106 @@
+import { acceptance } from "discourse/tests/helpers/qunit-helpers";
+import { test } from "qunit";
+import { fillIn, visit } from "@ember/test-helpers";
+import pretender, { response } from "discourse/tests/helpers/create-pretender";
+
+acceptance("Chat | Mentions", function (needs) {
+  const channelId = 1;
+  const actingUser = {
+    id: 1,
+    username: "acting_user",
+  };
+  const channel = {
+    id: channelId,
+    chatable_id: 1,
+    chatable_type: "Category",
+    meta: { message_bus_last_ids: {}, can_delete_self: true },
+    current_user_membership: { following: true },
+    allow_channel_wide_mentions: false,
+    chatable: { id: 1 },
+  };
+
+  needs.settings({ chat_enabled: true });
+
+  needs.user({
+    ...actingUser,
+    has_chat_enabled: true,
+    chat_channels: {
+      public_channels: [channel],
+      direct_message_channels: [],
+      meta: { message_bus_last_ids: {} },
+      tracking: {},
+    },
+  });
+
+  needs.hooks.beforeEach(function () {
+    pretender.post(`/chat/drafts`, () => response({}));
+    pretender.get(`/chat/api/channels/${channelId}/messages`, () =>
+      response({
+        messages: [],
+        meta: {
+          can_load_more_future: false,
+        },
+      })
+    );
+    pretender.get("/chat/api/mentions/groups.json", () =>
+      response({
+        unreachable: [],
+        over_members_limit: [],
+        invalid: ["and"],
+      })
+    );
+  });
+
+  test("shows warning when mention limit exceeded", async function (assert) {
+    this.siteSettings.max_mentions_per_chat_message = 2;
+
+    await visit(`/chat/c/-/${channelId}`);
+    await fillIn(".chat-composer__input", `Hey @user1 @user2 @user3`);
+
+    assert.dom(".chat-mention-warnings").exists();
+  });
+
+  test("shows warning for @here mentions when channel-wide mentions are disabled", async function (assert) {
+    await visit(`/chat/c/-/${channelId}`);
+    await fillIn(".chat-composer__input", `Hey @here`);
+
+    assert.dom(".chat-mention-warnings").exists();
+  });
+
+  test("shows warning for @all mention when channel-wide mentions are disabled", async function (assert) {
+    await visit(`/chat/c/-/${channelId}`);
+    await fillIn(".chat-composer__input", `Hey @all`);
+
+    assert.dom(".chat-mention-warnings").exists();
+  });
+
+  test("ignores duplicates when counting mentions", async function (assert) {
+    this.siteSettings.max_mentions_per_chat_message = 2;
+
+    await visit(`/chat/c/-/${channelId}`);
+    const mention = `@user1`;
+    await fillIn(
+      ".chat-composer__input",
+      `Hey ${mention} ${mention} ${mention}`
+    );
+
+    assert.dom(".chat-mention-warnings").doesNotExist();
+  });
+
+  test("doesn't consider code-blocks when counting mentions", async function (assert) {
+    this.siteSettings.max_mentions_per_chat_message = 2;
+
+    await visit(`/chat/c/-/${channelId}`);
+    // since @bar is inside a code-block it shouldn't be considered a mention
+    const message = `Hey @user1 @user2
+    \`\`\`
+      def foo
+        @bar = true
+      end
+    \`\`\`
+    `;
+    await fillIn(".chat-composer__input", message);
+
+    assert.dom(".chat-mention-warnings").doesNotExist();
+  });
+});