From c977f2ec9c4b856ecaff3879cc1b50baa2c9e8a1 Mon Sep 17 00:00:00 2001 From: Chris Johnston Date: Sun, 25 Aug 2019 06:28:05 -0700 Subject: [PATCH] fix: #1314 Don't parse tags within code blocks (#1318) * implement a fix for tags being found in code blocks still needs polish, consider this a rough draft * refactor to reuse a local function uses CheckWrappedInCode to check that there are no code blocks that surround the tag being parsed * Add more test coverage of MessageHelper.ParseTags * reset indexes for @ here mention * add a test case to catch error fixed from prev commit * wip commit of most test cases working * fix the Enclosed in block util method * code cleanup * lint whitespace * lint brackets for single line if blocks * move messagehelpertests to the new unit test dir * expose internals to the unit test project this seems to have been breaking the build, since CI would build the merged branch, where rest wasn't exposed to the unit tests --- src/Discord.Net.Commands/AssemblyInfo.cs | 5 +- src/Discord.Net.Core/AssemblyInfo.cs | 1 + src/Discord.Net.Rest/AssemblyInfo.cs | 1 + .../Entities/Messages/MessageHelper.cs | 49 +++++++- src/Discord.Net.WebSocket/AssemblyInfo.cs | 5 +- src/Discord.Net.Webhook/AssemblyInfo.cs | 5 +- .../MessageHelperTests.cs | 118 ++++++++++++++++++ 7 files changed, 177 insertions(+), 7 deletions(-) create mode 100644 test/Discord.Net.Tests.Unit/MessageHelperTests.cs diff --git a/src/Discord.Net.Commands/AssemblyInfo.cs b/src/Discord.Net.Commands/AssemblyInfo.cs index c6b5997b4..bbbaca3be 100644 --- a/src/Discord.Net.Commands/AssemblyInfo.cs +++ b/src/Discord.Net.Commands/AssemblyInfo.cs @@ -1,3 +1,4 @@ -using System.Runtime.CompilerServices; +using System.Runtime.CompilerServices; -[assembly: InternalsVisibleTo("Discord.Net.Tests")] \ No newline at end of file +[assembly: InternalsVisibleTo("Discord.Net.Tests")] +[assembly: InternalsVisibleTo("Discord.Net.Tests.Unit")] diff --git a/src/Discord.Net.Core/AssemblyInfo.cs b/src/Discord.Net.Core/AssemblyInfo.cs index 41ea23859..b7c60f3d3 100644 --- a/src/Discord.Net.Core/AssemblyInfo.cs +++ b/src/Discord.Net.Core/AssemblyInfo.cs @@ -6,4 +6,5 @@ using System.Runtime.CompilerServices; [assembly: InternalsVisibleTo("Discord.Net.WebSocket")] [assembly: InternalsVisibleTo("Discord.Net.Webhook")] [assembly: InternalsVisibleTo("Discord.Net.Commands")] +[assembly: InternalsVisibleTo("Discord.Net.Tests")] [assembly: InternalsVisibleTo("Discord.Net.Tests.Unit")] diff --git a/src/Discord.Net.Rest/AssemblyInfo.cs b/src/Discord.Net.Rest/AssemblyInfo.cs index 9c90919af..5c9351d64 100644 --- a/src/Discord.Net.Rest/AssemblyInfo.cs +++ b/src/Discord.Net.Rest/AssemblyInfo.cs @@ -5,6 +5,7 @@ using System.Runtime.CompilerServices; [assembly: InternalsVisibleTo("Discord.Net.Webhook")] [assembly: InternalsVisibleTo("Discord.Net.Commands")] [assembly: InternalsVisibleTo("Discord.Net.Tests")] +[assembly: InternalsVisibleTo("Discord.Net.Tests.Unit")] [assembly: TypeForwardedTo(typeof(Discord.Embed))] [assembly: TypeForwardedTo(typeof(Discord.EmbedBuilder))] diff --git a/src/Discord.Net.Rest/Entities/Messages/MessageHelper.cs b/src/Discord.Net.Rest/Entities/Messages/MessageHelper.cs index 1189572d6..ef6d9a14c 100644 --- a/src/Discord.Net.Rest/Entities/Messages/MessageHelper.cs +++ b/src/Discord.Net.Rest/Entities/Messages/MessageHelper.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; +using System.Text.RegularExpressions; using System.Threading.Tasks; using Model = Discord.API.Message; @@ -108,14 +109,56 @@ namespace Discord.Rest public static ImmutableArray ParseTags(string text, IMessageChannel channel, IGuild guild, IReadOnlyCollection userMentions) { var tags = ImmutableArray.CreateBuilder(); - int index = 0; + var codeIndex = 0; + + var inlineRegex = new Regex(@"[^\\]?(`).+?[^\\](`)", RegexOptions.Compiled | RegexOptions.Multiline | RegexOptions.Singleline); + var blockRegex = new Regex(@"[^\\]?(```).+?[^\\](```)", RegexOptions.Compiled | RegexOptions.Multiline | RegexOptions.Singleline); + + // checks if the tag being parsed is wrapped in code blocks + bool CheckWrappedCode() + { + // util to check if the index of a tag is within the bounds of the codeblock + bool EnclosedInBlock(Match m) + => m.Groups[1].Index < index && index < m.Groups[2].Index; + + // loop through all code blocks that are before the start of the tag + while (codeIndex < index) + { + var blockMatch = blockRegex.Match(text, codeIndex); + if (blockMatch.Success) + { + if (EnclosedInBlock(blockMatch)) + return true; + // continue if the end of the current code was before the start of the tag + codeIndex += blockMatch.Groups[2].Index + blockMatch.Groups[2].Length; + if (codeIndex < index) + continue; + return false; + } + var inlineMatch = inlineRegex.Match(text, codeIndex); + if (inlineMatch.Success) + { + if (EnclosedInBlock(inlineMatch)) + return true; + // continue if the end of the current code was before the start of the tag + codeIndex += inlineMatch.Groups[2].Index + inlineMatch.Groups[2].Length; + if (codeIndex < index) + continue; + return false; + } + return false; + } + return false; + } + while (true) { index = text.IndexOf('<', index); if (index == -1) break; int endIndex = text.IndexOf('>', index + 1); if (endIndex == -1) break; + if (CheckWrappedCode()) break; string content = text.Substring(index, endIndex - index + 1); if (MentionUtils.TryParseUser(content, out ulong id)) @@ -158,10 +201,12 @@ namespace Discord.Rest } index = 0; + codeIndex = 0; while (true) { index = text.IndexOf("@everyone", index); if (index == -1) break; + if (CheckWrappedCode()) break; var tagIndex = FindIndex(tags, index); if (tagIndex.HasValue) tags.Insert(tagIndex.Value, new Tag(TagType.EveryoneMention, index, "@everyone".Length, 0, guild?.EveryoneRole)); @@ -169,10 +214,12 @@ namespace Discord.Rest } index = 0; + codeIndex = 0; while (true) { index = text.IndexOf("@here", index); if (index == -1) break; + if (CheckWrappedCode()) break; var tagIndex = FindIndex(tags, index); if (tagIndex.HasValue) tags.Insert(tagIndex.Value, new Tag(TagType.HereMention, index, "@here".Length, 0, guild?.EveryoneRole)); diff --git a/src/Discord.Net.WebSocket/AssemblyInfo.cs b/src/Discord.Net.WebSocket/AssemblyInfo.cs index ca3e05e2f..442ec7dd8 100644 --- a/src/Discord.Net.WebSocket/AssemblyInfo.cs +++ b/src/Discord.Net.WebSocket/AssemblyInfo.cs @@ -1,4 +1,5 @@ -using System.Runtime.CompilerServices; +using System.Runtime.CompilerServices; [assembly: InternalsVisibleTo("Discord.Net.Relay")] -[assembly: InternalsVisibleTo("Discord.Net.Tests")] \ No newline at end of file +[assembly: InternalsVisibleTo("Discord.Net.Tests")] +[assembly: InternalsVisibleTo("Discord.Net.Tests.Unit")] diff --git a/src/Discord.Net.Webhook/AssemblyInfo.cs b/src/Discord.Net.Webhook/AssemblyInfo.cs index c6b5997b4..bbbaca3be 100644 --- a/src/Discord.Net.Webhook/AssemblyInfo.cs +++ b/src/Discord.Net.Webhook/AssemblyInfo.cs @@ -1,3 +1,4 @@ -using System.Runtime.CompilerServices; +using System.Runtime.CompilerServices; -[assembly: InternalsVisibleTo("Discord.Net.Tests")] \ No newline at end of file +[assembly: InternalsVisibleTo("Discord.Net.Tests")] +[assembly: InternalsVisibleTo("Discord.Net.Tests.Unit")] diff --git a/test/Discord.Net.Tests.Unit/MessageHelperTests.cs b/test/Discord.Net.Tests.Unit/MessageHelperTests.cs new file mode 100644 index 000000000..0c329f192 --- /dev/null +++ b/test/Discord.Net.Tests.Unit/MessageHelperTests.cs @@ -0,0 +1,118 @@ +using Xunit; +using Discord.Rest; + +namespace Discord +{ + /// + /// Tests for parsing. + /// + public class MessageHelperTests + { + /// + /// Tests that no tags are parsed while in code blocks + /// or inline code. + /// + [Theory] + [InlineData("`@everyone`")] + [InlineData("`<@163184946742034432>`")] + [InlineData("```@everyone```")] + [InlineData("```cs \n @everyone```")] + [InlineData("```cs <@163184946742034432> ```")] + [InlineData("``` test ``` ```cs <@163184946742034432> ```")] + [InlineData("`<:test:537920404019216384>`")] + [InlineData("``` @everyone `")] // discord client handles these weirdly + [InlineData("``` @everyone ``")] + [InlineData("` @here `")] + [InlineData("` @everyone @here <@163184946742034432> <@&163184946742034432> <#163184946742034432> <:test:537920404019216384> `")] + public void ParseTagsInCode(string testData) + { + // don't care that I'm passing in null channels/guilds/users + // as they shouldn't be required + var result = MessageHelper.ParseTags(testData, null, null, null); + Assert.Empty(result); + } + + /// Tests parsing tags that surround inline code or a code block. + [Theory] + [InlineData("`` <@&163184946742034432>")] + [InlineData("``` code block 1 ``` ``` code block 2 ``` <@&163184946742034432>")] + [InlineData("` code block 1 ``` ` code block 2 ``` <@&163184946742034432>")] + [InlineData("<@&163184946742034432> ``` code block 1 ```")] + [InlineData("``` code ``` ``` code ``` @here ``` code ``` ``` more ```")] + [InlineData("``` code ``` @here ``` more ```")] + public void ParseTagsAroundCode(string testData) + { + // don't care that I'm passing in null channels/guilds/users + // as they shouldn't be required + var result = MessageHelper.ParseTags(testData, null, null, null); + Assert.NotEmpty(result); + } + + [Theory] + [InlineData(@"\` @everyone \`")] + [InlineData(@"\`\`\` @everyone \`\`\`")] + [InlineData(@"hey\`\`\`@everyone\`\`\`!!")] + public void IgnoreEscapedCodeBlocks(string testData) + { + var result = MessageHelper.ParseTags(testData, null, null, null); + Assert.NotEmpty(result); + } + + // cannot test parsing a user, as it uses the ReadOnlyCollection arg. + // this could be done if mocked entities are merged in PR #1290 + + /// Tests parsing a mention of a role. + [Theory] + [InlineData("<@&163184946742034432>")] + [InlineData("**<@&163184946742034432>**")] + [InlineData("__<@&163184946742034432>__")] + [InlineData("<><@&163184946742034432>")] + public void ParseRole(string roleTag) + { + var result = MessageHelper.ParseTags(roleTag, null, null, null); + Assert.Contains(result, x => x.Type == TagType.RoleMention); + } + + /// Tests parsing a channel. + [Theory] + [InlineData("<#429115823748284417>")] + [InlineData("**<#429115823748284417>**")] + [InlineData("<><#429115823748284417>")] + public void ParseChannel(string channelTag) + { + var result = MessageHelper.ParseTags(channelTag, null, null, null); + Assert.Contains(result, x => x.Type == TagType.ChannelMention); + } + + /// Tests parsing an emoji. + [Theory] + [InlineData("<:test:537920404019216384>")] + [InlineData("**<:test:537920404019216384>**")] + [InlineData("<><:test:537920404019216384>")] + public void ParseEmoji(string emoji) + { + var result = MessageHelper.ParseTags(emoji, null, null, null); + Assert.Contains(result, x => x.Type == TagType.Emoji); + } + + /// Tests parsing a mention of @everyone. + [Theory] + [InlineData("@everyone")] + [InlineData("**@everyone**")] + public void ParseEveryone(string everyone) + { + var result = MessageHelper.ParseTags(everyone, null, null, null); + Assert.Contains(result, x => x.Type == TagType.EveryoneMention); + } + + /// Tests parsing a mention of @here. + [Theory] + [InlineData("@here")] + [InlineData("**@here**")] + public void ParseHere(string here) + { + var result = MessageHelper.ParseTags(here, null, null, null); + Assert.Contains(result, x => x.Type == TagType.HereMention); + } + } +}