From 7d64990a696780c5e08225726352880aed858d0e Mon Sep 17 00:00:00 2001 From: Pranav K Date: Tue, 2 Jan 2018 15:06:24 -0800 Subject: [PATCH] Ensure RazorPages in an area are not route-able through root based paths when root directory and area root directory overlap Fixes #7147 --- .../CompiledPageRouteModelProvider.cs | 12 ++-- .../Internal/PageSelectorModel.cs | 45 ++++++++----- .../RazorProjectPageRouteModelProvider.cs | 57 ++++++++++------- .../CompiledPageRouteModelProviderTest.cs | 57 +++++++++++++++++ .../RazorProjectPageRouteModelProviderTest.cs | 63 +++++++++++++++++++ 5 files changed, 191 insertions(+), 43 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/CompiledPageRouteModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/CompiledPageRouteModelProvider.cs index e4da857964..4a5f9aa72f 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/CompiledPageRouteModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/CompiledPageRouteModelProvider.cs @@ -73,14 +73,16 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal foreach (var viewDescriptor in GetViewDescriptors(_applicationManager)) { PageRouteModel model = null; - if (viewDescriptor.RelativePath.StartsWith(rootDirectory, StringComparison.OrdinalIgnoreCase)) - { - model = GetPageRouteModel(rootDirectory, viewDescriptor); - } - else if (_pagesOptions.AllowAreas && viewDescriptor.RelativePath.StartsWith(areaRootDirectory, StringComparison.OrdinalIgnoreCase)) + // When RootDirectory and AreaRootDirectory overlap (e.g. RootDirectory = '/', AreaRootDirectory = '/Areas'), we + // only want to allow a page to be associated with the area route. + if (_pagesOptions.AllowAreas && viewDescriptor.RelativePath.StartsWith(areaRootDirectory, StringComparison.OrdinalIgnoreCase)) { model = GetAreaPageRouteModel(areaRootDirectory, viewDescriptor); } + else if (viewDescriptor.RelativePath.StartsWith(rootDirectory, StringComparison.OrdinalIgnoreCase)) + { + model = GetPageRouteModel(rootDirectory, viewDescriptor); + } if (model != null) { diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageSelectorModel.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageSelectorModel.cs index 70c85bb035..c0ee83d144 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageSelectorModel.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageSelectorModel.cs @@ -56,6 +56,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal result = default; Debug.Assert(path.StartsWith("/", StringComparison.Ordinal)); + // 1. Parse the area name. This will be the first token we encounter. var areaEndIndex = path.IndexOf('/', startIndex: 1); if (areaEndIndex == -1 || areaEndIndex == path.Length) { @@ -63,25 +64,37 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal return false; } - // Normalize the pages root directory so that it has a - var normalizedPagesRootDirectory = razorPagesOptions.RootDirectory.TrimStart('/'); - if (!normalizedPagesRootDirectory.EndsWith("/", StringComparison.Ordinal)) - { - normalizedPagesRootDirectory += "/"; - } - - if (string.Compare(path, areaEndIndex + 1, normalizedPagesRootDirectory, 0, normalizedPagesRootDirectory.Length, StringComparison.OrdinalIgnoreCase) != 0) - { - logger.UnsupportedAreaPath(razorPagesOptions, path); - return false; - } - var areaName = path.Substring(1, areaEndIndex - 1); - var pagePathIndex = areaEndIndex + normalizedPagesRootDirectory.Length; - Debug.Assert(path.EndsWith(RazorViewEngine.ViewExtension), $"{path} does not end in extension '{RazorViewEngine.ViewExtension}'."); + string pageName; + if (razorPagesOptions.RootDirectory == "/") + { + // When RootDirectory is "/", every thing past the area name is the page path. + Debug.Assert(path.EndsWith(RazorViewEngine.ViewExtension), $"{path} does not end in extension '{RazorViewEngine.ViewExtension}'."); + pageName = path.Substring(areaEndIndex, path.Length - areaEndIndex - RazorViewEngine.ViewExtension.Length); + } + else + { + // Normalize the pages root directory so that it has a trailing slash. This ensures we're looking at a directory delimiter + // and not just the area name occuring as part of a segment. + Debug.Assert(razorPagesOptions.RootDirectory.StartsWith("/", StringComparison.Ordinal)); + var normalizedPagesRootDirectory = razorPagesOptions.RootDirectory.Substring(1); + if (!normalizedPagesRootDirectory.EndsWith("/", StringComparison.Ordinal)) + { + normalizedPagesRootDirectory += "/"; + } - var pageName = path.Substring(pagePathIndex, path.Length - pagePathIndex - RazorViewEngine.ViewExtension.Length); + Debug.Assert(normalizedPagesRootDirectory.Length > 0); + // If the pages root has a value i.e. it's not the app root "/", ensure that the area path contains this value. + if (string.Compare(path, areaEndIndex + 1, normalizedPagesRootDirectory, 0, normalizedPagesRootDirectory.Length, StringComparison.OrdinalIgnoreCase) != 0) + { + logger.UnsupportedAreaPath(razorPagesOptions, path); + return false; + } + + var pageNameIndex = areaEndIndex + normalizedPagesRootDirectory.Length; + pageName = path.Substring(pageNameIndex, path.Length - pageNameIndex - RazorViewEngine.ViewExtension.Length); + } var builder = new InplaceStringBuilder(areaEndIndex + pageName.Length); builder.Append(path, 0, areaEndIndex); diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/RazorProjectPageRouteModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/RazorProjectPageRouteModelProvider.cs index a2f0c8cf2d..d57f471aa8 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/RazorProjectPageRouteModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/RazorProjectPageRouteModelProvider.cs @@ -37,16 +37,25 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal public void OnProvidersExecuting(PageRouteModelProviderContext context) { - AddPageModels(context); - + // When RootDirectory and AreaRootDirectory overlap, e.g. RootDirectory = /, AreaRootDirectoryy = /Areas; + // we need to ensure that the page is only route-able via the area route. By adding area routes first, + // we'll ensure non area routes get skipped when it encounters an IsAlreadyRegistered check. if (_pagesOptions.AllowAreas) { AddAreaPageModels(context); } + + AddPageModels(context); } private void AddPageModels(PageRouteModelProviderContext context) { + var normalizedAreaRootDirectory = _pagesOptions.AreaRootDirectory; + if (!normalizedAreaRootDirectory.EndsWith("/", StringComparison.Ordinal)) + { + normalizedAreaRootDirectory += "/"; + } + foreach (var item in _project.EnumerateItems(_pagesOptions.RootDirectory)) { if (!IsRouteable(item)) @@ -54,23 +63,29 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal continue; } + var relativePath = item.CombinedPath; + if (IsAlreadyRegistered(context, relativePath)) + { + // A route for this file was already registered either by the CompiledPageRouteModel or as an area route. + // by this provider. Skip registering an additional entry. + continue; + } + if (!PageDirectiveFeature.TryGetPageDirective(_logger, item, out var routeTemplate)) { // .cshtml pages without @page are not RazorPages. continue; } - var routeModel = new PageRouteModel( - relativePath: item.CombinedPath, - viewEnginePath: item.FilePathWithoutExtension); - - if (IsAlreadyRegistered(context, routeModel)) + if (_pagesOptions.AllowAreas && relativePath.StartsWith(normalizedAreaRootDirectory, StringComparison.OrdinalIgnoreCase)) { - // The CompiledPageRouteModelProvider (or another provider) already registered a PageRoute for this path. - // Don't register a duplicate entry for this route. + // Ignore Razor pages that are under the area root directory when AllowAreas is enabled. + // Conforming page paths will be added by AddAreaPageModels. + _logger.UnsupportedAreaPath(_pagesOptions, relativePath); continue; } + var routeModel = new PageRouteModel(relativePath, viewEnginePath: item.FilePathWithoutExtension); PageSelectorModel.PopulateDefaults(routeModel, routeModel.ViewEnginePath, routeTemplate); context.RouteModels.Add(routeModel); } @@ -85,6 +100,14 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal continue; } + var relativePath = item.CombinedPath; + if (IsAlreadyRegistered(context, relativePath)) + { + // A route for this file was already registered either by the CompiledPageRouteModel. + // Skip registering an additional entry. + continue; + } + if (!PageDirectiveFeature.TryGetPageDirective(_logger, item, out var routeTemplate)) { // .cshtml pages without @page are not RazorPages. @@ -96,9 +119,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal continue; } - var routeModel = new PageRouteModel( - relativePath: item.CombinedPath, - viewEnginePath: areaResult.viewEnginePath) + var routeModel = new PageRouteModel(relativePath, viewEnginePath: areaResult.viewEnginePath) { RouteValues = { @@ -106,25 +127,17 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal }, }; - if (IsAlreadyRegistered(context, routeModel)) - { - // The CompiledPageRouteModelProvider (or another provider) already registered a PageRoute for this path. - // Don't register a duplicate entry for this route. - continue; - } - PageSelectorModel.PopulateDefaults(routeModel, areaResult.pageRoute, routeTemplate); context.RouteModels.Add(routeModel); } } - private bool IsAlreadyRegistered(PageRouteModelProviderContext context, PageRouteModel routeModel) + private bool IsAlreadyRegistered(PageRouteModelProviderContext context, string relativePath) { for (var i = 0; i < context.RouteModels.Count; i++) { var existingRouteModel = context.RouteModels[i]; - if (string.Equals(existingRouteModel.ViewEnginePath, routeModel.ViewEnginePath, StringComparison.OrdinalIgnoreCase) && - string.Equals(existingRouteModel.RelativePath, existingRouteModel.RelativePath, StringComparison.OrdinalIgnoreCase)) + if (string.Equals(relativePath, existingRouteModel.RelativePath, StringComparison.OrdinalIgnoreCase)) { return true; } diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/CompiledPageRouteModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/CompiledPageRouteModelProviderTest.cs index be8e08e304..ca8aca079e 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/CompiledPageRouteModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/CompiledPageRouteModelProviderTest.cs @@ -175,6 +175,63 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal }); } + [Fact] + public void OnProvidersExecuting_DoesNotAddAreaAndNonAreaRoutesForAPage() + { + // Arrange + var descriptors = new[] + { + GetDescriptor("/Areas/Accounts/Manage/Home.cshtml"), + GetDescriptor("/Areas/About.cshtml"), + GetDescriptor("/Contact.cshtml"), + }; + var options = new RazorPagesOptions + { + AllowAreas = true, + AreaRootDirectory = "/Areas", + RootDirectory = "/", + }; + var provider = new TestCompiledPageRouteModelProvider(descriptors, options); + var context = new PageRouteModelProviderContext(); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + Assert.Collection(context.RouteModels, + result => + { + Assert.Equal("/Areas/Accounts/Manage/Home.cshtml", result.RelativePath); + Assert.Equal("/Manage/Home", result.ViewEnginePath); + Assert.Collection(result.Selectors, + selector => Assert.Equal("Accounts/Manage/Home", selector.AttributeRouteModel.Template)); + Assert.Collection(result.RouteValues.OrderBy(k => k.Key), + kvp => + { + Assert.Equal("area", kvp.Key); + Assert.Equal("Accounts", kvp.Value); + }, + kvp => + { + Assert.Equal("page", kvp.Key); + Assert.Equal("/Manage/Home", kvp.Value); + }); + }, + result => + { + Assert.Equal("/Contact.cshtml", result.RelativePath); + Assert.Equal("/Contact", result.ViewEnginePath); + Assert.Collection(result.Selectors, + selector => Assert.Equal("Contact", selector.AttributeRouteModel.Template)); + Assert.Collection(result.RouteValues.OrderBy(k => k.Key), + kvp => + { + Assert.Equal("page", kvp.Key); + Assert.Equal("/Contact", kvp.Value); + }); + }); + } + [Fact] public void OnProvidersExecuting_AddsMultipleSelectorsForIndexPage_WithIndexAtRoot() { diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/RazorProjectPageRouteModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/RazorProjectPageRouteModelProviderTest.cs index 11bea486d7..eefef307f6 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/RazorProjectPageRouteModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/RazorProjectPageRouteModelProviderTest.cs @@ -171,6 +171,69 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal }); } + [Fact] + public void OnProvidersExecuting_DoesNotAddAreaAndNonAreaRoutesForAPage() + { + // Arrange + var fileProvider = new TestFileProvider(); + var conformingFileUnderAreasDirectory = fileProvider.AddFile("Categories.cshtml", "@page"); + // We shouldn't add a route for this. + var nonConformingFileUnderAreasDirectory = fileProvider.AddFile("Home.cshtml", "@page"); + var rootFile = fileProvider.AddFile("About.cshtml", "@page"); + + var productsDir = fileProvider.AddDirectoryContent("/Areas/Products", new[] { conformingFileUnderAreasDirectory }); + var areasDir = fileProvider.AddDirectoryContent("/Areas", new IFileInfo[] { productsDir, nonConformingFileUnderAreasDirectory }); + var rootDir = fileProvider.AddDirectoryContent("/", new IFileInfo[] { areasDir, rootFile }); + + var project = new TestRazorProject(fileProvider); + + var optionsManager = Options.Create(new RazorPagesOptions + { + RootDirectory = "/", + AreaRootDirectory = "/Areas", + AllowAreas = true, + }); + var provider = new RazorProjectPageRouteModelProvider(project, optionsManager, NullLoggerFactory.Instance); + var context = new PageRouteModelProviderContext(); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + Assert.Collection(context.RouteModels, + model => + { + Assert.Equal("/Areas/Products/Categories.cshtml", model.RelativePath); + Assert.Equal("/Categories", model.ViewEnginePath); + Assert.Collection(model.Selectors, + selector => Assert.Equal("Products/Categories", selector.AttributeRouteModel.Template)); + Assert.Collection(model.RouteValues.OrderBy(k => k.Key), + kvp => + { + Assert.Equal("area", kvp.Key); + Assert.Equal("Products", kvp.Value); + }, + kvp => + { + Assert.Equal("page", kvp.Key); + Assert.Equal("/Categories", kvp.Value); + }); + }, + model => + { + Assert.Equal("/About.cshtml", model.RelativePath); + Assert.Equal("/About", model.ViewEnginePath); + Assert.Collection(model.Selectors, + selector => Assert.Equal("About", selector.AttributeRouteModel.Template)); + Assert.Collection(model.RouteValues.OrderBy(k => k.Key), + kvp => + { + Assert.Equal("page", kvp.Key); + Assert.Equal("/About", kvp.Value); + }); + }); + } + [Fact] public void OnProvidersExecuting_AddsMultipleSelectorsForIndexPages() {