From 31b1a3eb2b78066c17db783e2d8b717475627d19 Mon Sep 17 00:00:00 2001 From: Vitor Rodrigues Date: Thu, 12 Aug 2021 17:05:43 +0200 Subject: [PATCH 1/3] Fixed incorrect node url being returned when no suitable domain exists for culture #10827 --- src/Umbraco.Core/Routing/SiteDomainMapper.cs | 16 ++-------------- .../Routing/UrlProviderExtensions.cs | 7 +++++++ .../Routing/SiteDomainMapperTests.cs | 4 +--- 3 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/Umbraco.Core/Routing/SiteDomainMapper.cs b/src/Umbraco.Core/Routing/SiteDomainMapper.cs index 8583784369..d877385ef1 100644 --- a/src/Umbraco.Core/Routing/SiteDomainMapper.cs +++ b/src/Umbraco.Core/Routing/SiteDomainMapper.cs @@ -336,8 +336,7 @@ namespace Umbraco.Cms.Core.Routing if (qualifiedSites == null) { return domainAndUris.FirstOrDefault(x => x.Culture.InvariantEquals(culture)) - ?? domainAndUris.FirstOrDefault(x => x.Culture.InvariantEquals(defaultCulture)) - ?? domainAndUris.First(); + ?? domainAndUris.FirstOrDefault(x => x.Culture.InvariantEquals(defaultCulture)); } // find a site that contains the current authority @@ -350,18 +349,7 @@ namespace Umbraco.Cms.Core.Routing : domainAndUris.FirstOrDefault(d => currentSite.Value.Contains(d.Uri.GetLeftPart(UriPartial.Authority))); // no match means that either current does not belong to a site, or the site it belongs to - // does not contain any of domainAndUris. Yet we have to return something. here, it becomes - // a bit arbitrary. - - // look through sites in order and pick the first domainAndUri that belongs to a site - ret = ret ?? qualifiedSites - .Where(site => site.Key != currentSite.Key) - .Select(site => domainAndUris.FirstOrDefault(domainAndUri => site.Value.Contains(domainAndUri.Uri.GetLeftPart(UriPartial.Authority)))) - .FirstOrDefault(domainAndUri => domainAndUri != null); - - // random, really - ret = ret ?? domainAndUris.FirstOrDefault(x => x.Culture.InvariantEquals(culture)) ?? domainAndUris.First(); - + // does not contain any of domainAndUris. return ret; } diff --git a/src/Umbraco.Core/Routing/UrlProviderExtensions.cs b/src/Umbraco.Core/Routing/UrlProviderExtensions.cs index 71ec01d0b5..c3e2bdee77 100644 --- a/src/Umbraco.Core/Routing/UrlProviderExtensions.cs +++ b/src/Umbraco.Core/Routing/UrlProviderExtensions.cs @@ -247,6 +247,13 @@ namespace Umbraco.Extensions return Attempt.Succeed(urlInfo); } + // collisions with a different culture of the same content can never be routed. + if (!culture.InvariantEquals(pcr.Culture)) + { + var urlInfo = UrlInfo.Message(textService.Localize("content", "routeErrorCannotRoute"), culture); + return Attempt.Succeed(urlInfo); + } + // no collision return Attempt.Fail(); } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Routing/SiteDomainMapperTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Routing/SiteDomainMapperTests.cs index c9109a5c14..2582059182 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Routing/SiteDomainMapperTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Routing/SiteDomainMapperTests.cs @@ -191,9 +191,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Routing siteDomainMapper.AddSite("site3", "domain3.com", "domain3.net", "domain3.org"); siteDomainMapper.AddSite("site4", "https://domain4.com", "https://domain4.net", "https://domain4.org"); - // this works, but it's purely by chance / arbitrary - // don't use the www in tests here! - var current = new Uri("https://www.domain1.com/foo/bar"); + var current = new Uri("https://domain1.com/foo/bar"); Domain[] domains = new[] { new Domain(1, "domain2.com", -1, s_cultureFr, false), From 8a4bf8c0b8ffd4541505dff38372a9cbd34019e0 Mon Sep 17 00:00:00 2001 From: Vitor Rodrigues Date: Fri, 3 Sep 2021 15:34:40 +0200 Subject: [PATCH 2/3] Added missing change removing required filter constraint --- src/Umbraco.Core/Routing/DomainUtilities.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Umbraco.Core/Routing/DomainUtilities.cs b/src/Umbraco.Core/Routing/DomainUtilities.cs index 153eb9d4e8..eb25496a68 100644 --- a/src/Umbraco.Core/Routing/DomainUtilities.cs +++ b/src/Umbraco.Core/Routing/DomainUtilities.cs @@ -199,10 +199,6 @@ namespace Umbraco.Cms.Core.Routing if (filter != null) { var domainAndUri = filter(cultureDomains ?? domainsAndUris, uri, culture, defaultCulture); - // if still nothing, pick the first one? - // no: move that constraint to the filter, but check - if (domainAndUri == null) - throw new InvalidOperationException("The filter returned null."); return domainAndUri; } From 03072ffcd86b6cbc1d5014effbc61e9a0e554dc7 Mon Sep 17 00:00:00 2001 From: Vitor Rodrigues Date: Fri, 3 Sep 2021 15:45:59 +0200 Subject: [PATCH 3/3] Don't use only the current uri to determine the domain when an culture is specified --- src/Umbraco.Core/Routing/DomainUtilities.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Core/Routing/DomainUtilities.cs b/src/Umbraco.Core/Routing/DomainUtilities.cs index eb25496a68..f103307ce7 100644 --- a/src/Umbraco.Core/Routing/DomainUtilities.cs +++ b/src/Umbraco.Core/Routing/DomainUtilities.cs @@ -190,7 +190,7 @@ namespace Umbraco.Cms.Core.Routing } // look for domains that would be the base of the uri - var baseDomains = SelectByBase(considerForBaseDomains, uri); + var baseDomains = SelectByBase(considerForBaseDomains, uri, culture); if (baseDomains.Count > 0) // found, return return baseDomains.First(); @@ -208,12 +208,15 @@ namespace Umbraco.Cms.Core.Routing private static bool IsBaseOf(DomainAndUri domain, Uri uri) => domain.Uri.EndPathWithSlash().IsBaseOf(uri); - private static IReadOnlyCollection SelectByBase(IReadOnlyCollection domainsAndUris, Uri uri) + private static bool MatchesCulture(DomainAndUri domain, string culture) + => culture == null || domain.Culture == culture; + + private static IReadOnlyCollection SelectByBase(IReadOnlyCollection domainsAndUris, Uri uri, string culture) { // look for domains that would be the base of the uri // ie current is www.example.com/foo/bar, look for domain www.example.com var currentWithSlash = uri.EndPathWithSlash(); - var baseDomains = domainsAndUris.Where(d => IsBaseOf(d, currentWithSlash)).ToList(); + var baseDomains = domainsAndUris.Where(d => IsBaseOf(d, currentWithSlash) && MatchesCulture(d, culture)).ToList(); // if none matches, try again without the port // ie current is www.example.com:1234/foo/bar, look for domain www.example.com