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

Commit

Permalink
Port fix for URL helper redirect (#6916)
Browse files Browse the repository at this point in the history
Fixes #6910
  • Loading branch information
sebastienros authored Oct 5, 2017
1 parent bf7c9e0 commit c4400d2
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 6 deletions.
52 changes: 46 additions & 6 deletions src/Microsoft.AspNetCore.Mvc.Core/Routing/UrlHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
/// </summary>
public class UrlHelper : IUrlHelper
{
internal const string UseRelaxedLocalRedirectValidationSwitch = "Switch.Microsoft.AspNetCore.Mvc.UseRelaxedLocalRedirectValidation";

// Perf: Share the StringBuilder object across multiple calls of GenerateURL for this UrlHelper
private StringBuilder _stringBuilder;
Expand Down Expand Up @@ -102,14 +103,53 @@ public virtual string Action(UrlActionContext actionContext)
/// <inheritdoc />
public virtual bool IsLocalUrl(string url)
{
return
!string.IsNullOrEmpty(url) &&
if (string.IsNullOrEmpty(url))
{
return false;
}

// Allows "/" or "/foo" but not "//" or "/\".
if (url[0] == '/')
{
// url is exactly "/"
if (url.Length == 1)
{
return true;
}

// url doesn't start with "//" or "/\"
if (url[1] != '/' && url[1] != '\\')
{
return true;
}

return false;
}

// Allows "~/" or "~/foo" but not "~//" or "~/\".
if (url[0] == '~' && url.Length > 1 && url[1] == '/')
{
// url is exactly "~/"
if (url.Length == 2)
{
return true;
}

// url doesn't start with "~//" or "~/\"
if (url[2] != '/' && url[2] != '\\')
{
return true;
}

// Allows "/" or "/foo" but not "//" or "/\".
((url[0] == '/' && (url.Length == 1 || (url[1] != '/' && url[1] != '\\'))) ||
if (AppContext.TryGetSwitch(UseRelaxedLocalRedirectValidationSwitch, out var relaxedLocalRedirectValidation) && relaxedLocalRedirectValidation)
{
return true;
}

// Allows "~/" or "~/foo".
(url.Length > 1 && url[0] == '~' && url[1] == '/'));
return false;
}

return false;
}

/// <inheritdoc />
Expand Down
42 changes: 42 additions & 0 deletions test/Microsoft.AspNetCore.Mvc.Core.Test/LocalRedirectResultTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ public void Execute_ReturnsExpectedValues()
}

[Theory]
[InlineData("", "//", "/test")]
[InlineData("", "/\\", "/test")]
[InlineData("", "//foo", "/test")]
[InlineData("", "/\\foo", "/test")]
[InlineData("", "Home/About", "/Home/About")]
[InlineData("/myapproot", "http://www.example.com", "/test")]
public void Execute_Throws_ForNonLocalUrl(
Expand All @@ -109,6 +113,44 @@ public void Execute_Throws_ForNonLocalUrl(
exception.Message);
}

[Theory]
[InlineData("", "~//", "//")]
[InlineData("", "~/\\", "/\\")]
[InlineData("", "~//foo", "//foo")]
[InlineData("", "~/\\foo", "/\\foo")]
public void Execute_Throws_ForNonLocalUrlTilde(
string appRoot,
string contentPath,
string expectedPath)
{
// Arrange
var httpResponse = new Mock<HttpResponse>();
httpResponse.Setup(o => o.Redirect(expectedPath, false))
.Verifiable();

var httpContext = GetHttpContext(appRoot, contentPath, expectedPath, httpResponse.Object);
var actionContext = GetActionContext(httpContext);
var result = new LocalRedirectResult(contentPath);

var relaxedLocalRedirectValidation = false;
var success = AppContext.TryGetSwitch(UrlHelper.UseRelaxedLocalRedirectValidationSwitch, out relaxedLocalRedirectValidation);

// Act & Assert
if (relaxedLocalRedirectValidation)
{
result.ExecuteResult(actionContext);
httpResponse.Verify();
}
else
{
var exception = Assert.Throws<InvalidOperationException>(() => result.ExecuteResult(actionContext));
Assert.Equal(
"The supplied URL is not local. A URL with an absolute path is considered local if it does not " +
"have a host/authority part. URLs using virtual paths ('~/') are also local.",
exception.Message);
}
}

private static ActionContext GetActionContext(HttpContext httpContext)
{
var routeData = new RouteData();
Expand Down
54 changes: 54 additions & 0 deletions test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/UrlHelperTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,60 @@ public void IsLocalUrl_RejectsInvalidUrls(string url)
Assert.False(result);
}

[Theory]
[InlineData("~//www.example.com")]
[InlineData("~//www.example.com?")]
[InlineData("~//www.example.com:80")]
[InlineData("~//www.example.com/foobar.html")]
[InlineData("~///www.example.com")]
[InlineData("~//////www.example.com")]
public void IsLocalUrl_RejectsTokenUrlsWithMissingSchemeName(string url)
{
// Arrange
var helper = CreateUrlHelper("www.mysite.com");

// Act
var result = helper.IsLocalUrl(url);

// Assert
var relaxedLocalRedirectValidation = false;
var success = AppContext.TryGetSwitch(UrlHelper.UseRelaxedLocalRedirectValidationSwitch, out relaxedLocalRedirectValidation);

if (relaxedLocalRedirectValidation)
{
Assert.True(result);
}
else
{
Assert.False(result);
}
}

[Theory]
[InlineData("~/\\")]
[InlineData("~/\\foo")]
public void IsLocalUrl_RejectsInvalidTokenUrls(string url)
{
// Arrange
var helper = CreateUrlHelper("www.mysite.com");

// Act
var result = helper.IsLocalUrl(url);

// Assert
var relaxedLocalRedirectValidation = false;
var success = AppContext.TryGetSwitch(UrlHelper.UseRelaxedLocalRedirectValidationSwitch, out relaxedLocalRedirectValidation);

if (relaxedLocalRedirectValidation)
{
Assert.True(result);
}
else
{
Assert.False(result);
}
}

[Fact]
public void RouteUrlWithDictionary()
{
Expand Down

0 comments on commit c4400d2

Please sign in to comment.