From 241bf7877b365ef509879e6284b93812c4f07091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 1 May 2024 08:54:10 +0200 Subject: [PATCH 1/4] Add test coverage for expected filtering behaviour --- osu.Server.Spectator.Tests/ChatFiltersTest.cs | 98 +++++++++++++++++++ .../Database/Models/chat_filter.cs | 2 + 2 files changed, 100 insertions(+) create mode 100644 osu.Server.Spectator.Tests/ChatFiltersTest.cs diff --git a/osu.Server.Spectator.Tests/ChatFiltersTest.cs b/osu.Server.Spectator.Tests/ChatFiltersTest.cs new file mode 100644 index 00000000..809486b2 --- /dev/null +++ b/osu.Server.Spectator.Tests/ChatFiltersTest.cs @@ -0,0 +1,98 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Threading.Tasks; +using Moq; +using osu.Game.Online.Multiplayer; +using osu.Server.Spectator.Database; +using osu.Server.Spectator.Database.Models; +using Xunit; + +namespace osu.Server.Spectator.Tests +{ + public class ChatFiltersTest + { + private readonly Mock factoryMock; + private readonly Mock databaseMock; + + public ChatFiltersTest() + { + factoryMock = new Mock(); + databaseMock = new Mock(); + + factoryMock.Setup(factory => factory.GetInstance()).Returns(databaseMock.Object); + } + + [Theory] + [InlineData("bad phrase", "good phrase")] + [InlineData("WHAT HAPPENS IF I SAY BAD THING IN CAPS", "WHAT HAPPENS IF I SAY good THING IN CAPS")] + [InlineData("thing is bad", "thing is good")] + [InlineData("look at this badness", "look at this goodness")] + public async Task TestPlainFilterReplacement(string input, string expectedOutput) + { + databaseMock.Setup(db => db.GetAllChatFiltersAsync()).ReturnsAsync([ + new chat_filter { match = "bad", replacement = "good" }, + new chat_filter { match = "fullword", replacement = "okay", whitespace_delimited = true }, + new chat_filter { match = "absolutely forbidden", replacement = "", block = true } + ]); + + var filters = new ChatFilters(factoryMock.Object); + + Assert.Equal(expectedOutput, await filters.FilterAsync(input)); + } + + [Theory] + [InlineData("fullword at the start", "okay at the start")] + [InlineData("FULLWORD IN CAPS!!", "okay IN CAPS!!")] + [InlineData("at the end is fullword", "at the end is okay")] + [InlineData("middle is where the fullword is", "middle is where the okay is")] + [InlineData("anotherfullword is not replaced", "anotherfullword is not replaced")] + [InlineData("fullword fullword2", "okay great")] + [InlineData("fullwordfullword2", "fullwordfullword2")] + [InlineData("i do a delimiter/inside", "i do a nice try")] + public async Task TestWhitespaceDelimitedFilterReplacement(string input, string expectedOutput) + { + databaseMock.Setup(db => db.GetAllChatFiltersAsync()).ReturnsAsync([ + new chat_filter { match = "bad", replacement = "good" }, + new chat_filter { match = "fullword", replacement = "okay", whitespace_delimited = true }, + new chat_filter { match = "fullword2", replacement = "great", whitespace_delimited = true }, + new chat_filter { match = "delimiter/inside", replacement = "nice try", whitespace_delimited = true }, + new chat_filter { match = "absolutely forbidden", replacement = "", block = true } + ]); + + var filters = new ChatFilters(factoryMock.Object); + + Assert.Equal(expectedOutput, await filters.FilterAsync(input)); + } + + [Theory] + [InlineData("absolutely forbidden")] + [InlineData("sPoNGeBoB SaYS aBSolUtElY FoRbIdDeN")] + [InlineData("this is absolutely forbidden full stop!!!")] + public async Task TestBlockingFilter(string input) + { + databaseMock.Setup(db => db.GetAllChatFiltersAsync()).ReturnsAsync([ + new chat_filter { match = "bad", replacement = "good" }, + new chat_filter { match = "fullword", replacement = "okay", whitespace_delimited = true }, + new chat_filter { match = "absolutely forbidden", replacement = "", block = true } + ]); + + var filters = new ChatFilters(factoryMock.Object); + + await Assert.ThrowsAsync(() => filters.FilterAsync(input)); + } + + [Fact] + public async Task TestLackOfBlockingFilters() + { + databaseMock.Setup(db => db.GetAllChatFiltersAsync()).ReturnsAsync([ + new chat_filter { match = "bad", replacement = "good" }, + new chat_filter { match = "fullword", replacement = "okay", whitespace_delimited = true }, + ]); + + var filters = new ChatFilters(factoryMock.Object); + + await filters.FilterAsync("this should be completely fine"); // should not throw + } + } +} diff --git a/osu.Server.Spectator/Database/Models/chat_filter.cs b/osu.Server.Spectator/Database/Models/chat_filter.cs index c362aac5..662022e6 100644 --- a/osu.Server.Spectator/Database/Models/chat_filter.cs +++ b/osu.Server.Spectator/Database/Models/chat_filter.cs @@ -13,5 +13,7 @@ public class chat_filter public long id { get; set; } public string match { get; set; } = string.Empty; public string replacement { get; set; } = string.Empty; + public bool block { get; set; } + public bool whitespace_delimited { get; set; } } } From 34b885e97bc1cde2ee329426440421b4a38d7455 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 1 May 2024 09:21:28 +0200 Subject: [PATCH 2/4] Add extended capabilities for chat filters --- osu.Server.Spectator/ChatFilters.cs | 68 ++++++++++++++++++++++++----- 1 file changed, 57 insertions(+), 11 deletions(-) diff --git a/osu.Server.Spectator/ChatFilters.cs b/osu.Server.Spectator/ChatFilters.cs index f583fe6c..610e3e39 100644 --- a/osu.Server.Spectator/ChatFilters.cs +++ b/osu.Server.Spectator/ChatFilters.cs @@ -1,9 +1,12 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System.Collections.Immutable; -using System.Text; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text.RegularExpressions; using System.Threading.Tasks; +using osu.Game.Online.Multiplayer; using osu.Server.Spectator.Database; using osu.Server.Spectator.Database.Models; @@ -12,7 +15,11 @@ namespace osu.Server.Spectator public class ChatFilters { private readonly IDatabaseFactory factory; - private ImmutableArray? filters; + + private bool filtersInitialised; + private Regex? blockRegex; + private List<(string match, string replacement)> nonWhitespaceDelimitedReplaces = new List<(string, string)>(); + private List<(Regex match, string replacement)> whitespaceDelimitedReplaces = new List<(Regex, string)>(); public ChatFilters(IDatabaseFactory factory) { @@ -21,18 +28,57 @@ public ChatFilters(IDatabaseFactory factory) public async Task FilterAsync(string input) { - if (filters == null) + if (!filtersInitialised) + await initialiseFilters(); + + if (blockRegex?.Match(input).Success == true) + throw new InvalidStateException(string.Empty); + + // this is a touch inefficient due to string allocs, + // but there's no way for `StringBuilder` to do case-insensitive replaces on strings + // or any replaces on regexes at all... + + foreach (var filter in nonWhitespaceDelimitedReplaces) + input = input.Replace(filter.match, filter.replacement, StringComparison.OrdinalIgnoreCase); + + foreach (var filter in whitespaceDelimitedReplaces) + input = filter.match.Replace(input, filter.replacement); + + return input; + } + + private async Task initialiseFilters() + { + using var db = factory.GetInstance(); + var allFilters = await db.GetAllChatFiltersAsync(); + + var blockingFilters = allFilters.Where(f => f.block).ToArray(); + if (blockingFilters.Length > 0) + blockRegex = new Regex(string.Join('|', blockingFilters.Select(singleFilterRegex)), RegexOptions.Compiled | RegexOptions.IgnoreCase); + + foreach (var nonBlockingFilter in allFilters.Where(f => !f.block)) { - using var db = factory.GetInstance(); - filters = (await db.GetAllChatFiltersAsync()).ToImmutableArray(); + if (nonBlockingFilter.whitespace_delimited) + { + whitespaceDelimitedReplaces.Add(( + new Regex(singleFilterRegex(nonBlockingFilter), RegexOptions.Compiled | RegexOptions.IgnoreCase), + nonBlockingFilter.replacement)); + } + else + { + nonWhitespaceDelimitedReplaces.Add((nonBlockingFilter.match, nonBlockingFilter.replacement)); + } } - var stringBuilder = new StringBuilder(input); - - foreach (var filter in filters) - stringBuilder.Replace(filter.match, filter.replacement); + filtersInitialised = true; + } - return stringBuilder.ToString(); + private static string singleFilterRegex(chat_filter filter) + { + string term = Regex.Escape(filter.match); + if (filter.whitespace_delimited) + term = $@"\b{term}\b"; + return term; } } } From 0f78897ca6debde589842e4071da552d44372a5b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 May 2024 20:44:29 +0800 Subject: [PATCH 3/4] Misc inspection cleanups --- osu.Server.Spectator.sln.DotSettings | 11 +++++++++++ osu.Server.Spectator/ChatFilters.cs | 5 +++-- osu.Server.Spectator/Program.cs | 2 -- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/osu.Server.Spectator.sln.DotSettings b/osu.Server.Spectator.sln.DotSettings index eb122e76..d4038fa7 100644 --- a/osu.Server.Spectator.sln.DotSettings +++ b/osu.Server.Spectator.sln.DotSettings @@ -769,9 +769,19 @@ See the LICENCE file in the repository root for full licence text. <Policy Inspect="True" Prefix="" Suffix="" Style="aa_bb" /> <Policy Inspect="True" Prefix="" Suffix="" Style="AA_BB" /> <Policy Inspect="True" Prefix="" Suffix="" Style="AaBb" /> + <Policy><Descriptor Staticness="Static" AccessRightKinds="Private" Description="Static readonly fields (private)"><ElementKinds><Kind Name="READONLY_FIELD" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="aa_bb" /></Policy> + <Policy><Descriptor Staticness="Any" AccessRightKinds="Private" Description="Constant fields (private)"><ElementKinds><Kind Name="CONSTANT_FIELD" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="aa_bb" /></Policy> + <Policy><Descriptor Staticness="Any" AccessRightKinds="Any" Description="Type parameters"><ElementKinds><Kind Name="TYPE_PARAMETER" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="AaBb" /></Policy> + <Policy><Descriptor Staticness="Instance" AccessRightKinds="Private" Description="Instance fields (private)"><ElementKinds><Kind Name="FIELD" /><Kind Name="READONLY_FIELD" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="aaBb"><ExtraRule Prefix="_" Suffix="" Style="aaBb" /></Policy></Policy> + <Policy><Descriptor Staticness="Any" AccessRightKinds="Protected, ProtectedInternal, Internal, Public, PrivateProtected" Description="Constant fields (not private)"><ElementKinds><Kind Name="CONSTANT_FIELD" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="AA_BB" /></Policy> + <Policy><Descriptor Staticness="Any" AccessRightKinds="Any" Description="Local functions"><ElementKinds><Kind Name="LOCAL_FUNCTION" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /></Policy> + <Policy><Descriptor Staticness="Any" AccessRightKinds="Any" Description="Enum members"><ElementKinds><Kind Name="ENUM_MEMBER" /></ElementKinds></Descriptor><Policy Inspect="False" Prefix="" Suffix="" Style="AaBb" /></Policy> <Policy><Descriptor Staticness="Static, Instance" AccessRightKinds="Private" Description="private methods"><ElementKinds><Kind Name="ASYNC_METHOD" /><Kind Name="METHOD" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /></Policy> <Policy><Descriptor Staticness="Static, Instance" AccessRightKinds="Protected, ProtectedInternal, Internal, Public" Description="internal/protected/public methods"><ElementKinds><Kind Name="ASYNC_METHOD" /><Kind Name="METHOD" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="AaBb" /></Policy> <Policy><Descriptor Staticness="Static, Instance" AccessRightKinds="Private" Description="private properties"><ElementKinds><Kind Name="PROPERTY" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /></Policy> + <Policy><Descriptor Staticness="Any" AccessRightKinds="Any" Description="Local constants"><ElementKinds><Kind Name="LOCAL_CONSTANT" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="aa_bb" /></Policy> + <Policy><Descriptor Staticness="Static" AccessRightKinds="Protected, ProtectedInternal, Internal, Public, PrivateProtected" Description="Static readonly fields (not private)"><ElementKinds><Kind Name="READONLY_FIELD" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="AA_BB" /></Policy> + <Policy><Descriptor Staticness="Static" AccessRightKinds="Private" Description="Static fields (private)"><ElementKinds><Kind Name="FIELD" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /></Policy> <Policy><Descriptor Staticness="Static, Instance" AccessRightKinds="Protected, ProtectedInternal, Internal, Public" Description="internal/protected/public properties"><ElementKinds><Kind Name="PROPERTY" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="AaBb" /></Policy> <Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /> <Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /> @@ -837,6 +847,7 @@ See the LICENCE file in the repository root for full licence text. True True True + True TestFolder True True diff --git a/osu.Server.Spectator/ChatFilters.cs b/osu.Server.Spectator/ChatFilters.cs index 610e3e39..b51c27ef 100644 --- a/osu.Server.Spectator/ChatFilters.cs +++ b/osu.Server.Spectator/ChatFilters.cs @@ -18,8 +18,9 @@ public class ChatFilters private bool filtersInitialised; private Regex? blockRegex; - private List<(string match, string replacement)> nonWhitespaceDelimitedReplaces = new List<(string, string)>(); - private List<(Regex match, string replacement)> whitespaceDelimitedReplaces = new List<(Regex, string)>(); + + private readonly List<(string match, string replacement)> nonWhitespaceDelimitedReplaces = new List<(string, string)>(); + private readonly List<(Regex match, string replacement)> whitespaceDelimitedReplaces = new List<(Regex, string)>(); public ChatFilters(IDatabaseFactory factory) { diff --git a/osu.Server.Spectator/Program.cs b/osu.Server.Spectator/Program.cs index aa75d3ba..7ef2abda 100644 --- a/osu.Server.Spectator/Program.cs +++ b/osu.Server.Spectator/Program.cs @@ -5,11 +5,9 @@ using System.IO; using System.Net; using Microsoft.AspNetCore.Hosting; -using Microsoft.AspNetCore.SignalR; using Microsoft.Extensions.Hosting; using osu.Framework.Logging; using osu.Framework.Platform; -using Sentry; using StatsdClient; namespace osu.Server.Spectator From c7c2f8c9713826350a885cbcd5ce04726336d3d6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 May 2024 20:50:08 +0800 Subject: [PATCH 4/4] Add arbitrary exception description --- osu.Server.Spectator/ChatFilters.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Server.Spectator/ChatFilters.cs b/osu.Server.Spectator/ChatFilters.cs index b51c27ef..85c4ee0d 100644 --- a/osu.Server.Spectator/ChatFilters.cs +++ b/osu.Server.Spectator/ChatFilters.cs @@ -33,7 +33,7 @@ public async Task FilterAsync(string input) await initialiseFilters(); if (blockRegex?.Match(input).Success == true) - throw new InvalidStateException(string.Empty); + throw new InvalidStateException("You can't say that."); // this is a touch inefficient due to string allocs, // but there's no way for `StringBuilder` to do case-insensitive replaces on strings