diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ViewEnginePath.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ViewEnginePath.cs index a77cd5882a..056a49d4eb 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ViewEnginePath.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ViewEnginePath.cs @@ -11,9 +11,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal { public static class ViewEnginePath { + public static readonly char[] PathSeparators = new[] { '/', '\\' }; private const string CurrentDirectoryToken = "."; private const string ParentDirectoryToken = ".."; - private static readonly char[] _pathSeparators = new[] { '/', '\\' }; public static string CombinePath(string first, string second) { @@ -47,13 +47,36 @@ public static string CombinePath(string first, string second) public static string ResolvePath(string path) { - if (!RequiresPathResolution(path)) + Debug.Assert(!string.IsNullOrEmpty(path)); + var pathSegment = new StringSegment(path); + if (path[0] == PathSeparators[0] || path[0] == PathSeparators[1]) + { + // Leading slashes (e.g. "/Views/Index.cshtml") always generate an empty first token. Ignore these + // for purposes of resolution. + pathSegment = pathSegment.Subsegment(1); + } + + var tokenizer = new StringTokenizer(pathSegment, PathSeparators); + var requiresResolution = false; + foreach (var segment in tokenizer) + { + // Determine if we need to do any path resolution. + // We need to resovle paths with multiple path separators (e.g "//" or "\\") or, directory traversals e.g. ("../" or "./"). + if (segment.Length == 0 || + segment.Equals(ParentDirectoryToken, StringComparison.Ordinal) || + segment.Equals(CurrentDirectoryToken, StringComparison.Ordinal)) + { + requiresResolution = true; + break; + } + } + + if (!requiresResolution) { return path; } var pathSegments = new List(); - var tokenizer = new StringTokenizer(path, _pathSeparators); foreach (var segment in tokenizer) { if (segment.Length == 0) @@ -92,11 +115,5 @@ public static string ResolvePath(string path) return builder.ToString(); } - - private static bool RequiresPathResolution(string path) - { - return path.IndexOf(ParentDirectoryToken, StringComparison.Ordinal) != -1 || - path.IndexOf(CurrentDirectoryToken, StringComparison.Ordinal) != -1; - } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs b/src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs index ad0ec5e1cf..d51099471d 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs +++ b/src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs @@ -205,8 +205,7 @@ private ViewLocationCacheResult LocatePageFromPath(string executingFilePath, str { var applicationRelativePath = GetAbsolutePath(executingFilePath, pagePath); var cacheKey = new ViewLocationCacheKey(applicationRelativePath, isMainPage); - ViewLocationCacheResult cacheResult; - if (!ViewLookupCache.TryGetValue(cacheKey, out cacheResult)) + if (!ViewLookupCache.TryGetValue(cacheKey, out ViewLocationCacheResult cacheResult)) { var expirationTokens = new HashSet(); cacheResult = CreateCacheResult(expirationTokens, applicationRelativePath, isMainPage); @@ -369,6 +368,8 @@ private ViewLocationCacheResult OnCacheMiss( expanderContext.ControllerName, expanderContext.AreaName); + path = ViewEnginePath.ResolvePath(path); + cacheResult = CreateCacheResult(expirationTokens, path, expanderContext.IsMainPage); if (cacheResult != null) { diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ViewEnginePathTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ViewEnginePathTest.cs new file mode 100644 index 0000000000..e76e274b82 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ViewEnginePathTest.cs @@ -0,0 +1,52 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Internal +{ + public class ViewEnginePathTest + { + [Theory] + [InlineData("Views/../Home/Index.cshtml", "/Home/Index.cshtml")] + [InlineData("/Views/Home/../Shared/Partial.cshtml", "/Views/Shared/Partial.cshtml")] + [InlineData("/Views/Shared/./Partial.cshtml", "/Views/Shared/Partial.cshtml")] + [InlineData("//Views/Index.cshtml", "/Views/Index.cshtml")] + public void ResolvePath_ResolvesPathTraversals(string input, string expected) + { + // Arrange & Act + var result = ViewEnginePath.ResolvePath(input); + + // Assert + Assert.Equal(expected, result); + } + + [Theory] + [InlineData("../Index.cshtml")] + [InlineData("Views/../../Index.cshtml")] + [InlineData("Views/../Shared/../../Index.cshtml")] + public void ResolvePath_DoesNotTraversePastApplicationRoot(string input) + { + // Arrange + var result = ViewEnginePath.ResolvePath(input); + + // Assert + Assert.Same(input, result); + } + + [Theory] + [InlineData("/Views/Index.cshtml")] + [InlineData(@"/Views\Index.cshtml")] + [InlineData("Index..cshtml")] + [InlineData("/directory.with.periods/sub-dir/index.cshtml")] + [InlineData("file.with.periods.cshtml")] + public void ResolvePath_DoesNotModifyPathsWithoutTraversals(string input) + { + // Arrange & Act + var result = ViewEnginePath.ResolvePath(input); + + // Assert + Assert.Same(input, result); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ViewEngineTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ViewEngineTests.cs index d8c8ac6328..8b901b2977 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ViewEngineTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ViewEngineTests.cs @@ -247,7 +247,14 @@ public async Task RazorViewEngine_RendersPartialViews(string actionName, string } [Fact] - public async Task RazorViewEngine_RendersViewsFromEmbeddedFileProvider() + public Task RazorViewEngine_RendersViewsFromEmbeddedFileProvider_WhenLookedupByName() + => RazorViewEngine_RendersIndexViewsFromEmbeddedFileProvider("/EmbeddedViews/LookupByName"); + + [Fact] + public Task RazorViewEngine_RendersViewsFromEmbeddedFileProvider_WhenLookedupByPath() + => RazorViewEngine_RendersIndexViewsFromEmbeddedFileProvider("/EmbeddedViews/LookupByPath"); + + private async Task RazorViewEngine_RendersIndexViewsFromEmbeddedFileProvider(string requestPath) { // Arrange var expected = @@ -257,7 +264,7 @@ Hello from Shared/_EmbeddedPartial "; // Act - var body = await Client.GetStringAsync("/EmbeddedViews"); + var body = await Client.GetStringAsync(requestPath); // Assert Assert.Equal(expected, body.Trim(), ignoreLineEndingDifferences: true); @@ -493,5 +500,18 @@ public async Task ViewEngine_NormalizesPathsReturnedByViewLocationExpanders() // Assert Assert.Equal(expected, responseContent.Trim()); } + + [Fact] + public async Task ViewEngine_ResolvesPathsWithSlashesThatDoNotHaveExtensions() + { + // Arrange + var expected = @"Hello from EmbeddedHome\EmbeddedPartial"; + + // Act + var responseContent = await Client.GetStringAsync("/EmbeddedViews/RelativeNonPath"); + + // Assert + Assert.Equal(expected, responseContent.Trim()); + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs b/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs index 3c8d4350ec..11aedb7f04 100644 --- a/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs @@ -24,7 +24,7 @@ using Moq; using Xunit; -namespace Microsoft.AspNetCore.Mvc.Razor.Test +namespace Microsoft.AspNetCore.Mvc.Razor { public class RazorViewEngineTest { @@ -1864,6 +1864,82 @@ public void ViewEngine_SetsPageValue_IfItIsSpecifiedInRouteValues() expander.Verify(); } + [Fact] + public void FindView_ResolvesDirectoryTraversalsPriorToInvokingPageFactory() + { + // Arrange + var pageFactory = new Mock(); + pageFactory.Setup(p => p.CreateFactory("/Views/Shared/_Partial.cshtml")) + .Returns(GetPageFactoryResult(() => Mock.Of())) + .Verifiable(); + var viewEngine = CreateViewEngine(pageFactory.Object); + var context = GetActionContext(new Dictionary()); + + // Act + var result = viewEngine.FindView(context, "../Shared/_Partial", isMainPage: false); + + // Assert + pageFactory.Verify(); + } + + [Fact] + public void FindPage_ResolvesDirectoryTraversalsPriorToInvokingPageFactory() + { + // Arrange + var pageFactory = new Mock(); + pageFactory.Setup(p => p.CreateFactory("/Views/Shared/_Partial.cshtml")) + .Returns(GetPageFactoryResult(() => Mock.Of())) + .Verifiable(); + var viewEngine = CreateViewEngine(pageFactory.Object); + var context = GetActionContext(new Dictionary()); + + // Act + var result = viewEngine.FindPage(context, "../Shared/_Partial"); + + // Assert + pageFactory.Verify(); + } + + // Tests to verify fix for https://github.com/aspnet/Mvc/issues/6672 + // Without normalizing the path, the view engine would have attempted to lookup "/Views//MyView.cshtml" + // which works for PhysicalFileProvider but fails for exact lookups performed during precompilation. + // We normalize it to "/Views/MyView.cshtml" to avoid this discrepancy. + [Fact] + public void FindView_ResolvesNormalizesSlashesPriorToInvokingPageFactory() + { + // Arrange + var pageFactory = new Mock(); + pageFactory.Setup(p => p.CreateFactory("/Views/MyView.cshtml")) + .Returns(GetPageFactoryResult(() => Mock.Of())) + .Verifiable(); + var viewEngine = CreateViewEngine(pageFactory.Object); + var context = GetActionContext(new Dictionary()); + + // Act + var result = viewEngine.FindView(context, "MyView", isMainPage: true); + + // Assert + pageFactory.Verify(); + } + + [Fact] + public void FindPage_ResolvesNormalizesSlashesPriorToInvokingPageFactory() + { + // Arrange + var pageFactory = new Mock(); + pageFactory.Setup(p => p.CreateFactory("/Views/MyPage.cshtml")) + .Returns(GetPageFactoryResult(() => Mock.Of())) + .Verifiable(); + var viewEngine = CreateViewEngine(pageFactory.Object); + var context = GetActionContext(new Dictionary()); + + // Act + var result = viewEngine.FindPage(context, "MyPage"); + + // Assert + pageFactory.Verify(); + } + // Return RazorViewEngine with a page factory provider that is always successful. private RazorViewEngine CreateSuccessfulViewEngine() { diff --git a/test/WebSites/RazorWebSite/Controllers/EmbeddedViewsController.cs b/test/WebSites/RazorWebSite/Controllers/EmbeddedViewsController.cs index 33837ea026..ca653f83a0 100644 --- a/test/WebSites/RazorWebSite/Controllers/EmbeddedViewsController.cs +++ b/test/WebSites/RazorWebSite/Controllers/EmbeddedViewsController.cs @@ -7,6 +7,10 @@ namespace RazorWebSite.Controllers { public class EmbeddedViewsController : Controller { - public IActionResult Index() => View("/Views/EmbeddedHome/Index.cshtml"); + public IActionResult LookupByName() => View("Index"); + + public IActionResult LookupByPath() => View("/Views/EmbeddedViews/Index.cshtml"); + + public IActionResult RelativeNonPath() => View(); } } diff --git a/test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedShared/_Layout.cshtml b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedShared/_Layout.cshtml similarity index 100% rename from test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedShared/_Layout.cshtml rename to test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedShared/_Layout.cshtml diff --git a/test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedShared/_Partial.cshtml b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedShared/_Partial.cshtml similarity index 100% rename from test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedShared/_Partial.cshtml rename to test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedShared/_Partial.cshtml diff --git a/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/EmbeddedPartial.cshtml b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/EmbeddedPartial.cshtml new file mode 100644 index 0000000000..cebe57816a --- /dev/null +++ b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/EmbeddedPartial.cshtml @@ -0,0 +1 @@ +Hello from EmbeddedHome\EmbeddedPartial \ No newline at end of file diff --git a/test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedHome/Index.cshtml b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/Index.cshtml similarity index 100% rename from test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedHome/Index.cshtml rename to test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/Index.cshtml diff --git a/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/RelativeNonPath.cshtml b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/RelativeNonPath.cshtml new file mode 100644 index 0000000000..1aca712209 --- /dev/null +++ b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/RelativeNonPath.cshtml @@ -0,0 +1,2 @@ +@{ Layout = "../EmbeddedShared/_Layout"; } +@Html.Partial("./EmbeddedPartial") \ No newline at end of file diff --git a/test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedHome/_ViewImports.cshtml b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/_ViewImports.cshtml similarity index 100% rename from test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedHome/_ViewImports.cshtml rename to test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/_ViewImports.cshtml diff --git a/test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedHome/_ViewStart.cshtml b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/_ViewStart.cshtml similarity index 100% rename from test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedHome/_ViewStart.cshtml rename to test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/_ViewStart.cshtml diff --git a/test/WebSites/RazorWebSite/EmbeddedViews/Views/Shared/_EmbeddedPartial.cshtml b/test/WebSites/RazorWebSite/EmbeddedResources/Views/Shared/_EmbeddedPartial.cshtml similarity index 100% rename from test/WebSites/RazorWebSite/EmbeddedViews/Views/Shared/_EmbeddedPartial.cshtml rename to test/WebSites/RazorWebSite/EmbeddedResources/Views/Shared/_EmbeddedPartial.cshtml diff --git a/test/WebSites/RazorWebSite/RazorWebSite.csproj b/test/WebSites/RazorWebSite/RazorWebSite.csproj index 5ac6a97546..dac6890ecb 100644 --- a/test/WebSites/RazorWebSite/RazorWebSite.csproj +++ b/test/WebSites/RazorWebSite/RazorWebSite.csproj @@ -7,7 +7,7 @@ - + diff --git a/test/WebSites/RazorWebSite/Startup.cs b/test/WebSites/RazorWebSite/Startup.cs index 4bb646bc5c..7fb43f75e8 100644 --- a/test/WebSites/RazorWebSite/Startup.cs +++ b/test/WebSites/RazorWebSite/Startup.cs @@ -30,7 +30,7 @@ public void ConfigureServices(IServiceCollection services) { options.FileProviders.Add(new EmbeddedFileProvider( typeof(Startup).GetTypeInfo().Assembly, - $"{nameof(RazorWebSite)}.EmbeddedViews")); + $"{nameof(RazorWebSite)}.EmbeddedResources")); options.FileProviders.Add(updateableFileProvider); options.ViewLocationExpanders.Add(new NonMainPageViewLocationExpander()); options.ViewLocationExpanders.Add(new ForwardSlashExpander());