From eb398c811dbe43c23032b66fd7432b895181bdca Mon Sep 17 00:00:00 2001 From: Ajay Bhargav Baaskaran Date: Wed, 21 Oct 2015 13:14:47 -0700 Subject: [PATCH] Added LocalRedirectresult - Fixes #3346 - Added helper method in controller - Added relevant tests --- src/Microsoft.AspNet.Mvc.Core/IUrlHelper.cs | 4 +- .../LocalRedirectResult.cs | 104 ++++++++++++++ .../LocalRedirectResultLoggerExtensions.cs | 26 ++++ .../Properties/Resources.Designer.cs | 37 ++++- src/Microsoft.AspNet.Mvc.Core/Resources.resx | 3 + .../Controller.cs | 34 +++++ .../LocalRedirectResultTest.cs | 133 ++++++++++++++++++ .../ControllerTest.cs | 45 ++++++ 8 files changed, 379 insertions(+), 7 deletions(-) create mode 100644 src/Microsoft.AspNet.Mvc.Core/LocalRedirectResult.cs create mode 100644 src/Microsoft.AspNet.Mvc.Core/Logging/LocalRedirectResultLoggerExtensions.cs create mode 100644 test/Microsoft.AspNet.Mvc.Core.Test/LocalRedirectResultTest.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/IUrlHelper.cs b/src/Microsoft.AspNet.Mvc.Core/IUrlHelper.cs index a85f2404b2..bb45249355 100644 --- a/src/Microsoft.AspNet.Mvc.Core/IUrlHelper.cs +++ b/src/Microsoft.AspNet.Mvc.Core/IUrlHelper.cs @@ -30,8 +30,8 @@ public interface IUrlHelper string Content(string contentPath); /// - /// Returns a value that indicates whether the URL is local. An URL with an absolute path is considered local - /// if it does not have a host/authority part. URLs using the virtual paths ('~/') are also local. + /// Returns a value that indicates whether the URL is 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. /// /// The URL. /// true if the URL is local; otherwise, false. diff --git a/src/Microsoft.AspNet.Mvc.Core/LocalRedirectResult.cs b/src/Microsoft.AspNet.Mvc.Core/LocalRedirectResult.cs new file mode 100644 index 0000000000..1bed45e0ac --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/LocalRedirectResult.cs @@ -0,0 +1,104 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.AspNet.Mvc.Core; +using Microsoft.AspNet.Mvc.Logging; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNet.Mvc +{ + /// + /// An that returns a redirect to the supplied local URL. + /// + public class LocalRedirectResult : ActionResult + { + private string _localUrl; + + /// + /// Initializes a new instance of the class with the values + /// provided. + /// + /// The local URL to redirect to. + public LocalRedirectResult(string localUrl) + : this(localUrl, permanent: false) + { + } + + /// + /// Initializes a new instance of the class with the values + /// provided. + /// + /// The local URL to redirect to. + /// Specifies whether the redirect should be permanent (301) or temporary (302). + public LocalRedirectResult(string localUrl, bool permanent) + { + if (string.IsNullOrEmpty(localUrl)) + { + throw new ArgumentException(Resources.ArgumentCannotBeNullOrEmpty, nameof(localUrl)); + } + + Permanent = permanent; + Url = localUrl; + } + + /// + /// Gets or sets the value that specifies that the redirect should be permanent if true or temporary if false. + /// + public bool Permanent { get; set; } + + /// + /// Gets or sets the local URL to redirect to. + /// + public string Url + { + get + { + return _localUrl; + } + set + { + if (string.IsNullOrEmpty(value)) + { + throw new ArgumentException(Resources.ArgumentCannotBeNullOrEmpty, nameof(value)); + } + + _localUrl = value; + } + } + + /// + /// Gets or sets the for this result. + /// + public IUrlHelper UrlHelper { get; set; } + + /// + public override void ExecuteResult(ActionContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + var loggerFactory = context.HttpContext.RequestServices.GetRequiredService(); + var logger = loggerFactory.CreateLogger(); + + var urlHelper = GetUrlHelper(context); + + if (!urlHelper.IsLocalUrl(Url)) + { + throw new InvalidOperationException(Resources.UrlNotLocal); + } + + var destinationUrl = urlHelper.Content(Url); + logger.LocalRedirectResultExecuting(destinationUrl); + context.HttpContext.Response.Redirect(destinationUrl, Permanent); + } + + private IUrlHelper GetUrlHelper(ActionContext context) + { + return UrlHelper ?? context.HttpContext.RequestServices.GetRequiredService(); + } + } +} diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/LocalRedirectResultLoggerExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/LocalRedirectResultLoggerExtensions.cs new file mode 100644 index 0000000000..8764b6d354 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/LocalRedirectResultLoggerExtensions.cs @@ -0,0 +1,26 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNet.Mvc.Logging +{ + public static class LocalRedirectResultLoggerExtensions + { + private static Action _localRedirectResultExecuting; + + static LocalRedirectResultLoggerExtensions() + { + _localRedirectResultExecuting = LoggerMessage.Define( + LogLevel.Information, + 1, + "Executing LocalRedirectResult, redirecting to {Destination}."); + } + + public static void LocalRedirectResultExecuting(this ILogger logger, string destination) + { + _localRedirectResultExecuting(logger, destination, null); + } + } +} diff --git a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs index 1621734ba2..e62d6a880e 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs @@ -650,11 +650,6 @@ internal static string FormatFileResult_InvalidPath(object p0) return string.Format(CultureInfo.CurrentCulture, GetString("FileResult_InvalidPath"), p0); } - internal static string FormatFileResult_PathNotRooted(object p0) - { - return string.Format(CultureInfo.CurrentCulture, GetString("FileResult_PathNotRooted"), p0); - } - /// /// The input was not valid. /// @@ -1023,6 +1018,38 @@ internal static string FormatHttpResponseStreamWriter_InvalidBufferSize(object p return string.Format(CultureInfo.CurrentCulture, GetString("HttpResponseStreamWriter_InvalidBufferSize"), p0, p1, p2, p3, p4); } + /// + /// Path '{0}' was not rooted. + /// + internal static string FileResult_PathNotRooted + { + get { return GetString("FileResult_PathNotRooted"); } + } + + /// + /// Path '{0}' was not rooted. + /// + internal static string FormatFileResult_PathNotRooted(object p0) + { + return string.Format(CultureInfo.CurrentCulture, GetString("FileResult_PathNotRooted"), p0); + } + + /// + /// 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. + /// + internal static string UrlNotLocal + { + get { return GetString("UrlNotLocal"); } + } + + /// + /// 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. + /// + internal static string FormatUrlNotLocal() + { + return GetString("UrlNotLocal"); + } + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.AspNet.Mvc.Core/Resources.resx b/src/Microsoft.AspNet.Mvc.Core/Resources.resx index 28379ba92b..d57c5ed85d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.Core/Resources.resx @@ -319,4 +319,7 @@ Path '{0}' was not rooted. {0} is the path which wasn't rooted + + 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. + \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/Controller.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/Controller.cs index c15a738f44..5f610a30dc 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/Controller.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/Controller.cs @@ -613,6 +613,40 @@ public virtual RedirectResult RedirectPermanent(string url) return new RedirectResult(url, permanent: true); } + /// + /// Creates a object that redirects to + /// the specified local . + /// + /// The local URL to redirect to. + /// The created for the response. + [NonAction] + public virtual LocalRedirectResult LocalRedirect(string localUrl) + { + if (string.IsNullOrEmpty(localUrl)) + { + throw new ArgumentException(Resources.ArgumentCannotBeNullOrEmpty, nameof(localUrl)); + } + + return new LocalRedirectResult(localUrl); + } + + /// + /// Creates a object with + /// set to true using the specified . + /// + /// The local URL to redirect to. + /// The created for the response. + [NonAction] + public virtual LocalRedirectResult LocalRedirectPermanent(string localUrl) + { + if (string.IsNullOrEmpty(localUrl)) + { + throw new ArgumentException(Resources.ArgumentCannotBeNullOrEmpty, nameof(localUrl)); + } + + return new LocalRedirectResult(localUrl, permanent: true); + } + /// /// Redirects to the specified action using the . /// diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/LocalRedirectResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/LocalRedirectResultTest.cs new file mode 100644 index 0000000000..d2933bc972 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/LocalRedirectResultTest.cs @@ -0,0 +1,133 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.AspNet.Http; +using Microsoft.AspNet.Mvc.Abstractions; +using Microsoft.AspNet.Mvc.Infrastructure; +using Microsoft.AspNet.Mvc.Routing; +using Microsoft.AspNet.Routing; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Moq; +using Xunit; + +namespace Microsoft.AspNet.Mvc +{ + public class LocalRedirectResultTest + { + [Fact] + public void Constructor_WithParameterUrl_SetsResultUrlAndNotPermanent() + { + // Arrange + var url = "/test/url"; + + // Act + var result = new LocalRedirectResult(url); + + // Assert + Assert.False(result.Permanent); + Assert.Same(url, result.Url); + } + + [Fact] + public void Constructor_WithParameterUrlAndPermanent_SetsResultUrlAndPermanent() + { + // Arrange + var url = "/test/url"; + + // Act + var result = new LocalRedirectResult(url, permanent: true); + + // Assert + Assert.True(result.Permanent); + Assert.Same(url, result.Url); + } + + [Fact] + public void Execute_ReturnsExpectedValues() + { + // Arrange + var appRoot = "/"; + var contentPath = "~/Home/About"; + var expectedPath = "/Home/About"; + var httpResponse = new Mock(); + 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); + + // Act + result.ExecuteResult(actionContext); + + // Assert + httpResponse.Verify(); + } + + [Theory] + [InlineData("", "Home/About", "/Home/About")] + [InlineData("/myapproot", "http://www.example.com", "/test")] + public void Execute_Throws_ForNonLocalUrl( + string appRoot, + string contentPath, + string expectedPath) + { + // Arrange + var httpResponse = new Mock(); + 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); + + // Act & Assert + var exception = Assert.Throws(() => 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(); + routeData.Routers.Add(new Mock().Object); + + return new ActionContext(httpContext, routeData, new ActionDescriptor()); + } + + private static IServiceProvider GetServiceProvider(IUrlHelper urlHelper) + { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddInstance(urlHelper); + serviceCollection.AddTransient(); + return serviceCollection.BuildServiceProvider(); + } + + private static HttpContext GetHttpContext( + string appRoot, + string contentPath, + string expectedPath, + HttpResponse response) + { + var httpContext = new Mock(); + var actionContext = GetActionContext(httpContext.Object); + var actionContextAccessor = new ActionContextAccessor() { ActionContext = actionContext }; + var mockActionSelector = new Mock(); + var urlHelper = new UrlHelper(actionContextAccessor, mockActionSelector.Object); + var serviceProvider = GetServiceProvider(urlHelper); + + httpContext.Setup(o => o.Response) + .Returns(response); + httpContext.SetupGet(o => o.RequestServices) + .Returns(serviceProvider); + httpContext.Setup(o => o.Request.PathBase) + .Returns(new PathString(appRoot)); + + return httpContext.Object; + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ControllerTest.cs b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ControllerTest.cs index 3a5209547a..f509bf50ec 100644 --- a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ControllerTest.cs +++ b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ControllerTest.cs @@ -107,6 +107,51 @@ public void Redirect_WithParameter_NullOrEmptyUrl_Throws(string url) () => controller.Redirect(url: url), "url"); } + [Fact] + public void LocalRedirect_WithParameterUrl_SetsLocalRedirectResultWithSameUrl() + { + // Arrange + var controller = new TestableController(); + var url = "/test/url"; + + // Act + var result = controller.LocalRedirect(url); + + // Assert + Assert.IsType(result); + Assert.False(result.Permanent); + Assert.Same(url, result.Url); + } + + [Fact] + public void LocalRedirectPermanent_WithParameterUrl_SetsLocalRedirectResultPermanentWithSameUrl() + { + // Arrange + var controller = new TestableController(); + var url = "/test/url"; + + // Act + var result = controller.LocalRedirectPermanent(url); + + // Assert + Assert.IsType(result); + Assert.True(result.Permanent); + Assert.Same(url, result.Url); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + public void LocalRedirect_WithParameter_NullOrEmptyUrl_Throws(string url) + { + // Arrange + var controller = new TestableController(); + + // Act & Assert + ExceptionAssert.ThrowsArgumentNullOrEmpty( + () => controller.LocalRedirect(localUrl: url), "localUrl"); + } + [Theory] [InlineData(null)] [InlineData("")]