From 1a05a5d2f02dca6d74be4c8af2a33e111379d8f1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Jun 2021 13:50:04 +0900 Subject: [PATCH 1/6] Add test covering failure to resolve relative URLs --- osu.Game.Tests/Chat/MessageFormatterTests.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/osu.Game.Tests/Chat/MessageFormatterTests.cs b/osu.Game.Tests/Chat/MessageFormatterTests.cs index b80da928c8..6f9007d670 100644 --- a/osu.Game.Tests/Chat/MessageFormatterTests.cs +++ b/osu.Game.Tests/Chat/MessageFormatterTests.cs @@ -489,5 +489,14 @@ namespace osu.Game.Tests.Chat Assert.AreEqual(result.Links[2].Url, "\uD83D\uDE00"); Assert.AreEqual(result.Links[3].Url, "\uD83D\uDE20"); } + + [Test] + public void TestRelativeExternalLinks() + { + LinkDetails result = MessageFormatter.GetLinkDetails("/relative"); + + Assert.AreEqual(LinkAction.External, result.Action); + Assert.AreEqual("/relative", result.Argument); + } } } From 111bfd4d88abe484931b8789d36653832b005264 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Jun 2021 13:50:20 +0900 Subject: [PATCH 2/6] Fix relative URLs having a null argument after resolution to `LinkDetails` --- osu.Game/Online/Chat/MessageFormatter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Online/Chat/MessageFormatter.cs b/osu.Game/Online/Chat/MessageFormatter.cs index b80720a0aa..54d7030457 100644 --- a/osu.Game/Online/Chat/MessageFormatter.cs +++ b/osu.Game/Online/Chat/MessageFormatter.cs @@ -217,7 +217,7 @@ namespace osu.Game.Online.Chat return new LinkDetails(LinkAction.JoinMultiplayerMatch, args[1]); } - return new LinkDetails(LinkAction.External, null); + return new LinkDetails(LinkAction.External, url); } private static MessageFormatterResult format(string toFormat, int startIndex = 0, int space = 3) From a9f4bc6285dcc84bd9cf4582b6bc2b7557815908 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Jun 2021 14:09:35 +0900 Subject: [PATCH 3/6] Never return a null argument Enable nullable --- osu.Game.Tests/Chat/MessageFormatterTests.cs | 13 +++++++++++-- osu.Game/Online/Chat/MessageFormatter.cs | 8 +++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Chat/MessageFormatterTests.cs b/osu.Game.Tests/Chat/MessageFormatterTests.cs index 6f9007d670..ecb37706b0 100644 --- a/osu.Game.Tests/Chat/MessageFormatterTests.cs +++ b/osu.Game.Tests/Chat/MessageFormatterTests.cs @@ -24,10 +24,10 @@ namespace osu.Game.Tests.Chat [TestCase(LinkAction.OpenBeatmap, "456", "https://dev.ppy.sh/beatmapsets/123#osu/456")] [TestCase(LinkAction.OpenBeatmap, "456", "https://dev.ppy.sh/beatmapsets/123#osu/456?whatever")] [TestCase(LinkAction.OpenBeatmap, "456", "https://dev.ppy.sh/beatmapsets/123/456")] - [TestCase(LinkAction.External, null, "https://dev.ppy.sh/beatmapsets/abc/def")] + [TestCase(LinkAction.External, "https://dev.ppy.sh/beatmapsets/abc/def", "https://dev.ppy.sh/beatmapsets/abc/def")] [TestCase(LinkAction.OpenBeatmapSet, "123", "https://dev.ppy.sh/beatmapsets/123")] [TestCase(LinkAction.OpenBeatmapSet, "123", "https://dev.ppy.sh/beatmapsets/123/whatever")] - [TestCase(LinkAction.External, null, "https://dev.ppy.sh/beatmapsets/abc")] + [TestCase(LinkAction.External, "https://dev.ppy.sh/beatmapsets/abc", "https://dev.ppy.sh/beatmapsets/abc")] public void TestBeatmapLinks(LinkAction expectedAction, string expectedArg, string link) { MessageFormatter.WebsiteRootUrl = "dev.ppy.sh"; @@ -490,6 +490,15 @@ namespace osu.Game.Tests.Chat Assert.AreEqual(result.Links[3].Url, "\uD83D\uDE20"); } + [Test] + public void TestAbsoluteExternalLinks() + { + LinkDetails result = MessageFormatter.GetLinkDetails("https://google.com"); + + Assert.AreEqual(LinkAction.External, result.Action); + Assert.AreEqual("https://google.com", result.Argument); + } + [Test] public void TestRelativeExternalLinks() { diff --git a/osu.Game/Online/Chat/MessageFormatter.cs b/osu.Game/Online/Chat/MessageFormatter.cs index 54d7030457..e08cb1b35f 100644 --- a/osu.Game/Online/Chat/MessageFormatter.cs +++ b/osu.Game/Online/Chat/MessageFormatter.cs @@ -6,6 +6,8 @@ using System.Collections.Generic; using System.Linq; using System.Text.RegularExpressions; +#nullable enable + namespace osu.Game.Online.Chat { public static class MessageFormatter @@ -61,7 +63,7 @@ namespace osu.Game.Online.Chat private static string websiteRootUrl = "osu.ppy.sh"; - private static void handleMatches(Regex regex, string display, string link, MessageFormatterResult result, int startIndex = 0, LinkAction? linkActionOverride = null, char[] escapeChars = null) + private static void handleMatches(Regex regex, string display, string link, MessageFormatterResult result, int startIndex = 0, LinkAction? linkActionOverride = null, char[]? escapeChars = null) { int captureOffset = 0; @@ -170,12 +172,12 @@ namespace osu.Game.Online.Chat } } - return new LinkDetails(LinkAction.External, null); + break; case "osu": // every internal link also needs some kind of argument if (args.Length < 3) - return new LinkDetails(LinkAction.External, null); + break; LinkAction linkType; From 145e42928b08e22902fe74c9e8559e82a39c649f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Jun 2021 16:46:29 +0900 Subject: [PATCH 4/6] Fix remaining null checks --- osu.Game/Graphics/Containers/LinkFlowContainer.cs | 4 ++-- osu.Game/OsuGame.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Graphics/Containers/LinkFlowContainer.cs b/osu.Game/Graphics/Containers/LinkFlowContainer.cs index 054febeec3..f89d7fd779 100644 --- a/osu.Game/Graphics/Containers/LinkFlowContainer.cs +++ b/osu.Game/Graphics/Containers/LinkFlowContainer.cs @@ -43,7 +43,7 @@ namespace osu.Game.Graphics.Containers AddText(text[previousLinkEnd..link.Index]); string displayText = text.Substring(link.Index, link.Length); - string linkArgument = link.Argument ?? link.Url; + string linkArgument = link.Argument; string tooltip = displayText == link.Url ? null : link.Url; AddLink(displayText, link.Action, linkArgument, tooltip); @@ -57,7 +57,7 @@ namespace osu.Game.Graphics.Containers createLink(AddText(text, creationParameters), new LinkDetails(LinkAction.External, url), url); public void AddLink(string text, Action action, string tooltipText = null, Action creationParameters = null) - => createLink(AddText(text, creationParameters), new LinkDetails(LinkAction.Custom, null), tooltipText, action); + => createLink(AddText(text, creationParameters), new LinkDetails(LinkAction.Custom, @"action"), tooltipText, action); public void AddLink(string text, LinkAction action, string argument, string tooltipText = null, Action creationParameters = null) => createLink(AddText(text, creationParameters), new LinkDetails(action, argument), tooltipText); diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index b3b0773eff..c51624341e 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -277,7 +277,7 @@ namespace osu.Game { case LinkAction.OpenBeatmap: // TODO: proper query params handling - if (link.Argument != null && int.TryParse(link.Argument.Contains('?') ? link.Argument.Split('?')[0] : link.Argument, out int beatmapId)) + if (int.TryParse(link.Argument.Contains('?') ? link.Argument.Split('?')[0] : link.Argument, out int beatmapId)) ShowBeatmap(beatmapId); break; From 3668f1861f3369a3819da6d44dd52c648f54c494 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 1 Jun 2021 17:10:09 +0900 Subject: [PATCH 5/6] Fix one more null issue --- osu.Game/Graphics/Containers/LinkFlowContainer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Graphics/Containers/LinkFlowContainer.cs b/osu.Game/Graphics/Containers/LinkFlowContainer.cs index f89d7fd779..458bc8876f 100644 --- a/osu.Game/Graphics/Containers/LinkFlowContainer.cs +++ b/osu.Game/Graphics/Containers/LinkFlowContainer.cs @@ -70,7 +70,7 @@ namespace osu.Game.Graphics.Containers createLink(spriteText.Yield(), new LinkDetails(action, argument), tooltipText); } - public void AddLink(IEnumerable text, LinkAction action = LinkAction.External, string linkArgument = null, string tooltipText = null) + public void AddLink(IEnumerable text, LinkAction action, string linkArgument, string tooltipText = null) { foreach (var t in text) AddArbitraryDrawable(t); From ac761bb0755bb0f377a1c57a09688354b6fea5a1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 2 Jun 2021 14:43:35 +0900 Subject: [PATCH 6/6] Use `string.Empty` instead of arbitrary string --- osu.Game/Graphics/Containers/LinkFlowContainer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Graphics/Containers/LinkFlowContainer.cs b/osu.Game/Graphics/Containers/LinkFlowContainer.cs index 458bc8876f..5ff2fdf6b2 100644 --- a/osu.Game/Graphics/Containers/LinkFlowContainer.cs +++ b/osu.Game/Graphics/Containers/LinkFlowContainer.cs @@ -57,7 +57,7 @@ namespace osu.Game.Graphics.Containers createLink(AddText(text, creationParameters), new LinkDetails(LinkAction.External, url), url); public void AddLink(string text, Action action, string tooltipText = null, Action creationParameters = null) - => createLink(AddText(text, creationParameters), new LinkDetails(LinkAction.Custom, @"action"), tooltipText, action); + => createLink(AddText(text, creationParameters), new LinkDetails(LinkAction.Custom, string.Empty), tooltipText, action); public void AddLink(string text, LinkAction action, string argument, string tooltipText = null, Action creationParameters = null) => createLink(AddText(text, creationParameters), new LinkDetails(action, argument), tooltipText);