Normalize paths in RazorViewEngine prior to invoking page factory

Fixes #6672

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<StringSegment>();
-            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;
-        }
     }
 }

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<IChangeToken>();
                 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)
                 {

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);
+        }
+    }
+}

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
 </embdedded-layout>";
 
             // 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 = @"<embdedded-layout>Hello from EmbeddedHome\EmbeddedPartial</embdedded-layout>";
+
+            // Act
+            var responseContent = await Client.GetStringAsync("/EmbeddedViews/RelativeNonPath");
+
+            // Assert
+            Assert.Equal(expected, responseContent.Trim());
+        }
     }
 }

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<IRazorPageFactoryProvider>();
+            pageFactory.Setup(p => p.CreateFactory("/Views/Shared/_Partial.cshtml"))
+                .Returns(GetPageFactoryResult(() => Mock.Of<IRazorPage>()))
+                .Verifiable();
+            var viewEngine = CreateViewEngine(pageFactory.Object);
+            var context = GetActionContext(new Dictionary<string, object>());
+
+            // Act
+            var result = viewEngine.FindView(context, "../Shared/_Partial", isMainPage: false);
+
+            // Assert
+            pageFactory.Verify();
+        }
+
+        [Fact]
+        public void FindPage_ResolvesDirectoryTraversalsPriorToInvokingPageFactory()
+        {
+            // Arrange
+            var pageFactory = new Mock<IRazorPageFactoryProvider>();
+            pageFactory.Setup(p => p.CreateFactory("/Views/Shared/_Partial.cshtml"))
+                .Returns(GetPageFactoryResult(() => Mock.Of<IRazorPage>()))
+                .Verifiable();
+            var viewEngine = CreateViewEngine(pageFactory.Object);
+            var context = GetActionContext(new Dictionary<string, object>());
+
+            // 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<IRazorPageFactoryProvider>();
+            pageFactory.Setup(p => p.CreateFactory("/Views/MyView.cshtml"))
+                .Returns(GetPageFactoryResult(() => Mock.Of<IRazorPage>()))
+                .Verifiable();
+            var viewEngine = CreateViewEngine(pageFactory.Object);
+            var context = GetActionContext(new Dictionary<string, object>());
+
+            // Act
+            var result = viewEngine.FindView(context, "MyView", isMainPage: true);
+
+            // Assert
+            pageFactory.Verify();
+        }
+
+        [Fact]
+        public void FindPage_ResolvesNormalizesSlashesPriorToInvokingPageFactory()
+        {
+            // Arrange
+            var pageFactory = new Mock<IRazorPageFactoryProvider>();
+            pageFactory.Setup(p => p.CreateFactory("/Views/MyPage.cshtml"))
+                .Returns(GetPageFactoryResult(() => Mock.Of<IRazorPage>()))
+                .Verifiable();
+            var viewEngine = CreateViewEngine(pageFactory.Object);
+            var context = GetActionContext(new Dictionary<string, object>());
+
+            // Act
+            var result = viewEngine.FindPage(context, "MyPage");
+
+            // Assert
+            pageFactory.Verify();
+        }
+
         // Return RazorViewEngine with a page factory provider that is always successful.
         private RazorViewEngine CreateSuccessfulViewEngine()
         {

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();
     }
 }

test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/EmbeddedPartial.cshtml

@@ -0,0 +1 @@
+Hello from EmbeddedHome\EmbeddedPartial

test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/RelativeNonPath.cshtml

@@ -0,0 +1,2 @@
+@{ Layout = "../EmbeddedShared/_Layout"; }
+@Html.Partial("./EmbeddedPartial")

test/WebSites/RazorWebSite/RazorWebSite.csproj

@@ -7,7 +7,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <EmbeddedResource Include="EmbeddedViews\**" />
+    <EmbeddedResource Include="EmbeddedResources\**" />
   </ItemGroup>
 
   <ItemGroup>

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());