diff --git a/zerver/lib/markdown/__init__.py b/zerver/lib/markdown/__init__.py index 0334f4c4bf..f934cb7884 100644 --- a/zerver/lib/markdown/__init__.py +++ b/zerver/lib/markdown/__init__.py @@ -1884,11 +1884,22 @@ class UserMentionPattern(markdown.inlinepatterns.InlineProcessor): wildcard = mention.user_mention_matches_wildcard(name) - id_syntax_match = re.match(r"(.+)?\|(?P\d+)$", name) + # For @**|id** and @**name|id** mention syntaxes. + id_syntax_match = re.match(r"(?P.+)?\|(?P\d+)$", name) if id_syntax_match: + full_name = id_syntax_match.group("full_name") id = int(id_syntax_match.group("user_id")) user = db_data["mention_data"].get_user_by_id(id) + + # For @**name|id**, we need to specifically check that + # name matches the full_name of user in mention_data. + # This enforces our decision that + # @**user_1_name|id_for_user_2** should be invalid syntax. + if full_name: + if user and user["full_name"] != full_name: + return None, None, None else: + # For @**name** syntax. user = db_data["mention_data"].get_user_by_name(name) if wildcard: @@ -2445,8 +2456,7 @@ def get_possible_mentions_info(realm_id: int, mention_texts: Set[str]) -> List[F if not mention_texts: return [] - full_names = set() - mention_ids: Set[int] = set() + q_list = set() name_re = r"(?P.+)?\|(?P\d+)$" for mention_text in mention_texts: @@ -2455,15 +2465,15 @@ def get_possible_mentions_info(realm_id: int, mention_texts: Set[str]) -> List[F full_name = name_syntax_match.group("full_name") mention_id = name_syntax_match.group("mention_id") if full_name: - full_names.add(name_syntax_match.group("full_name")) + # For **name|id** mentions as mention_id + # cannot be null inside this block. + q_list.add(Q(full_name__iexact=full_name, id=mention_id)) else: - mention_ids.add(int(mention_id)) + # For **|id** syntax. + q_list.add(Q(id=mention_id)) else: - full_names.add(mention_text) - - q_list = {Q(full_name__iexact=full_name) for full_name in full_names} - id_q_list = {Q(id=id) for id in mention_ids} - q_list |= id_q_list + # For **name** syntax. + q_list.add(Q(full_name__iexact=mention_text)) rows = ( UserProfile.objects.filter( diff --git a/zerver/tests/test_markdown.py b/zerver/tests/test_markdown.py index 1bd4f59528..b84aa0b0db 100644 --- a/zerver/tests/test_markdown.py +++ b/zerver/tests/test_markdown.py @@ -1867,6 +1867,24 @@ class MarkdownTest(ZulipTestCase): ) self.assertEqual(msg.mentions_user_ids, {user_profile.id}) + def test_invalid_mention_not_uses_valid_mention_data(self) -> None: + sender_user_profile = self.example_user("othello") + hamlet = self.example_user("hamlet") + msg = Message(sender=sender_user_profile, sending_client=get_client("test")) + + # Even though King Hamlet will be present in mention data as + # it was was fetched for first mention but second mention is + # incorrect(as it uses hamlet's id) so it should not be able + # to use that data for creating a valid mention. + + content = f"@**King Hamlet|10** and @**aaron|{hamlet.id}**" + self.assertEqual( + render_markdown(msg, content), + f'

' + f"@King Hamlet and @aaron|{hamlet.id}

", + ) + self.assertEqual(msg.mentions_user_ids, {hamlet.id}) + def test_silent_mention_invalid_followed_by_valid(self) -> None: sender_user_profile = self.example_user("othello") user_profile = self.example_user("hamlet")