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

Commit

Permalink
Fix IsLocalUrl
Browse files Browse the repository at this point in the history
  • Loading branch information
@sebastienros
sebastienros authored Apr 26, 2017
1 parent ba2ab93 commit eec16ff
Show file tree
Hide file tree
Showing 5 changed files with 188 additions and 8 deletions.
64 changes: 58 additions & 6 deletions src/Microsoft.AspNetCore.Mvc.Core/Routing/UrlHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@

using System;
using System.Collections.Generic;
#if NET451
using System.Configuration;
using System.Linq;
#endif
using System.Diagnostics;
using System.Text;
using Microsoft.AspNetCore.Http;
Expand All @@ -16,6 +20,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 +107,61 @@ 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 "/\".
((url[0] == '/' && (url.Length == 1 || (url[1] != '/' && url[1] != '\\'))) ||
// Allows "/" or "/foo" but not "//" or "/\".
if (url[0] == '/')
{
// url is exactly "/"
if (url.Length == 1)
{
return true;
}

// Allows "~/" or "~/foo".
(url.Length > 1 && url[0] == '~' && url[1] == '/'));
// 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;
}

var relaxedLocalRedirectValidation = false;

#if NET451
var value = ConfigurationManager.AppSettings.GetValues(UseRelaxedLocalRedirectValidationSwitch)?.FirstOrDefault();
var success = bool.TryParse(value, out relaxedLocalRedirectValidation);
#else
var success = AppContext.TryGetSwitch(UseRelaxedLocalRedirectValidationSwitch, out relaxedLocalRedirectValidation);
#endif
if (relaxedLocalRedirectValidation)
{
return true;
}

return false;
}

return false;
}

/// <inheritdoc />
Expand Down
6 changes: 5 additions & 1 deletion src/Microsoft.AspNetCore.Mvc.Core/project.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,11 @@
"System.Diagnostics.DiagnosticSource": "4.3.1"
},
"frameworks": {
"net451": {},
"net451": {
"frameworkAssemblies": {
"System.Configuration": ""
}
},
"netstandard1.6": {}
}
}
52 changes: 52 additions & 0 deletions test/Microsoft.AspNetCore.Mvc.Core.Test/LocalRedirectResultTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
#if NET451
using System.Configuration;
using System.Linq;
#endif
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Internal;
using Microsoft.AspNetCore.Mvc.Abstractions;
Expand Down Expand Up @@ -68,6 +72,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 @@ -92,6 +100,50 @@ 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;

#if NET451
var value = ConfigurationManager.AppSettings.GetValues(UrlHelper.UseRelaxedLocalRedirectValidationSwitch)?.FirstOrDefault();
var success = bool.TryParse(value, out relaxedLocalRedirectValidation);
#else
var success = AppContext.TryGetSwitch(UrlHelper.UseRelaxedLocalRedirectValidationSwitch, out relaxedLocalRedirectValidation);
#endif

// 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
68 changes: 68 additions & 0 deletions test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/UrlHelperTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@

using System;
using System.Collections.Generic;
#if NET451
using System.Configuration;
using System.Linq;
# endif
using System.Text;
using System.Text.Encodings.Web;
using System.Threading.Tasks;
Expand Down Expand Up @@ -261,6 +265,70 @@ 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;

#if NET451
var value = ConfigurationManager.AppSettings.GetValues(UrlHelper.UseRelaxedLocalRedirectValidationSwitch)?.FirstOrDefault();
var success = bool.TryParse(value, out relaxedLocalRedirectValidation);
#else
var success = AppContext.TryGetSwitch(UrlHelper.UseRelaxedLocalRedirectValidationSwitch, out relaxedLocalRedirectValidation);
#endif
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;

#if NET451
var value = ConfigurationManager.AppSettings.GetValues(UrlHelper.UseRelaxedLocalRedirectValidationSwitch)?.FirstOrDefault();
var success = bool.TryParse(value, out relaxedLocalRedirectValidation);
#else
var success = AppContext.TryGetSwitch(UrlHelper.UseRelaxedLocalRedirectValidationSwitch, out relaxedLocalRedirectValidation);
#endif
if (relaxedLocalRedirectValidation)
{
Assert.True(result);
}
else
{
Assert.False(result);
}
}

[Fact]
public void RouteUrlWithDictionary()
{
Expand Down
6 changes: 5 additions & 1 deletion test/Microsoft.AspNetCore.Mvc.Core.Test/project.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@
"System.Diagnostics.TraceSource": "4.3.0"
}
},
"net451": {}
"net451": {
"frameworkAssemblies": {
"System.Configuration": ""
}
}
}
}

0 comments on commit eec16ff

Please sign in to comment.