From d5e044f693fee9b782775c5958133e145bfc5ac8 Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Thu, 12 Apr 2018 05:39:58 -0700 Subject: [PATCH] [Fixes #7373] Assigning to the context's Result property, when implementing IPageFilter, causes an exception --- .../MvcSandbox/Views/Shared/_Layout.cshtml | 1 + .../Internal/ResourceInvoker.cs | 2 +- .../Internal/PageActionInvoker.cs | 55 ++-- .../RazorPagesTest.cs | 26 ++ .../Internal/PageActionInvokerTest.cs | 280 +++++++++++++++++- .../Pages/ShortCircuitPageAtAuthFilter.cshtml | 5 + .../ShortCircuitPageAtAuthFilter.cshtml.cs | 51 ++++ .../Pages/ShortCircuitPageAtPageFilter.cshtml | 5 + .../ShortCircuitPageAtPageFilter.cshtml.cs | 67 +++++ 9 files changed, 465 insertions(+), 27 deletions(-) create mode 100644 test/WebSites/RazorPagesWebSite/Pages/ShortCircuitPageAtAuthFilter.cshtml create mode 100644 test/WebSites/RazorPagesWebSite/Pages/ShortCircuitPageAtAuthFilter.cshtml.cs create mode 100644 test/WebSites/RazorPagesWebSite/Pages/ShortCircuitPageAtPageFilter.cshtml create mode 100644 test/WebSites/RazorPagesWebSite/Pages/ShortCircuitPageAtPageFilter.cshtml.cs diff --git a/samples/MvcSandbox/Views/Shared/_Layout.cshtml b/samples/MvcSandbox/Views/Shared/_Layout.cshtml index 77b4265ee2..2308354c26 100644 --- a/samples/MvcSandbox/Views/Shared/_Layout.cshtml +++ b/samples/MvcSandbox/Views/Shared/_Layout.cshtml @@ -21,6 +21,7 @@
diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResourceInvoker.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResourceInvoker.cs index 8da6e6012e..2b73da1c0f 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResourceInvoker.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ResourceInvoker.cs @@ -126,7 +126,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal protected abstract Task InvokeInnerFilterAsync(); - protected async Task InvokeResultAsync(IActionResult result) + protected virtual async Task InvokeResultAsync(IActionResult result) { var actionContext = _actionContext; diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvoker.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvoker.cs index b0a429213d..cd21e58430 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvoker.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvoker.cs @@ -106,6 +106,36 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal } } + protected override Task InvokeResultAsync(IActionResult result) + { + // We also have some special initialization we need to do for PageResult. + if (result is PageResult pageResult) + { + // If we used a PageModel then the Page isn't initialized yet. + if (_viewContext == null) + { + _viewContext = new ViewContext( + _pageContext, + NullView.Instance, + _pageContext.ViewData, + _tempDataFactory.GetTempData(_pageContext.HttpContext), + TextWriter.Null, + _htmlHelperOptions); + _viewContext.ExecutingFilePath = _pageContext.ActionDescriptor.RelativePath; + } + + if (_page == null) + { + _page = (PageBase)CacheEntry.PageFactory(_pageContext, _viewContext); + } + + pageResult.Page = _page; + pageResult.ViewData = pageResult.ViewData ?? _pageContext.ViewData; + } + + return base.InvokeResultAsync(result); + } + private object CreateInstance() { if (HasPageModel) @@ -248,31 +278,6 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal { _result = new PageResult(); } - - // We also have some special initialization we need to do for PageResult. - if (_result is PageResult pageResult) - { - // If we used a PageModel then the Page isn't initialized yet. - if (_viewContext == null) - { - _viewContext = new ViewContext( - _pageContext, - NullView.Instance, - _pageContext.ViewData, - _tempDataFactory.GetTempData(_pageContext.HttpContext), - TextWriter.Null, - _htmlHelperOptions); - _viewContext.ExecutingFilePath = _pageContext.ActionDescriptor.RelativePath; - } - - if (_page == null) - { - _page = (PageBase)CacheEntry.PageFactory(_pageContext, _viewContext); - } - - pageResult.Page = _page; - pageResult.ViewData = pageResult.ViewData ?? _pageContext.ViewData; - } } private Task Next(ref State next, ref Scope scope, ref object state, ref bool isCompleted) diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs index 0f896568d5..1a0cdb4ac7 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs @@ -11,6 +11,8 @@ using System.Net.Http.Headers; using System.Reflection; using System.Threading.Tasks; using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Authorization; +using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Testing; using Xunit; @@ -1269,6 +1271,30 @@ Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary`1[AspNetCore.InjectedPa Assert.Equal(expected, content); } + [Theory] + [InlineData(nameof(IAuthorizationFilter.OnAuthorization))] + [InlineData(nameof(IAsyncAuthorizationFilter.OnAuthorizationAsync))] + public async Task PageResultSetAt_AuthorizationFilter_Works(string targetName) + { + // Act + var content = await Client.GetStringAsync("http://localhost/Pages/ShortCircuitPageAtAuthFilter?target=" + targetName); + + // Assert + Assert.Equal("From ShortCircuitPageAtAuthFilter.cshtml", content); + } + + [Theory] + [InlineData(nameof(IPageFilter.OnPageHandlerExecuting))] + [InlineData(nameof(IAsyncPageFilter.OnPageHandlerExecutionAsync))] + public async Task PageResultSetAt_PageFilter_Works(string targetName) + { + // Act + var content = await Client.GetStringAsync("http://localhost/Pages/ShortCircuitPageAtPageFilter?target=" + targetName); + + // Assert + Assert.Equal("From ShortCircuitPageAtPageFilter.cshtml", content); + } + private async Task AddAntiforgeryHeaders(HttpRequestMessage request) { var getResponse = await Client.GetAsync(request.RequestUri); diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs index 126f03bd4d..f90da88615 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs @@ -432,7 +432,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal .Verifiable(); filter1 .Setup(f => f.OnPageHandlerExecutionAsync(It.IsAny(), It.IsAny())) - .Returns(async(c, next) => + .Returns(async (c, next) => { Assert.Same(handler, c.HandlerMethod); await next(); @@ -529,6 +529,284 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal Assert.Same(Result, result); } + [Fact] + public async Task InvokeAction_PageResultSetAt_AsyncAuthorizeFilter_PopulatesProperties() + { + // Arrange + var expectedResult = new PageResult(); + + IActionResult result = null; + var filter = new Mock(MockBehavior.Strict); + filter + .Setup(f => f.OnAuthorizationAsync(It.IsAny())) + .Returns((context) => + { + context.Result = expectedResult; + result = context.Result; + return Task.CompletedTask; + }) + .Verifiable(); + + var invoker = CreateInvoker(filter.Object); + + // Act + await invoker.InvokeAsync(); + + // Assert + filter.Verify( + f => f.OnAuthorizationAsync(It.IsAny()), + Times.Once()); + + var pageResult = Assert.IsType(result); + Assert.Same(expectedResult, pageResult); + Assert.NotNull(pageResult.Page); + Assert.NotNull(pageResult.ViewData); + Assert.NotNull(pageResult.Page.ViewContext); + } + + [Fact] + public async Task InvokeAction_PageResultSetAt_SyncAuthorizeFilter_PopulatesProperties() + { + // Arrange + var expectedResult = new PageResult(); + + IActionResult result = null; + var filter = new Mock(MockBehavior.Strict); + filter + .Setup(f => f.OnAuthorization(It.IsAny())) + .Callback((context) => + { + context.Result = expectedResult; + result = context.Result; + }) + .Verifiable(); + + var invoker = CreateInvoker(filter.Object); + + // Act + await invoker.InvokeAsync(); + + // Assert + filter.Verify( + f => f.OnAuthorization(It.IsAny()), + Times.Once()); + + var pageResult = Assert.IsType(result); + Assert.Same(expectedResult, pageResult); + Assert.NotNull(pageResult.Page); + Assert.NotNull(pageResult.ViewData); + Assert.NotNull(pageResult.Page.ViewContext); + } + + [Fact] + public async Task InvokeAction_PageResultSetAt_AsyncResourceFilter_PopulatesProperties() + { + // Arrange + var expectedResult = new PageResult(); + + IActionResult result = null; + var filter = new Mock(MockBehavior.Strict); + filter + .Setup(f => f.OnResourceExecutionAsync(It.IsAny(), It.IsAny())) + .Returns((context, next) => + { + context.Result = expectedResult; + result = context.Result; + return Task.CompletedTask; + }) + .Verifiable(); + + var invoker = CreateInvoker(filter.Object); + + // Act + await invoker.InvokeAsync(); + + // Assert + filter.Verify( + f => f.OnResourceExecutionAsync(It.IsAny(), It.IsAny()), + Times.Once()); + + var pageResult = Assert.IsType(result); + Assert.Same(expectedResult, pageResult); + Assert.NotNull(pageResult.Page); + Assert.NotNull(pageResult.ViewData); + Assert.NotNull(pageResult.Page.ViewContext); + } + + [Fact] + public async Task InvokeAction_PageResultSetAt_SyncResourceFilter_PopulatesProperties() + { + // Arrange + var expectedResult = new PageResult(); + + IActionResult result = null; + var filter = new Mock(MockBehavior.Strict); + filter + .Setup(f => f.OnResourceExecuting(It.IsAny())) + .Callback((context) => + { + context.Result = expectedResult; + result = context.Result; + }) + .Verifiable(); + + var invoker = CreateInvoker(filter.Object); + + // Act + await invoker.InvokeAsync(); + + // Assert + filter.Verify( + f => f.OnResourceExecuting(It.IsAny()), + Times.Once()); + + var pageResult = Assert.IsType(result); + Assert.Same(expectedResult, pageResult); + Assert.NotNull(pageResult.Page); + Assert.NotNull(pageResult.ViewData); + Assert.NotNull(pageResult.Page.ViewContext); + } + + [Fact] + public async Task InvokeAction_PageResultSetAt_AsyncResultFilter_PopulatesProperties() + { + // Arrange + var expectedResult = new PageResult(); + + IActionResult result = null; + var filter = new Mock(MockBehavior.Strict); + filter + .Setup(f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny())) + .Returns((context, next) => + { + context.Result = expectedResult; + result = context.Result; + return next(); + }) + .Verifiable(); + + var invoker = CreateInvoker(filter.Object); + + // Act + await invoker.InvokeAsync(); + + // Assert + filter.Verify( + f => f.OnResultExecutionAsync(It.IsAny(), It.IsAny()), + Times.Once()); + + var pageResult = Assert.IsType(result); + Assert.Same(expectedResult, pageResult); + Assert.NotNull(pageResult.Page); + Assert.NotNull(pageResult.ViewData); + Assert.NotNull(pageResult.Page.ViewContext); + } + + [Fact] + public async Task InvokeAction_PageResultSetAt_SyncResultFilter_PopulatesProperties() + { + // Arrange + var expectedResult = new PageResult(); + + IActionResult result = null; + var filter = new Mock(); + filter + .Setup(f => f.OnResultExecuting(It.IsAny())) + .Callback((context) => + { + context.Result = expectedResult; + result = context.Result; + }) + .Verifiable(); + + var invoker = CreateInvoker(filter.Object); + + // Act + await invoker.InvokeAsync(); + + // Assert + filter.Verify( + f => f.OnResultExecuting(It.IsAny()), + Times.Once()); + + var pageResult = Assert.IsType(result); + Assert.Same(expectedResult, pageResult); + Assert.NotNull(pageResult.Page); + Assert.NotNull(pageResult.ViewData); + Assert.NotNull(pageResult.Page.ViewContext); + } + + [Fact] + public async Task InvokeAction_PageResultSetAt_AsyncPageFilter_PopulatesProperties() + { + // Arrange + var expectedResult = new PageResult(); + + IActionResult result = null; + var filter = new Mock(MockBehavior.Strict); + AllowSelector(filter); + filter + .Setup(f => f.OnPageHandlerExecutionAsync(It.IsAny(), It.IsAny())) + .Returns((context, next) => + { + context.Result = expectedResult; + result = context.Result; + return Task.CompletedTask; + }) + .Verifiable(); + + var invoker = CreateInvoker(filter.Object); + + // Act + await invoker.InvokeAsync(); + + // Assert + filter.Verify( + f => f.OnPageHandlerExecutionAsync(It.IsAny(), It.IsAny()), + Times.Once()); + + var pageResult = Assert.IsType(result); + Assert.Same(expectedResult, pageResult); + Assert.NotNull(pageResult.Page); + Assert.NotNull(pageResult.ViewData); + Assert.NotNull(pageResult.Page.ViewContext); + } + + [Fact] + public async Task InvokeAction_PageResultSetAt_SyncPageFilter_PopulatesProperties() + { + // Arrange + var expectedResult = new PageResult(); + + IActionResult result = null; + var filter = new Mock(MockBehavior.Strict); + AllowSelector(filter); + filter + .Setup(f => f.OnPageHandlerExecuting(It.IsAny())) + .Callback((context) => + { + context.Result = expectedResult; + result = context.Result; + }) + .Verifiable(); + + var invoker = CreateInvoker(filter.Object); + + // Act + await invoker.InvokeAsync(); + + // Assert + filter.Verify( + f => f.OnPageHandlerExecuting(It.IsAny()), + Times.Once()); + + var pageResult = Assert.IsType(result); + Assert.Same(expectedResult, pageResult); + Assert.NotNull(pageResult.Page); + Assert.NotNull(pageResult.ViewData); + Assert.NotNull(pageResult.Page.ViewContext); + } + [Fact] public async Task InvokeAction_InvokesPageFilter_ShortCircuit() { diff --git a/test/WebSites/RazorPagesWebSite/Pages/ShortCircuitPageAtAuthFilter.cshtml b/test/WebSites/RazorPagesWebSite/Pages/ShortCircuitPageAtAuthFilter.cshtml new file mode 100644 index 0000000000..ca22d58eec --- /dev/null +++ b/test/WebSites/RazorPagesWebSite/Pages/ShortCircuitPageAtAuthFilter.cshtml @@ -0,0 +1,5 @@ +@page +@model ShortCircuitAtAuthFilterPageModel +@{ +} +From ShortCircuitPageAtAuthFilter.cshtml \ No newline at end of file diff --git a/test/WebSites/RazorPagesWebSite/Pages/ShortCircuitPageAtAuthFilter.cshtml.cs b/test/WebSites/RazorPagesWebSite/Pages/ShortCircuitPageAtAuthFilter.cshtml.cs new file mode 100644 index 0000000000..174ad98b1d --- /dev/null +++ b/test/WebSites/RazorPagesWebSite/Pages/ShortCircuitPageAtAuthFilter.cshtml.cs @@ -0,0 +1,51 @@ +// 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 System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Mvc.RazorPages; + +namespace RazorPagesWebSite.Pages +{ + [AsyncTestAuthorizationFilter] + [SyncTestAuthorizationFilter] + public class ShortCircuitAtAuthFilterPageModel : PageModel + { + public IActionResult OnGet() + { + return Page(); + } + + private static bool ShouldShortCircuit(HttpContext httpContext, string currentTargetName) + { + return httpContext.Request.Query.TryGetValue("target", out var expectedTargetName) + && string.Equals(expectedTargetName, currentTargetName, StringComparison.OrdinalIgnoreCase); + } + + private class AsyncTestAuthorizationFilterAttribute : Attribute, IAsyncAuthorizationFilter + { + public Task OnAuthorizationAsync(AuthorizationFilterContext context) + { + if (ShouldShortCircuit(context.HttpContext, nameof(OnAuthorizationAsync))) + { + context.Result = new PageResult(); + } + return Task.CompletedTask; + } + } + + private class SyncTestAuthorizationFilterAttribute : Attribute, IAuthorizationFilter + { + public void OnAuthorization(AuthorizationFilterContext context) + { + if (ShouldShortCircuit(context.HttpContext, nameof(OnAuthorization))) + { + context.Result = new PageResult(); + } + } + } + } +} diff --git a/test/WebSites/RazorPagesWebSite/Pages/ShortCircuitPageAtPageFilter.cshtml b/test/WebSites/RazorPagesWebSite/Pages/ShortCircuitPageAtPageFilter.cshtml new file mode 100644 index 0000000000..f824be566a --- /dev/null +++ b/test/WebSites/RazorPagesWebSite/Pages/ShortCircuitPageAtPageFilter.cshtml @@ -0,0 +1,5 @@ +@page +@model ShortCircuitAtPageFilterPageModel +@{ +} +From ShortCircuitPageAtPageFilter.cshtml \ No newline at end of file diff --git a/test/WebSites/RazorPagesWebSite/Pages/ShortCircuitPageAtPageFilter.cshtml.cs b/test/WebSites/RazorPagesWebSite/Pages/ShortCircuitPageAtPageFilter.cshtml.cs new file mode 100644 index 0000000000..58d9f62766 --- /dev/null +++ b/test/WebSites/RazorPagesWebSite/Pages/ShortCircuitPageAtPageFilter.cshtml.cs @@ -0,0 +1,67 @@ +// 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 System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Mvc.RazorPages; + +namespace RazorPagesWebSite.Pages +{ + [AsyncTestPageFilter] + [SyncTestPageFilter] + public class ShortCircuitAtPageFilterPageModel : PageModel + { + public IActionResult OnGet() + { + return Page(); + } + + private static bool ShouldShortCircuit(HttpContext httpContext, string currentTargetName) + { + return httpContext.Request.Query.TryGetValue("target", out var expectedTargetName) + && string.Equals(expectedTargetName, currentTargetName, StringComparison.OrdinalIgnoreCase); + } + + private class AsyncTestPageFilterAttribute : Attribute, IAsyncPageFilter + { + public Task OnPageHandlerSelectionAsync(PageHandlerSelectedContext context) + { + return Task.CompletedTask; + } + + public Task OnPageHandlerExecutionAsync( + PageHandlerExecutingContext context, + PageHandlerExecutionDelegate next) + { + if (ShouldShortCircuit(context.HttpContext, nameof(OnPageHandlerExecutionAsync))) + { + context.Result = new PageResult(); + return Task.CompletedTask; + } + return next(); + } + } + + private class SyncTestPageFilterAttribute : Attribute, IPageFilter + { + public void OnPageHandlerSelected(PageHandlerSelectedContext context) + { + } + + public void OnPageHandlerExecuting(PageHandlerExecutingContext context) + { + if (ShouldShortCircuit(context.HttpContext, nameof(OnPageHandlerExecuting))) + { + context.Result = new PageResult(); + } + } + + public void OnPageHandlerExecuted(PageHandlerExecutedContext context) + { + } + } + } +}