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

Commit eec16ff

Browse files
sebastienrossebastienros
authored
Fix IsLocalUrl
1 parent ba2ab93 commit eec16ff

File tree

5 files changed

+188
-8
lines changed

5 files changed

+188
-8
lines changed

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

Lines changed: 58 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33

44
using System;
55
using System.Collections.Generic;
6+
#if NET451
7+
using System.Configuration;
8+
using System.Linq;
9+
#endif
610
using System.Diagnostics;
711
using System.Text;
812
using Microsoft.AspNetCore.Http;
@@ -16,6 +20,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
1620
/// </summary>
1721
public class UrlHelper : IUrlHelper
1822
{
23+
internal const string UseRelaxedLocalRedirectValidationSwitch = "Switch.Microsoft.AspNetCore.Mvc.UseRelaxedLocalRedirectValidation";
1924

2025
// Perf: Share the StringBuilder object across multiple calls of GenerateURL for this UrlHelper
2126
private StringBuilder _stringBuilder;
@@ -102,14 +107,61 @@ public virtual string Action(UrlActionContext actionContext)
102107
/// <inheritdoc />
103108
public virtual bool IsLocalUrl(string url)
104109
{
105-
return
106-
!string.IsNullOrEmpty(url) &&
110+
if (string.IsNullOrEmpty(url))
111+
{
112+
return false;
113+
}
107114

108-
// Allows "/" or "/foo" but not "//" or "/\".
109-
((url[0] == '/' && (url.Length == 1 || (url[1] != '/' && url[1] != '\\'))) ||
115+
// Allows "/" or "/foo" but not "//" or "/\".
116+
if (url[0] == '/')
117+
{
118+
// url is exactly "/"
119+
if (url.Length == 1)
120+
{
121+
return true;
122+
}
110123

111-
// Allows "~/" or "~/foo".
112-
(url.Length > 1 && url[0] == '~' && url[1] == '/'));
124+
// url doesn't start with "//" or "/\"
125+
if (url[1] != '/' && url[1] != '\\')
126+
{
127+
return true;
128+
}
129+
130+
return false;
131+
}
132+
133+
// Allows "~/" or "~/foo" but not "~//" or "~/\".
134+
if (url[0] == '~' && url.Length > 1 && url[1] == '/')
135+
{
136+
// url is exactly "~/"
137+
if (url.Length == 2)
138+
{
139+
return true;
140+
}
141+
142+
// url doesn't start with "~//" or "~/\"
143+
if (url[2] != '/' && url[2] != '\\')
144+
{
145+
return true;
146+
}
147+
148+
var relaxedLocalRedirectValidation = false;
149+
150+
#if NET451
151+
var value = ConfigurationManager.AppSettings.GetValues(UseRelaxedLocalRedirectValidationSwitch)?.FirstOrDefault();
152+
var success = bool.TryParse(value, out relaxedLocalRedirectValidation);
153+
#else
154+
var success = AppContext.TryGetSwitch(UseRelaxedLocalRedirectValidationSwitch, out relaxedLocalRedirectValidation);
155+
#endif
156+
if (relaxedLocalRedirectValidation)
157+
{
158+
return true;
159+
}
160+
161+
return false;
162+
}
163+
164+
return false;
113165
}
114166

115167
/// <inheritdoc />

‎src/Microsoft.AspNetCore.Mvc.Core/project.json

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,11 @@
5959
"System.Diagnostics.DiagnosticSource": "4.3.1"
6060
},
6161
"frameworks": {
62-
"net451": {},
62+
"net451": {
63+
"frameworkAssemblies": {
64+
"System.Configuration": ""
65+
}
66+
},
6367
"netstandard1.6": {}
6468
}
6569
}

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5+
#if NET451
6+
using System.Configuration;
7+
using System.Linq;
8+
#endif
59
using Microsoft.AspNetCore.Http;
610
using Microsoft.AspNetCore.Http.Internal;
711
using Microsoft.AspNetCore.Mvc.Abstractions;
@@ -68,6 +72,10 @@ public void Execute_ReturnsExpectedValues()
6872
}
6973

7074
[Theory]
75+
[InlineData("", "//", "/test")]
76+
[InlineData("", "/\\", "/test")]
77+
[InlineData("", "//foo", "/test")]
78+
[InlineData("", "/\\foo", "/test")]
7179
[InlineData("", "Home/About", "/Home/About")]
7280
[InlineData("/myapproot", "http://www.example.com", "/test")]
7381
public void Execute_Throws_ForNonLocalUrl(
@@ -92,6 +100,50 @@ public void Execute_Throws_ForNonLocalUrl(
92100
exception.Message);
93101
}
94102

103+
[Theory]
104+
[InlineData("", "~//", "//")]
105+
[InlineData("", "~/\\", "/\\")]
106+
[InlineData("", "~//foo", "//foo")]
107+
[InlineData("", "~/\\foo", "/\\foo")]
108+
public void Execute_Throws_ForNonLocalUrlTilde(
109+
string appRoot,
110+
string contentPath,
111+
string expectedPath)
112+
{
113+
// Arrange
114+
var httpResponse = new Mock<HttpResponse>();
115+
httpResponse.Setup(o => o.Redirect(expectedPath, false))
116+
.Verifiable();
117+
118+
var httpContext = GetHttpContext(appRoot, contentPath, expectedPath, httpResponse.Object);
119+
var actionContext = GetActionContext(httpContext);
120+
var result = new LocalRedirectResult(contentPath);
121+
122+
var relaxedLocalRedirectValidation = false;
123+
124+
#if NET451
125+
var value = ConfigurationManager.AppSettings.GetValues(UrlHelper.UseRelaxedLocalRedirectValidationSwitch)?.FirstOrDefault();
126+
var success = bool.TryParse(value, out relaxedLocalRedirectValidation);
127+
#else
128+
var success = AppContext.TryGetSwitch(UrlHelper.UseRelaxedLocalRedirectValidationSwitch, out relaxedLocalRedirectValidation);
129+
#endif
130+
131+
// Act & Assert
132+
if (relaxedLocalRedirectValidation)
133+
{
134+
result.ExecuteResult(actionContext);
135+
httpResponse.Verify();
136+
}
137+
else
138+
{
139+
var exception = Assert.Throws<InvalidOperationException>(() => result.ExecuteResult(actionContext));
140+
Assert.Equal(
141+
"The supplied URL is not local. A URL with an absolute path is considered local if it does not " +
142+
"have a host/authority part. URLs using virtual paths ('~/') are also local.",
143+
exception.Message);
144+
}
145+
}
146+
95147
private static ActionContext GetActionContext(HttpContext httpContext)
96148
{
97149
var routeData = new RouteData();

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33

44
using System;
55
using System.Collections.Generic;
6+
#if NET451
7+
using System.Configuration;
8+
using System.Linq;
9+
# endif
610
using System.Text;
711
using System.Text.Encodings.Web;
812
using System.Threading.Tasks;
@@ -261,6 +265,70 @@ public void IsLocalUrl_RejectsInvalidUrls(string url)
261265
Assert.False(result);
262266
}
263267

268+
[Theory]
269+
[InlineData("~//www.example.com")]
270+
[InlineData("~//www.example.com?")]
271+
[InlineData("~//www.example.com:80")]
272+
[InlineData("~//www.example.com/foobar.html")]
273+
[InlineData("~///www.example.com")]
274+
[InlineData("~//////www.example.com")]
275+
public void IsLocalUrl_RejectsTokenUrlsWithMissingSchemeName(string url)
276+
{
277+
// Arrange
278+
var helper = CreateUrlHelper("www.mysite.com");
279+
280+
// Act
281+
var result = helper.IsLocalUrl(url);
282+
283+
// Assert
284+
var relaxedLocalRedirectValidation = false;
285+
286+
#if NET451
287+
var value = ConfigurationManager.AppSettings.GetValues(UrlHelper.UseRelaxedLocalRedirectValidationSwitch)?.FirstOrDefault();
288+
var success = bool.TryParse(value, out relaxedLocalRedirectValidation);
289+
#else
290+
var success = AppContext.TryGetSwitch(UrlHelper.UseRelaxedLocalRedirectValidationSwitch, out relaxedLocalRedirectValidation);
291+
#endif
292+
if (relaxedLocalRedirectValidation)
293+
{
294+
Assert.True(result);
295+
}
296+
else
297+
{
298+
Assert.False(result);
299+
}
300+
}
301+
302+
[Theory]
303+
[InlineData("~/\\")]
304+
[InlineData("~/\\foo")]
305+
public void IsLocalUrl_RejectsInvalidTokenUrls(string url)
306+
{
307+
// Arrange
308+
var helper = CreateUrlHelper("www.mysite.com");
309+
310+
// Act
311+
var result = helper.IsLocalUrl(url);
312+
313+
// Assert
314+
var relaxedLocalRedirectValidation = false;
315+
316+
#if NET451
317+
var value = ConfigurationManager.AppSettings.GetValues(UrlHelper.UseRelaxedLocalRedirectValidationSwitch)?.FirstOrDefault();
318+
var success = bool.TryParse(value, out relaxedLocalRedirectValidation);
319+
#else
320+
var success = AppContext.TryGetSwitch(UrlHelper.UseRelaxedLocalRedirectValidationSwitch, out relaxedLocalRedirectValidation);
321+
#endif
322+
if (relaxedLocalRedirectValidation)
323+
{
324+
Assert.True(result);
325+
}
326+
else
327+
{
328+
Assert.False(result);
329+
}
330+
}
331+
264332
[Fact]
265333
public void RouteUrlWithDictionary()
266334
{

‎test/Microsoft.AspNetCore.Mvc.Core.Test/project.json

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@
3838
"System.Diagnostics.TraceSource": "4.3.0"
3939
}
4040
},
41-
"net451": {}
41+
"net451": {
42+
"frameworkAssemblies": {
43+
"System.Configuration": ""
44+
}
45+
}
4246
}
4347
}

0 commit comments

Comments
 (0)