diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/SaveTempDataFilter.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/SaveTempDataFilter.cs
index eb4653484d..2ecf4cbbef 100644
--- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/SaveTempDataFilter.cs
+++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/SaveTempDataFilter.cs
@@ -1,6 +1,7 @@
// 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.Collections.Generic;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
@@ -14,7 +15,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
public class SaveTempDataFilter : IResourceFilter, IResultFilter
{
// Internal for unit testing
- internal static readonly object TempDataSavedKey = new object();
+ internal static readonly object SaveTempDataFilterContextKey = new object();
private readonly ITempDataDictionaryFactory _factory;
@@ -30,18 +31,34 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
///
public void OnResourceExecuting(ResourceExecutingContext context)
{
+ if (!context.HttpContext.Items.ContainsKey(SaveTempDataFilterContextKey))
+ {
+ var tempDataContext = new SaveTempDataContext()
+ {
+ Filters = context.Filters,
+ TempDataDictionaryFactory = _factory
+ };
+ context.HttpContext.Items.Add(SaveTempDataFilterContextKey, tempDataContext);
+ }
+
if (!context.HttpContext.Response.HasStarted)
{
context.HttpContext.Response.OnStarting((state) =>
{
- var saveTempDataContext = (SaveTempDataContext)state;
+ var httpContext = (HttpContext)state;
+
+ var saveTempDataContext = GetTempDataContext(context.HttpContext);
+ if (saveTempDataContext.RequestHasUnhandledException)
+ {
+ return Task.CompletedTask;
+ }
// If temp data was already saved, skip trying to save again as the calls here would potentially fail
// because the session feature might not be available at this point.
// Example: An action returns NoContentResult and since NoContentResult does not write anything to
// the body of the response, this delegate would get executed way late in the pipeline at which point
// the session feature would have been removed.
- if (saveTempDataContext.HttpContext.Items.TryGetValue(TempDataSavedKey, out var obj))
+ if (saveTempDataContext.TempDataSaved)
{
return Task.CompletedTask;
}
@@ -50,22 +67,29 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
result: null,
factory: saveTempDataContext.TempDataDictionaryFactory,
filters: saveTempDataContext.Filters,
- httpContext: saveTempDataContext.HttpContext);
+ httpContext: httpContext);
return Task.CompletedTask;
},
- state: new SaveTempDataContext()
- {
- Filters = context.Filters,
- HttpContext = context.HttpContext,
- TempDataDictionaryFactory = _factory
- });
+ state: context.HttpContext);
}
}
///
public void OnResourceExecuted(ResourceExecutedContext context)
{
+ // If there is an unhandled exception, we would like to avoid setting tempdata as
+ // the end user is going to see an error page anyway and also it helps us in avoiding
+ // accessing resources like Session too late in the request lifecyle where SessionFeature might
+ // not be available.
+ if (!context.HttpContext.Response.HasStarted && context.Exception != null)
+ {
+ var saveTempDataContext = GetTempDataContext(context.HttpContext);
+ if (saveTempDataContext != null)
+ {
+ saveTempDataContext.RequestHasUnhandledException = true;
+ }
+ }
}
///
@@ -82,14 +106,25 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
if (!context.HttpContext.Response.HasStarted)
{
SaveTempData(context.Result, _factory, context.Filters, context.HttpContext);
- // If SaveTempDataFilter got added twice this might already be in there.
- if (!context.HttpContext.Items.ContainsKey(TempDataSavedKey))
+
+ var saveTempDataContext = GetTempDataContext(context.HttpContext);
+ if (saveTempDataContext != null)
{
- context.HttpContext.Items.Add(TempDataSavedKey, true);
+ saveTempDataContext.TempDataSaved = true;
}
}
}
+ private SaveTempDataContext GetTempDataContext(HttpContext httpContext)
+ {
+ SaveTempDataContext saveTempDataContext = null;
+ if (httpContext.Items.TryGetValue(SaveTempDataFilterContextKey, out var value))
+ {
+ saveTempDataContext = (SaveTempDataContext)value;
+ }
+ return saveTempDataContext;
+ }
+
private static void SaveTempData(
IActionResult result,
ITempDataDictionaryFactory factory,
@@ -114,10 +149,11 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
tempData.Save();
}
- private class SaveTempDataContext
+ internal class SaveTempDataContext
{
+ public bool RequestHasUnhandledException { get; set; }
+ public bool TempDataSaved { get; set; }
public IList Filters { get; set; }
- public HttpContext HttpContext { get; set; }
public ITempDataDictionaryFactory TempDataDictionaryFactory { get; set; }
}
}
diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TempDataTestBase.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TempDataTestBase.cs
index ab9c284483..c370123994 100644
--- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TempDataTestBase.cs
+++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TempDataTestBase.cs
@@ -207,6 +207,42 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
Assert.Equal(HttpStatusCode.NoContent, response.StatusCode);
}
+ [Fact]
+ public async Task SaveTempDataFilter_DoesNotSaveTempData_OnUnhandledException()
+ {
+ // Arrange & Act
+ var response = await Client.GetAsync("/TempData/UnhandledExceptionAndSettingTempData");
+
+ // Assert
+ Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode);
+ var responseBody = await response.Content.ReadAsStringAsync();
+ Assert.Contains("Exception from action UnhandledExceptionAndSettingTempData", responseBody);
+
+ // Arrange & Act
+ response = await Client.GetAsync("/TempData/UnhandledExceptionAndGetTempData");
+
+ // Assert
+ Assert.Equal(HttpStatusCode.NoContent, response.StatusCode);
+ }
+
+ [Fact]
+ public async Task SaveTempDataFilter_DoesNotSaveTempData_OnHandledExceptions()
+ {
+ // Arrange & Act
+ var response = await Client.GetAsync("/TempData/UnhandledExceptionAndSettingTempData?handleException=true");
+
+ // Assert
+ Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+ var responseBody = await response.Content.ReadAsStringAsync();
+ Assert.Contains("Exception was handled in TestExceptionFilter", responseBody);
+
+ // Arrange & Act
+ response = await Client.GetAsync("/TempData/UnhandledExceptionAndGetTempData");
+
+ // Assert
+ Assert.Equal(HttpStatusCode.NoContent, response.StatusCode);
+ }
+
public HttpRequestMessage GetRequest(string path, HttpResponseMessage response)
{
var request = new HttpRequestMessage(HttpMethod.Get, path);
diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/SaveTempDataFilterTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/SaveTempDataFilterTest.cs
index 00d4b86db3..9ccf374554 100644
--- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/SaveTempDataFilterTest.cs
+++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/SaveTempDataFilterTest.cs
@@ -102,7 +102,29 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
// Arrange
var responseFeature = new TestResponseFeature();
var httpContext = GetHttpContext(responseFeature);
- httpContext.Items[SaveTempDataFilter.TempDataSavedKey] = true; // indicate that tempdata was already saved
+ httpContext.Items[SaveTempDataFilter.SaveTempDataFilterContextKey] = new SaveTempDataFilter.SaveTempDataContext() { TempDataSaved = true };
+ var tempDataFactory = new Mock(MockBehavior.Strict);
+ tempDataFactory
+ .Setup(f => f.GetTempData(It.IsAny()))
+ .Verifiable();
+ var filter = new SaveTempDataFilter(tempDataFactory.Object);
+ var context = GetResourceExecutingContext(httpContext);
+ filter.OnResourceExecuting(context); // registers callback
+
+ // Act
+ await responseFeature.FireOnSendingHeadersAsync();
+
+ // Assert
+ tempDataFactory.Verify(tdf => tdf.GetTempData(It.IsAny()), Times.Never());
+ }
+
+ [Fact]
+ public async Task OnResourceExecuting_DoesNotSaveTempData_WhenUnhandledExceptionOccurs()
+ {
+ // Arrange
+ var responseFeature = new TestResponseFeature();
+ var httpContext = GetHttpContext(responseFeature);
+ httpContext.Items[SaveTempDataFilter.SaveTempDataFilterContextKey] = new SaveTempDataFilter.SaveTempDataContext() { RequestHasUnhandledException = true };
var tempDataFactory = new Mock(MockBehavior.Strict);
tempDataFactory
.Setup(f => f.GetTempData(It.IsAny()))
diff --git a/test/WebSites/BasicWebSite/BasicWebSite.csproj b/test/WebSites/BasicWebSite/BasicWebSite.csproj
index c5fd68a314..363c11aefe 100644
--- a/test/WebSites/BasicWebSite/BasicWebSite.csproj
+++ b/test/WebSites/BasicWebSite/BasicWebSite.csproj
@@ -13,5 +13,6 @@
+
diff --git a/test/WebSites/BasicWebSite/Controllers/TempDataController.cs b/test/WebSites/BasicWebSite/Controllers/TempDataController.cs
index 1ad0d7a732..a0fedabe02 100644
--- a/test/WebSites/BasicWebSite/Controllers/TempDataController.cs
+++ b/test/WebSites/BasicWebSite/Controllers/TempDataController.cs
@@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Threading.Tasks;
+using BasicWebSite.Filters;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
@@ -102,5 +103,19 @@ namespace BasicWebSite.Controllers
{
return TempData["LargeValue"]?.ToString();
}
+
+ [HttpGet]
+ [TestExceptionFilter]
+ public IActionResult UnhandledExceptionAndSettingTempData()
+ {
+ TempData[nameof(UnhandledExceptionAndSettingTempData)] = "James";
+ throw new InvalidOperationException($"Exception from action {nameof(UnhandledExceptionAndSettingTempData)}");
+ }
+
+ [HttpGet]
+ public string UnhandledExceptionAndGetTempData()
+ {
+ return TempData[nameof(UnhandledExceptionAndSettingTempData)]?.ToString();
+ }
}
}
diff --git a/test/WebSites/BasicWebSite/Filters/TestExceptionFilter.cs b/test/WebSites/BasicWebSite/Filters/TestExceptionFilter.cs
new file mode 100644
index 0000000000..592d36dcd2
--- /dev/null
+++ b/test/WebSites/BasicWebSite/Filters/TestExceptionFilter.cs
@@ -0,0 +1,23 @@
+// 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 Microsoft.AspNetCore.Mvc;
+using Microsoft.AspNetCore.Mvc.Filters;
+
+namespace BasicWebSite.Filters
+{
+ public class TestExceptionFilter : ExceptionFilterAttribute
+ {
+ public override void OnException(ExceptionContext context)
+ {
+ if (context.HttpContext.Request.Query.TryGetValue("handleException", out var handleException))
+ {
+ if (handleException.Equals("true"))
+ {
+ context.Result = new ContentResult() { Content = "Exception was handled in TestExceptionFilter", StatusCode = 200 };
+ context.ExceptionHandled = true;
+ }
+ }
+ }
+ }
+}
diff --git a/test/WebSites/BasicWebSite/Startup.cs b/test/WebSites/BasicWebSite/Startup.cs
index 9cc9d295eb..158b3604c1 100644
--- a/test/WebSites/BasicWebSite/Startup.cs
+++ b/test/WebSites/BasicWebSite/Startup.cs
@@ -28,6 +28,8 @@ namespace BasicWebSite
public void Configure(IApplicationBuilder app)
{
+ app.UseDeveloperExceptionPage();
+
app.UseStaticFiles();
// Initializes the RequestId service for each request
diff --git a/test/WebSites/BasicWebSite/StartupWithSessionTempDataProvider.cs b/test/WebSites/BasicWebSite/StartupWithSessionTempDataProvider.cs
index 6500601f0d..cc13dd6695 100644
--- a/test/WebSites/BasicWebSite/StartupWithSessionTempDataProvider.cs
+++ b/test/WebSites/BasicWebSite/StartupWithSessionTempDataProvider.cs
@@ -19,6 +19,7 @@ namespace BasicWebSite
public void Configure(IApplicationBuilder app)
{
+ app.UseDeveloperExceptionPage();
app.UseSession();
app.UseMvcWithDefaultRoute();
}