diff --git a/src/Umbraco.Core/Configuration/GlobalSettings.cs b/src/Umbraco.Core/Configuration/GlobalSettings.cs index 91ea96096f..6ea260327d 100644 --- a/src/Umbraco.Core/Configuration/GlobalSettings.cs +++ b/src/Umbraco.Core/Configuration/GlobalSettings.cs @@ -58,20 +58,9 @@ namespace Umbraco.Core.Configuration { get { - //first we need to get the parsed base path VirtualPaths in the RouteTable. - //NOTE: for performance we would normally store this result in a variable, however since this class is static - // we cannot gaurantee that all of the routes will be added to the table before this method is called so - // unfortunately, we are stuck resolving these every single time. I don't think the performance will be bad - // but it could be better. Need to fix once we fix up all the configuration classes. - var parser = new RouteParser(RouteTable.Routes); - var reservedFromRouteTable = string.Join(",", parser.ParsedVirtualUrlsFromRouteTable()); - - var config = ConfigurationManager.AppSettings.ContainsKey("umbracoReservedPaths") - ? ConfigurationManager.AppSettings["umbracoReservedPaths"] - : string.Empty; - - //now add the reserved paths - return config.IsNullOrWhiteSpace() ? reservedFromRouteTable : config + "," + reservedFromRouteTable; + return ConfigurationManager.AppSettings.ContainsKey("umbracoReservedPaths") + ? ConfigurationManager.AppSettings["umbracoReservedPaths"] + : string.Empty; } } @@ -561,7 +550,29 @@ namespace Umbraco.Core.Configuration } } - /// + /// + /// Determines whether the current request is reserved based on the route table and + /// whether the specified URL is reserved or is inside a reserved path. + /// + /// + /// + /// The route collection to lookup the request in + /// + public static bool IsReservedPathOrUrl(string url, HttpContextBase httpContext, RouteCollection routes) + { + if (httpContext == null) throw new ArgumentNullException("httpContext"); + if (routes == null) throw new ArgumentNullException("routes"); + + //check if the current request matches a route, if so then it is reserved. + var route = routes.GetRouteData(httpContext); + if (route != null) + return true; + + //continue with the standard ignore routine + return IsReservedPathOrUrl(url); + } + + /// /// Determines whether the specified URL is reserved or is inside a reserved path. /// /// The URL to check. @@ -569,7 +580,7 @@ namespace Umbraco.Core.Configuration /// true if the specified URL is reserved; otherwise, false. /// public static bool IsReservedPathOrUrl(string url) - { + { // check if GlobalSettings.ReservedPaths and GlobalSettings.ReservedUrls are unchanged if (!object.ReferenceEquals(_reservedPathsCache, GlobalSettings.ReservedPaths) || !object.ReferenceEquals(_reservedUrlsCache, GlobalSettings.ReservedUrls)) diff --git a/src/Umbraco.Core/Logging/LogHelper.cs b/src/Umbraco.Core/Logging/LogHelper.cs index 4de0c29171..f0f59f8909 100644 --- a/src/Umbraco.Core/Logging/LogHelper.cs +++ b/src/Umbraco.Core/Logging/LogHelper.cs @@ -220,7 +220,7 @@ namespace Umbraco.Core.Logging { if (trace != null) { - trace.Write(string.Format(generateMessageFormat, formatItems.Select(x => x()))); + trace.Write(string.Format(generateMessageFormat, formatItems.Select(x => x()).ToArray())); } Debug(typeof(T), generateMessageFormat, formatItems); } diff --git a/src/Umbraco.Core/RouteParser.cs b/src/Umbraco.Core/RouteParser.cs deleted file mode 100644 index 78696e66c9..0000000000 --- a/src/Umbraco.Core/RouteParser.cs +++ /dev/null @@ -1,79 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Text.RegularExpressions; -using System.Web.Routing; - -namespace Umbraco.Core -{ - /// - /// A class that is used to parse routes in the route table to determine which route paths to ignore in the GlobalSettings. - /// - /// - /// The parser is not that intelligent, it will not work for very complex URL structures. It will only support simple URL structures that - /// have consecutive tokens. - /// - /// - /// Example routes that will parse: - /// - /// /Test/{controller}/{action}/{id} - /// /Test/MyController/{action}/{id} - /// - /// - internal class RouteParser - { - private readonly RouteCollection _routes; - private readonly Regex _matchAction = new Regex(@"\{action\}", RegexOptions.Compiled); - private readonly Regex _matchController = new Regex(@"\{controller\}", RegexOptions.Compiled); - private readonly Regex _matchId = new Regex(@"\{id\}", RegexOptions.Compiled); - - public RouteParser(RouteCollection routes) - { - _routes = routes; - } - - /// - /// Returned all successfully parsed virtual urls from the route collection - /// - /// - public IEnumerable ParsedVirtualUrlsFromRouteTable() - { - return _routes.OfType() - .Select(TryGetBaseVirtualPath) - .Where(x => x.Success) - .Select(x => x.Result); - } - - internal Attempt TryGetBaseVirtualPath(Route route) - { - var url = route.Url; - var controllers = _matchController.Matches(url); - var actions = _matchAction.Matches(url); - var ids = _matchId.Matches(url); - if (controllers.Count > 1) - return new Attempt(new InvalidOperationException("The URL cannot be parsed, it must contain zero or one {controller} tokens")); - if (actions.Count != 1) - return new Attempt(new InvalidOperationException("The URL cannot be parsed, it must contain one {action} tokens")); - if (ids.Count != 1) - return new Attempt(new InvalidOperationException("The URL cannot be parsed, it must contain one {id} tokens")); - - //now we need to validate that the order - if (controllers.Count == 1) - { - //actions must occur after controllers - if (actions[0].Index < controllers[0].Index) - return new Attempt(new InvalidOperationException("The {action} token must be placed after the {controller} token")); - } - //ids must occur after actions - if (ids[0].Index < actions[0].Index) - return new Attempt(new InvalidOperationException("The {id} token must be placed after the {action} token")); - - //this is all validated, so now we need to return the 'base' virtual path of the route - return new Attempt( - true, - "~/" + url.Substring(0, (controllers.Count == 1 ? controllers[0].Index : actions[0].Index)) - .TrimEnd('/')); - } - } -} diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 36b279ce81..1f7e785ef4 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -128,7 +128,6 @@ - diff --git a/src/Umbraco.Tests/GlobalSettingsTests.cs b/src/Umbraco.Tests/GlobalSettingsTests.cs index d0046fac04..5d57679b61 100644 --- a/src/Umbraco.Tests/GlobalSettingsTests.cs +++ b/src/Umbraco.Tests/GlobalSettingsTests.cs @@ -1,6 +1,8 @@ using System.Configuration; +using System.Web.Routing; using NUnit.Framework; using Umbraco.Tests.TestHelpers; +using System.Web.Mvc; namespace Umbraco.Tests { @@ -53,5 +55,42 @@ namespace Umbraco.Tests { Assert.IsFalse(Umbraco.Core.Configuration.GlobalSettings.IsReservedPathOrUrl(url)); } + + + [TestCase("/Do/Not/match", false)] + [TestCase("/Umbraco/RenderMvcs", false)] + [TestCase("/Umbraco/RenderMvc", true)] + [TestCase("/Umbraco/RenderMvc/Index", true)] + [TestCase("/Umbraco/RenderMvc/Index/1234", true)] + [TestCase("/Umbraco/RenderMvc/Index/1234/9876", false)] + [TestCase("/api", true)] + [TestCase("/api/WebApiTest", true)] + [TestCase("/api/WebApiTest/1234", true)] + [TestCase("/api/WebApiTest/Index/1234", false)] + public void Is_Reserved_By_Route(string url, bool shouldMatch) + { + //reset the app config, we only want to test routes not the hard coded paths + ConfigurationManager.AppSettings.Set("umbracoReservedPaths", ""); + ConfigurationManager.AppSettings.Set("umbracoReservedUrls", ""); + + var routes = new RouteCollection(); + + routes.MapRoute( + "Umbraco_default", + "Umbraco/RenderMvc/{action}/{id}", + new { controller = "RenderMvc", action = "Index", id = UrlParameter.Optional }); + routes.MapRoute( + "WebAPI", + "api/{controller}/{id}", + new { controller = "WebApiTestController", action = "Index", id = UrlParameter.Optional }); + + + var context = new FakeHttpContextFactory(url); + + + Assert.AreEqual( + shouldMatch, + Umbraco.Core.Configuration.GlobalSettings.IsReservedPathOrUrl(url, context.HttpContext, routes)); + } } } \ No newline at end of file diff --git a/src/Umbraco.Tests/RouteParserTests.cs b/src/Umbraco.Tests/RouteParserTests.cs deleted file mode 100644 index ba161009a2..0000000000 --- a/src/Umbraco.Tests/RouteParserTests.cs +++ /dev/null @@ -1,96 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Web; -using System.Web.Routing; -using NUnit.Framework; -using Umbraco.Core; - -namespace Umbraco.Tests -{ - [TestFixture] - public class RouteParserTests - { - /// - /// used for testing - /// - private class MyRoute : RouteBase - { - private readonly string _url; - - public MyRoute(string url) - { - _url = url; - } - - public override RouteData GetRouteData(HttpContextBase httpContext) - { - throw new NotImplementedException(); - } - - public override VirtualPathData GetVirtualPath(RequestContext requestContext, RouteValueDictionary values) - { - throw new NotImplementedException(); - } - } - - [Test] - public void Get_Successfully_Parsed_Urls() - { - var urls = new RouteBase[] - { - new Route("Umbraco/RenderMvc/{action}/{id}", null), - new Route("Another/Test/{controller}/{action}/{id}", null), - new MyRoute("Umbraco/RenderMvc/{action}/{id}"), //wont match because its not a route object - new Route("Another/Test/{id}/{controller}/{action}", null) //invalid - }; - var valid = new string[] - { - "~/Umbraco/RenderMvc", - "~/Another/Test" - }; - - var collection = new RouteCollection(); - foreach (var u in urls) - { - collection.Add(u); - } - var parser = new RouteParser(collection); - - var result = parser.ParsedVirtualUrlsFromRouteTable(); - - Assert.AreEqual(2, result.Count()); - Assert.IsTrue(result.ContainsAll(valid)); - } - - [TestCase("Umbraco/RenderMvc/{action}/{id}", "~/Umbraco/RenderMvc")] - [TestCase("Install/PackageInstaller/{action}/{id}", "~/Install/PackageInstaller")] - [TestCase("Test/{controller}/{action}/{id}", "~/Test")] - [TestCase("Another/Test/{controller}/{action}/{id}", "~/Another/Test")] - public void Successful_Virtual_Path(string url, string result) - { - var route = new Route(url, null); - var parser = new RouteParser(new RouteCollection()); - var attempt = parser.TryGetBaseVirtualPath(route); - if (!attempt.Success && attempt.Error != null) - throw attempt.Error; //throw this so we can see the error in the test output - - Assert.IsTrue(attempt.Success); - Assert.AreEqual(result, attempt.Result); - } - - [TestCase("Umbraco/RenderMvc/{action}/{controller}/{id}")] - [TestCase("Install/PackageInstaller/{id}/{action}")] - [TestCase("Test/{controller}/{id}/{action}")] - [TestCase("Another/Test/{id}/{controller}/{action}")] - public void Non_Parsable_Virtual_Path(string url) - { - var route = new Route(url, null); - var parser = new RouteParser(new RouteCollection()); - var attempt = parser.TryGetBaseVirtualPath(route); - Assert.IsFalse(attempt.Success); - } - - } -} diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 6b0607a1f0..ae497719f9 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -86,7 +86,6 @@ - diff --git a/src/Umbraco.Web/UmbracoModule.cs b/src/Umbraco.Web/UmbracoModule.cs index bc021fc761..29457c2ce7 100644 --- a/src/Umbraco.Web/UmbracoModule.cs +++ b/src/Umbraco.Web/UmbracoModule.cs @@ -213,7 +213,7 @@ namespace Umbraco.Web // at that point, either we have no extension, or it is .aspx // if the path is reserved then it cannot be a document request - if (maybeDoc && GlobalSettings.IsReservedPathOrUrl(lpath)) + if (maybeDoc && GlobalSettings.IsReservedPathOrUrl(lpath, httpContext, RouteTable.Routes)) maybeDoc = false; //NOTE: No need to warn, plus if we do we should log the document, as this message doesn't really tell us anything :)