U4-1780 - detect colliding urls (#1243)

* U4-1780 - detect colliding urls

* U4-1780 - fix error message

* U4-1780 - fix XmlHelper issue with sortOrder

* U4-1780 - bugfix
This commit is contained in:
Stephan
2016-05-04 12:45:20 +02:00
committed by Sebastiaan Janssen
parent 729b179473
commit 5f8747b4d3
7 changed files with 93 additions and 16 deletions

View File

@@ -210,6 +210,9 @@ namespace Umbraco.Core
var childNodesAndOrder = parentNode.SelectNodes(childNodesXPath).Cast<XmlNode>()
.Select(x => Tuple.Create(x, orderBy(x))).ToArray();
// only one node = node is in the right place already, obviously
if (childNodesAndOrder.Length == 1) return false;
// find the first node with a sortOrder > node.sortOrder
var i = 0;
while (i < childNodesAndOrder.Length && childNodesAndOrder[i].Item2 <= nodeSortOrder)
@@ -220,17 +223,18 @@ namespace Umbraco.Core
{
// and node is just before, we're done already
// else we need to move it right before the node that was found
if (i > 0 && childNodesAndOrder[i - 1].Item1 != node)
if (i == 0 || childNodesAndOrder[i - 1].Item1 != node)
{
parentNode.InsertBefore(node, childNodesAndOrder[i].Item1);
return true;
}
}
else
else // i == childNodesAndOrder.Length && childNodesAndOrder.Length > 1
{
// and node is the last one, we're done already
// else we need to append it as the last one
if (i > 0 && childNodesAndOrder[i - 1].Item1 != node)
// (and i > 1, see above)
if (childNodesAndOrder[i - 1].Item1 != node)
{
parentNode.AppendChild(node);
return true;

View File

@@ -147,6 +147,7 @@
<key alias="otherElements">Properties</key>
<key alias="parentNotPublished">This document is published but is not visible because the parent '%0%' is unpublished</key>
<key alias="parentNotPublishedAnomaly">Oops: this document is published but is not in the cache (internal error)</key>
<key alias="routeError">Oops: this document is published but its url would collide with content %0%</key>
<key alias="publish">Publish</key>
<key alias="publishStatus">Publication Status</key>
<key alias="releaseDate">Publish at</key>

View File

@@ -148,6 +148,7 @@
<key alias="otherElements">Properties</key>
<key alias="parentNotPublished">This document is published but is not visible because the parent '%0%' is unpublished</key>
<key alias="parentNotPublishedAnomaly">Oops: this document is published but is not in the cache (internal error)</key>
<key alias="routeError">Oops: this document is published but its url would collide with content %0%</key>
<key alias="publish">Publish</key>
<key alias="publishStatus">Publication Status</key>
<key alias="releaseDate">Publish at</key>
@@ -676,7 +677,7 @@ To manage your website, simply open the Umbraco back office and start adding con
<key alias="resetPasswordEmailCopySubject">Umbraco: Reset Password</key>
<key alias="resetPasswordEmailCopyFormat">
<![CDATA[<p>Your username to login to the Umbraco back-office is: <strong>%0%</strong></p><p>Click <a href="%1%"><strong>here</strong></a> to reset your password or copy/paste this URL into your browser:</p><p><em>%1%</em></p>]]>
</key>
</key>
</area>
<area alias="main">
<key alias="dashboard">Dashboard</key>

View File

@@ -48,6 +48,8 @@ namespace Umbraco.Web.Models
? null // for tests only
: umbracoContext.ContentCache.GetRouteById(contentId); // cached
if (route != null && route.StartsWith("err/")) route = null;
var domainHelper = new DomainHelper(domainService);
IDomain domain;

View File

@@ -55,19 +55,28 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache
// cache if we have a content and not previewing
if (content != null && preview == false)
{
var domainRootNodeId = route.StartsWith("/") ? -1 : int.Parse(route.Substring(0, route.IndexOf('/')));
var iscanon =
UnitTesting == false
&& DomainHelper.ExistsDomainInPath(umbracoContext.Application.Services.DomainService.GetAll(false), content.Path, domainRootNodeId) == false;
// and only if this is the canonical url (the one GetUrl would return)
if (iscanon)
_routesCache.Store(content.Id, route);
}
AddToCacheIfDeepestRoute(umbracoContext, content, route);
return content;
}
private void AddToCacheIfDeepestRoute(UmbracoContext umbracoContext, IPublishedContent content, string route)
{
var domainRootNodeId = route.StartsWith("/") ? -1 : int.Parse(route.Substring(0, route.IndexOf('/')));
// so we have a route that maps to a content... say "1234/path/to/content" - however, there could be a
// domain set on "to" and route "4567/content" would also map to the same content - and due to how
// urls computing work (by walking the tree up to the first domain we find) it is that second route
// that would be returned - the "deepest" route - and that is the route we want to cache, *not* the
// longer one - so make sure we don't cache the wrong route
var deepest = UnitTesting == false
&& DomainHelper.ExistsDomainInPath(umbracoContext.Application.Services.DomainService.GetAll(false), content.Path, domainRootNodeId) == false;
if (deepest)
_routesCache.Store(content.Id, route);
}
public virtual string GetRouteById(UmbracoContext umbracoContext, bool preview, int contentId)
{
// try to get from cache if not previewing
@@ -80,11 +89,35 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache
// else actually determine the route
route = DetermineRouteById(umbracoContext, preview, contentId);
// cache if we have a route and not previewing
if (route != null && preview == false)
// node not found
if (route == null)
return null;
// find the content back, detect routes collisions: we should find ourselves back,
// else it means that another content with "higher priority" is sharing the same route.
// perf impact:
// - non-colliding, adds one complete "by route" lookup, only on the first time a url is computed (then it's cached anyways)
// - colliding, adds one "by route" lookup, the first time the url is computed, then one dictionary looked each time it is computed again
// assuming no collisions, the impact is one complete "by route" lookup the first time each url is computed
var loopId = preview ? 0 : _routesCache.GetNodeId(route); // might be cached already in case of collision
if (loopId == 0)
{
var content = DetermineIdByRoute(umbracoContext, preview, route, GlobalSettings.HideTopLevelNodeFromPath);
// add the other route to cache so next time we have it already
if (content != null && preview == false)
AddToCacheIfDeepestRoute(umbracoContext, content, route);
loopId = content == null ? 0 : content.Id; // though... 0 here would be quite weird?
}
// cache if we have a route and not previewing and it's not a colliding route
// (the result of DetermineRouteById is always the deepest route)
if (/*route != null &&*/ preview == false && loopId == contentId)
_routesCache.Store(contentId, route);
return route;
// return route if no collision, else report collision
return loopId == contentId ? route : ("err/" + loopId);
}
IPublishedContent DetermineIdByRoute(UmbracoContext umbracoContext, bool preview, string route, bool hideTopLevelNode)

View File

@@ -52,6 +52,7 @@ namespace Umbraco.Web.Routing
// will not use cache if previewing
var route = umbracoContext.ContentCache.GetRouteById(id);
if (string.IsNullOrWhiteSpace(route))
{
LogHelper.Debug<DefaultUrlProvider>(
@@ -60,6 +61,14 @@ namespace Umbraco.Web.Routing
return null;
}
if (route.StartsWith("err/"))
{
LogHelper.Debug<DefaultUrlProvider>(
"Page with nodeId={0} has a colliding url with page with nodeId={1}.",
() => id, () => route.Substring(4));
return "#err-" + route.Substring(4);
}
var domainHelper = new DomainHelper(umbracoContext.Application.Services.DomainService);
// extract domainUri and path
@@ -102,6 +111,9 @@ namespace Umbraco.Web.Routing
return null;
}
if (route.StartsWith("err/"))
return null;
var domainHelper = new DomainHelper(umbracoContext.Application.Services.DomainService);
// extract domainUri and path

View File

@@ -48,6 +48,30 @@ namespace Umbraco.Web.Routing
else
urls.Add(ui.Text("content", "parentNotPublished", parent.Name, umbracoContext.Security.CurrentUser));
}
else if (url.StartsWith("#err-"))
{
// route error, report
var id = int.Parse(url.Substring(5));
var o = umbracoContext.ContentCache.GetById(id);
string s;
if (o == null)
{
s = "(unknown)";
}
else
{
var l = new List<string>();
while (o != null)
{
l.Add(o.Name);
o = o.Parent;
}
l.Reverse();
s = "/" + string.Join("/", l) + " (id=" + id + ")";
}
urls.Add(ui.Text("content", "routeError", s, umbracoContext.Security.CurrentUser));
}
else
{
urls.Add(url);