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

Commit c4400d2

Browse files
authored
Port fix for URL helper redirect (#6916)
Fixes #6910
1 parent bf7c9e0 commit c4400d2

File tree

3 files changed

+142
-6
lines changed

3 files changed

+142
-6
lines changed

src/Microsoft.AspNetCore.Mvc.Core/Routing/UrlHelper.cs

+46-6
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
1616
/// </summary>
1717
public class UrlHelper : IUrlHelper
1818
{
19+
internal const string UseRelaxedLocalRedirectValidationSwitch = "Switch.Microsoft.AspNetCore.Mvc.UseRelaxedLocalRedirectValidation";
1920

2021
// Perf: Share the StringBuilder object across multiple calls of GenerateURL for this UrlHelper
2122
private StringBuilder _stringBuilder;
@@ -102,14 +103,53 @@ public virtual string Action(UrlActionContext actionContext)
102103
/// <inheritdoc />
103104
public virtual bool IsLocalUrl(string url)
104105
{
105-
return
106-
!string.IsNullOrEmpty(url) &&
106+
if (string.IsNullOrEmpty(url))
107+
{
108+
return false;
109+
}
110+
111+
// Allows "/" or "/foo" but not "//" or "/\".
112+
if (url[0] == '/')
113+
{
114+
// url is exactly "/"
115+
if (url.Length == 1)
116+
{
117+
return true;
118+
}
119+
120+
// url doesn't start with "//" or "/\"
121+
if (url[1] != '/' && url[1] != '\\')
122+
{
123+
return true;
124+
}
125+
126+
return false;
127+
}
128+
129+
// Allows "~/" or "~/foo" but not "~//" or "~/\".
130+
if (url[0] == '~' && url.Length > 1 && url[1] == '/')
131+
{
132+
// url is exactly "~/"
133+
if (url.Length == 2)
134+
{
135+
return true;
136+
}
137+
138+
// url doesn't start with "~//" or "~/\"
139+
if (url[2] != '/' && url[2] != '\\')
140+
{
141+
return true;
142+
}
107143

108-
// Allows "/" or "/foo" but not "//" or "/\".
109-
((url[0] == '/' && (url.Length == 1 || (url[1] != '/' && url[1] != '\\'))) ||
144+
if (AppContext.TryGetSwitch(UseRelaxedLocalRedirectValidationSwitch, out var relaxedLocalRedirectValidation) && relaxedLocalRedirectValidation)
145+
{
146+
return true;
147+
}
110148

111-
// Allows "~/" or "~/foo".
112-
(url.Length > 1 && url[0] == '~' && url[1] == '/'));
149+
return false;
150+
}
151+
152+
return false;
113153
}
114154

115155
/// <inheritdoc />

test/Microsoft.AspNetCore.Mvc.Core.Test/LocalRedirectResultTest.cs

+42
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@ public void Execute_ReturnsExpectedValues()
8585
}
8686

8787
[Theory]
88+
[InlineData("", "//", "/test")]
89+
[InlineData("", "/\\", "/test")]
90+
[InlineData("", "//foo", "/test")]
91+
[InlineData("", "/\\foo", "/test")]
8892
[InlineData("", "Home/About", "/Home/About")]
8993
[InlineData("/myapproot", "http://www.example.com", "/test")]
9094
public void Execute_Throws_ForNonLocalUrl(
@@ -109,6 +113,44 @@ public void Execute_Throws_ForNonLocalUrl(
109113
exception.Message);
110114
}
111115

116+
[Theory]
117+
[InlineData("", "~//", "//")]
118+
[InlineData("", "~/\\", "/\\")]
119+
[InlineData("", "~//foo", "//foo")]
120+
[InlineData("", "~/\\foo", "/\\foo")]
121+
public void Execute_Throws_ForNonLocalUrlTilde(
122+
string appRoot,
123+
string contentPath,
124+
string expectedPath)
125+
{
126+
// Arrange
127+
var httpResponse = new Mock<HttpResponse>();
128+
httpResponse.Setup(o => o.Redirect(expectedPath, false))
129+
.Verifiable();
130+
131+
var httpContext = GetHttpContext(appRoot, contentPath, expectedPath, httpResponse.Object);
132+
var actionContext = GetActionContext(httpContext);
133+
var result = new LocalRedirectResult(contentPath);
134+
135+
var relaxedLocalRedirectValidation = false;
136+
var success = AppContext.TryGetSwitch(UrlHelper.UseRelaxedLocalRedirectValidationSwitch, out relaxedLocalRedirectValidation);
137+
138+
// Act & Assert
139+
if (relaxedLocalRedirectValidation)
140+
{
141+
result.ExecuteResult(actionContext);
142+
httpResponse.Verify();
143+
}
144+
else
145+
{
146+
var exception = Assert.Throws<InvalidOperationException>(() => result.ExecuteResult(actionContext));
147+
Assert.Equal(
148+
"The supplied URL is not local. A URL with an absolute path is considered local if it does not " +
149+
"have a host/authority part. URLs using virtual paths ('~/') are also local.",
150+
exception.Message);
151+
}
152+
}
153+
112154
private static ActionContext GetActionContext(HttpContext httpContext)
113155
{
114156
var routeData = new RouteData();

test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/UrlHelperTest.cs

+54
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,60 @@ public void IsLocalUrl_RejectsInvalidUrls(string url)
261261
Assert.False(result);
262262
}
263263

264+
[Theory]
265+
[InlineData("~//www.example.com")]
266+
[InlineData("~//www.example.com?")]
267+
[InlineData("~//www.example.com:80")]
268+
[InlineData("~//www.example.com/foobar.html")]
269+
[InlineData("~///www.example.com")]
270+
[InlineData("~//////www.example.com")]
271+
public void IsLocalUrl_RejectsTokenUrlsWithMissingSchemeName(string url)
272+
{
273+
// Arrange
274+
var helper = CreateUrlHelper("www.mysite.com");
275+
276+
// Act
277+
var result = helper.IsLocalUrl(url);
278+
279+
// Assert
280+
var relaxedLocalRedirectValidation = false;
281+
var success = AppContext.TryGetSwitch(UrlHelper.UseRelaxedLocalRedirectValidationSwitch, out relaxedLocalRedirectValidation);
282+
283+
if (relaxedLocalRedirectValidation)
284+
{
285+
Assert.True(result);
286+
}
287+
else
288+
{
289+
Assert.False(result);
290+
}
291+
}
292+
293+
[Theory]
294+
[InlineData("~/\\")]
295+
[InlineData("~/\\foo")]
296+
public void IsLocalUrl_RejectsInvalidTokenUrls(string url)
297+
{
298+
// Arrange
299+
var helper = CreateUrlHelper("www.mysite.com");
300+
301+
// Act
302+
var result = helper.IsLocalUrl(url);
303+
304+
// Assert
305+
var relaxedLocalRedirectValidation = false;
306+
var success = AppContext.TryGetSwitch(UrlHelper.UseRelaxedLocalRedirectValidationSwitch, out relaxedLocalRedirectValidation);
307+
308+
if (relaxedLocalRedirectValidation)
309+
{
310+
Assert.True(result);
311+
}
312+
else
313+
{
314+
Assert.False(result);
315+
}
316+
}
317+
264318
[Fact]
265319
public void RouteUrlWithDictionary()
266320
{

0 commit comments

Comments
 (0)