From 35e8f2e4604b265ccc4059bc007bce588cb73553 Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Wed, 16 Oct 2024 16:53:10 +0200 Subject: [PATCH] V15: Rich Text Editor links do not work with query strings and anchors (#17288) * fix: anchors and query strings do not work Since the change from UDIs to localLinks in href, the pattern matched a little too much in the href section completely ignoring any "extras" such as querystrings and anchors after the locallink, which meant that the locallink did not get replaced at all if they were present. This is fixed by limiting the regexp a bit. * fix: legacy links do not follow the same regexp as new links Because we are no longer matching the whole `href` attribute but only some of its contents, we need to fix up the old pattern. It has been extended with matching groups that follow the same pattern as the new links. * feat: allow a-tags to be multiline example: ```html Test ``` * fix: split regex into two parts: first a tokenizer for a-tags and then a type-finder * fix: ensure only "document" and "media" are matching to speed up the pattern * feat: allow a-tags to be multiline --- .../Templates/HtmlLocalLinkParser.cs | 76 +++++++++---------- .../Templates/HtmlLocalLinkParserTests.cs | 36 ++++++++- 2 files changed, 69 insertions(+), 43 deletions(-) diff --git a/src/Umbraco.Core/Templates/HtmlLocalLinkParser.cs b/src/Umbraco.Core/Templates/HtmlLocalLinkParser.cs index dc0fbf281d..ba5d1a844c 100644 --- a/src/Umbraco.Core/Templates/HtmlLocalLinkParser.cs +++ b/src/Umbraco.Core/Templates/HtmlLocalLinkParser.cs @@ -14,11 +14,15 @@ public sealed class HtmlLocalLinkParser // media // other page internal static readonly Regex LocalLinkTagPattern = new( - @"document|media)['""].*?(?href=[""']/{localLink:(?[a-fA-F0-9-]+)})[""'])|((?href=[""']/{localLink:(?[a-fA-F0-9-]+)})[""'].*?type=(['""])(?document|media)(?:['""])))|(?:(?:type=['""](?document|media)['""])|(?:(?href=[""']/{localLink:[a-fA-F0-9-]+})[""'])))[^>]*>", + @"\/?{localLink:(?[a-fA-F0-9-]+)})[^>]*?>", + RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace | RegexOptions.Singleline); + + internal static readonly Regex TypePattern = new( + """type=['"](?(?:media|document))['"]""", RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace); internal static readonly Regex LocalLinkPattern = new( - @"href=""[/]?(?:\{|\%7B)localLink:([a-zA-Z0-9-://]+)(?:\}|\%7D)", + @"href=['""](?\/?(?:\{|\%7B)localLink:(?[a-zA-Z0-9-://]+)(?:\}|\%7D))", RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace); private readonly IPublishedUrlProvider _publishedUrlProvider; @@ -58,24 +62,20 @@ public sealed class HtmlLocalLinkParser { if (tagData.Udi is not null) { - var newLink = "#"; - if (tagData.Udi?.EntityType == Constants.UdiEntityType.Document) + var newLink = tagData.Udi?.EntityType switch { - newLink = _publishedUrlProvider.GetUrl(tagData.Udi.Guid); - } - else if (tagData.Udi?.EntityType == Constants.UdiEntityType.Media) - { - newLink = _publishedUrlProvider.GetMediaUrl(tagData.Udi.Guid); - } - + Constants.UdiEntityType.Document => _publishedUrlProvider.GetUrl(tagData.Udi.Guid), + Constants.UdiEntityType.Media => _publishedUrlProvider.GetMediaUrl(tagData.Udi.Guid), + _ => "" + }; text = StripTypeAttributeFromTag(text, tagData.Udi!.EntityType); - text = text.Replace(tagData.TagHref, "href=\"" + newLink); + text = text.Replace(tagData.TagHref, newLink); } else if (tagData.IntId.HasValue) { var newLink = _publishedUrlProvider.GetUrl(tagData.IntId.Value); - text = text.Replace(tagData.TagHref, "href=\"" + newLink); + text = text.Replace(tagData.TagHref, newLink); } } @@ -83,7 +83,7 @@ public sealed class HtmlLocalLinkParser } // under normal circumstances, the type attribute is preceded by a space - // to cover the rare occasion where it isn't, we first replace with a a space and then without. + // to cover the rare occasion where it isn't, we first replace with a space and then without. private string StripTypeAttributeFromTag(string tag, string type) => tag.Replace($" type=\"{type}\"", string.Empty) .Replace($"type=\"{type}\"", string.Empty); @@ -93,21 +93,22 @@ public sealed class HtmlLocalLinkParser MatchCollection localLinkTagMatches = LocalLinkTagPattern.Matches(text); foreach (Match linkTag in localLinkTagMatches) { - if (linkTag.Groups.Count < 1) + if (Guid.TryParse(linkTag.Groups["guid"].Value, out Guid guid) is false) { continue; } - if (Guid.TryParse(linkTag.Groups["guid"].Value, out Guid guid) is false) + // Find the type attribute + Match typeMatch = TypePattern.Match(linkTag.Value); + if (typeMatch.Success is false) { continue; } yield return new LocalLinkTag( null, - new GuidUdi(linkTag.Groups["type"].Value, guid), - linkTag.Groups["locallink"].Value, - linkTag.Value); + new GuidUdi(typeMatch.Groups["type"].Value, guid), + linkTag.Groups["locallink"].Value); } // also return legacy results for values that have not been migrated @@ -124,25 +125,26 @@ public sealed class HtmlLocalLinkParser MatchCollection tags = LocalLinkPattern.Matches(text); foreach (Match tag in tags) { - if (tag.Groups.Count > 0) + if (tag.Groups.Count <= 0) { - var id = tag.Groups[1].Value; // .Remove(tag.Groups[1].Value.Length - 1, 1); + continue; + } - // The id could be an int or a UDI - if (UdiParser.TryParse(id, out Udi? udi)) - { - var guidUdi = udi as GuidUdi; - if (guidUdi is not null) - { - yield return new LocalLinkTag(null, guidUdi, tag.Value, null); - } - } + var id = tag.Groups["guid"].Value; - if (int.TryParse(id, NumberStyles.Integer, CultureInfo.InvariantCulture, out var intId)) + // The id could be an int or a UDI + if (UdiParser.TryParse(id, out Udi? udi)) + { + if (udi is GuidUdi guidUdi) { - yield return new LocalLinkTag (intId, null, tag.Value, null); + yield return new LocalLinkTag(null, guidUdi, tag.Groups["locallink"].Value); } } + + if (int.TryParse(id, NumberStyles.Integer, CultureInfo.InvariantCulture, out var intId)) + { + yield return new LocalLinkTag (intId, null, tag.Groups["locallink"].Value); + } } } @@ -155,20 +157,10 @@ public sealed class HtmlLocalLinkParser TagHref = tagHref; } - public LocalLinkTag(int? intId, GuidUdi? udi, string tagHref, string? fullTag) - { - IntId = intId; - Udi = udi; - TagHref = tagHref; - FullTag = fullTag; - } - public int? IntId { get; } public GuidUdi? Udi { get; } public string TagHref { get; } - - public string? FullTag { get; } } } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Templates/HtmlLocalLinkParserTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Templates/HtmlLocalLinkParserTests.cs index 298cf5ddc4..5a35a096ee 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Templates/HtmlLocalLinkParserTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Templates/HtmlLocalLinkParserTests.cs @@ -117,6 +117,34 @@ public class HtmlLocalLinkParserTests [TestCase( "world", "world")] + [TestCase( + "

world

world

", + "

world

world

")] + + // attributes order should not matter + [TestCase( + "world", + "world")] + [TestCase( + "world", + "world")] + [TestCase( + "world", + "world")] + + // anchors and query strings + [TestCase( + "world", + "world")] + [TestCase( + "world", + "world")] + + // custom type ignored + [TestCase( + "world", + "world")] + // legacy [TestCase( "hello href=\"{localLink:1234}\" world ", @@ -127,9 +155,15 @@ public class HtmlLocalLinkParserTests [TestCase( "hello href=\"{localLink:umb://document/9931BDE0AAC34BABB838909A7B47570E}\" world ", "hello href=\"/my-test-url\" world ")] + [TestCase( + "hello href=\"{localLink:umb://document/9931BDE0AAC34BABB838909A7B47570E}#anchor\" world ", + "hello href=\"/my-test-url#anchor\" world ")] [TestCase( "hello href=\"{localLink:umb://media/9931BDE0AAC34BABB838909A7B47570E}\" world ", "hello href=\"/media/1001/my-image.jpg\" world ")] + [TestCase( + "hello href='{localLink:umb://media/9931BDE0AAC34BABB838909A7B47570E}' world ", + "hello href='/media/1001/my-image.jpg' world ")] // This one has an invalid char so won't match. [TestCase( @@ -137,7 +171,7 @@ public class HtmlLocalLinkParserTests "hello href=\"{localLink:umb^://document/9931BDE0-AAC3-4BAB-B838-909A7B47570E}\" world ")] [TestCase( "hello href=\"{localLink:umb://document-type/9931BDE0-AAC3-4BAB-B838-909A7B47570E}\" world ", - "hello href=\"#\" world ")] + "hello href=\"\" world ")] public void ParseLocalLinks(string input, string result) { // setup a mock URL provider which we'll use for testing