Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Commit

Permalink
Normalize paths in RazorViewEngine prior to invoking page factory
Browse files Browse the repository at this point in the history
Fixes #6672
  • Loading branch information
pranavkm committed Sep 6, 2017
1 parent 1d1a520 commit b3c7a3c
Show file tree
Hide file tree
Showing 15 changed files with 154 additions and 17 deletions.
40 changes: 32 additions & 8 deletions src/Microsoft.AspNetCore.Mvc.Core/Internal/ViewEnginePath.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,26 @@ 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[] { '/', '\\' };
// Tokens that suggest directory traversal such as ./, \.. etc. All parent directory traversals (such as "..\") are covered
// by current directory tokens (".\").
private static readonly string[] TokensRequiringNormalization = new string[]
{
// ./
CurrentDirectoryToken + PathSeparators[0],
// .\
CurrentDirectoryToken + PathSeparators[1],
// /.
PathSeparators[0] + CurrentDirectoryToken,
// \.
PathSeparators[1] + CurrentDirectoryToken,
// //
string.Empty + PathSeparators[0] + PathSeparators[0],
// \\
string.Empty + PathSeparators[1] + PathSeparators[1],
};

public static string CombinePath(string first, string second)
{
Expand Down Expand Up @@ -42,18 +59,18 @@ public static string CombinePath(string first, string second)
result = first.Substring(0, index + 1) + second;
}

return ResolvePath(result);
return NormalizePath(result);
}

public static string ResolvePath(string path)
public static string NormalizePath(string path)
{
if (!RequiresPathResolution(path))
if (!RequiresPathNormalization(path))
{
return path;
}

var pathSegments = new List<StringSegment>();
var tokenizer = new StringTokenizer(path, _pathSeparators);
var tokenizer = new StringTokenizer(path, PathSeparators);
foreach (var segment in tokenizer)
{
if (segment.Length == 0)
Expand Down Expand Up @@ -93,10 +110,17 @@ public static string ResolvePath(string path)
return builder.ToString();
}

private static bool RequiresPathResolution(string path)
private static bool RequiresPathNormalization(string path)
{
return path.IndexOf(ParentDirectoryToken, StringComparison.Ordinal) != -1 ||
path.IndexOf(CurrentDirectoryToken, StringComparison.Ordinal) != -1;
for (var i = 0; i < TokensRequiringNormalization.Length; i++)
{
if (path.IndexOf(TokensRequiringNormalization[i]) != -1)
{
return true;
}
}

return false;
}
}
}
7 changes: 4 additions & 3 deletions src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -316,7 +315,7 @@ public string GetAbsolutePath(string executingFilePath, string pagePath)
// path relative to currently-executing view, if any.
// Not yet executing a view. Start in app root.
var absolutePath = "/" + pagePath;
return ViewEnginePath.ResolvePath(absolutePath);
return ViewEnginePath.NormalizePath(absolutePath);
}

return ViewEnginePath.CombinePath(executingFilePath, pagePath);
Expand Down Expand Up @@ -369,6 +368,8 @@ private ViewLocationCacheResult OnCacheMiss(
expanderContext.ControllerName,
expanderContext.AreaName);

path = ViewEnginePath.NormalizePath(path);

cacheResult = CreateCacheResult(expirationTokens, path, expanderContext.IsMainPage);
if (cacheResult != null)
{
Expand Down
34 changes: 32 additions & 2 deletions test/Microsoft.AspNetCore.Mvc.FunctionalTests/ViewEngineTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ public async Task RazorViewEngine_RendersPartialViews(string actionName, string
}

[Fact]
public async Task RazorViewEngine_RendersViewsFromEmbeddedFileProvider()
public async Task RazorViewEngine_RendersViewsFromEmbeddedFileProvider_WhenLookedupByName()
{
// Arrange
var expected =
Expand All @@ -257,7 +257,24 @@ Hello from Shared/_EmbeddedPartial
</embdedded-layout>";

// Act
var body = await Client.GetStringAsync("/EmbeddedViews");
var body = await Client.GetStringAsync("/EmbeddedViews/LookupByName");

// Assert
Assert.Equal(expected, body.Trim(), ignoreLineEndingDifferences: true);
}

[Fact]
public async Task RazorViewEngine_RendersViewsFromEmbeddedFileProvider_WhenLookedupByPath()
{
// Arrange
var expected =
@"<embdedded-layout>Hello from EmbeddedShared/_Partial
Hello from Shared/_EmbeddedPartial
<a href=""/EmbeddedViews"">Tag Helper Link</a>
</embdedded-layout>";

// Act
var body = await Client.GetStringAsync("/EmbeddedViews/LookupByPath");

// Assert
Assert.Equal(expected, body.Trim(), ignoreLineEndingDifferences: true);
Expand Down Expand Up @@ -493,5 +510,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());
}
}
}
77 changes: 76 additions & 1 deletion test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
using Moq;
using Xunit;

namespace Microsoft.AspNetCore.Mvc.Razor.Test
namespace Microsoft.AspNetCore.Mvc.Razor
{
public class RazorViewEngineTest
{
Expand Down Expand Up @@ -1864,6 +1864,81 @@ 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();
}

// Prior to 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()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hello from EmbeddedHome\EmbeddedPartial
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@{ Layout = "../EmbeddedShared/_Layout"; }
@Html.Partial("./EmbeddedPartial")
2 changes: 1 addition & 1 deletion test/WebSites/RazorWebSite/RazorWebSite.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
</PropertyGroup>

<ItemGroup>
<EmbeddedResource Include="EmbeddedViews\**" />
<EmbeddedResource Include="EmbeddedResources\**" />
</ItemGroup>

<ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion test/WebSites/RazorWebSite/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down

0 comments on commit b3c7a3c

Please sign in to comment.