Fix #6480

This change logs when we encounter and exception reading temp data from a
cookie and swallows the exception. Additionally, we clear the cookie so
that this won't happen on subsequent requests.

This will handle cases where data protection is misconfigured, or a
request just has general garbage in the cookies.

src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/MvcViewFeaturesLoggerExtensions.cs

@@ -30,6 +30,10 @@ internal static class MvcViewFeaturesLoggerExtensions
         private static readonly Action<ILogger, string, Exception> _viewFound;
         private static readonly Action<ILogger, string, IEnumerable<string>, Exception> _viewNotFound;
 
+        private static readonly Action<ILogger, string, Exception> _tempDataCookieNotFound;
+        private static readonly Action<ILogger, string, Exception> _tempDataCookieLoadSuccess;
+        private static readonly Action<ILogger, string, Exception> _tempDataCookieLoadFailure;
+
         static MvcViewFeaturesLoggerExtensions()
         {
             _viewComponentExecuting = LoggerMessage.Define<string, string[]>(
@@ -82,6 +86,21 @@ static MvcViewFeaturesLoggerExtensions()
                 LogLevel.Error,
                 3,
                 "The view '{ViewName}' was not found. Searched locations: {SearchedViewLocations}");
+
+            _tempDataCookieNotFound = LoggerMessage.Define<string>(
+                LogLevel.Debug,
+                1,
+                "The temp data cookie {CookieName} was not found.");
+
+            _tempDataCookieLoadSuccess = LoggerMessage.Define<string>(
+                LogLevel.Debug,
+                2,
+                "The temp data cookie {CookieName} was used to successfully load temp data.");
+
+            _tempDataCookieLoadFailure = LoggerMessage.Define<string>(
+                LogLevel.Warning,
+                3,
+                "The temp data cookie {CookieName} could not be loaded.");
         }
 
         public static IDisposable ViewComponentScope(this ILogger logger, ViewComponentContext context)
@@ -192,6 +211,21 @@ public static void ViewNotFound(this ILogger logger, string viewName,
             _viewNotFound(logger, viewName, searchedLocations, null);
         }
 
+        public static void TempDataCookieNotFound(this ILogger logger, string cookieName)
+        {
+            _tempDataCookieNotFound(logger, cookieName, null);
+        }
+
+        public static void TempDataCookieLoadSuccess(this ILogger logger, string cookieName)
+        {
+            _tempDataCookieLoadSuccess(logger, cookieName, null);
+        }
+
+        public static void TempDataCookieLoadFailure(this ILogger logger, string cookieName, Exception exception)
+        {
+            _tempDataCookieLoadFailure(logger, cookieName, exception);
+        }
+
         private class ViewComponentLogScope : IReadOnlyList<KeyValuePair<string, object>>
         {
             private readonly ViewComponentDescriptor _descriptor;

src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/CookieTempDataProvider.cs

@@ -9,6 +9,7 @@
 using Microsoft.AspNetCore.Internal;
 using Microsoft.AspNetCore.Mvc.ViewFeatures.Internal;
 using Microsoft.Extensions.Options;
+using Microsoft.Extensions.Logging;
 
 namespace Microsoft.AspNetCore.Mvc.ViewFeatures
 {
@@ -19,14 +20,20 @@ public class CookieTempDataProvider : ITempDataProvider
     {
         public static readonly string CookieName = ".AspNetCore.Mvc.CookieTempDataProvider";
         private static readonly string Purpose = "Microsoft.AspNetCore.Mvc.CookieTempDataProviderToken.v1";
+
         private readonly IDataProtector _dataProtector;
+        private readonly ILogger _logger;
         private readonly TempDataSerializer _tempDataSerializer;
         private readonly ChunkingCookieManager _chunkingCookieManager;
         private readonly CookieTempDataProviderOptions _options;
 
-        public CookieTempDataProvider(IDataProtectionProvider dataProtectionProvider, IOptions<CookieTempDataProviderOptions> options)
+        public CookieTempDataProvider(
+            IDataProtectionProvider dataProtectionProvider,
+            ILoggerFactory loggerFactory,
+            IOptions<CookieTempDataProviderOptions> options)
         {
             _dataProtector = dataProtectionProvider.CreateProtector(Purpose);
+            _logger = loggerFactory.CreateLogger<CookieTempDataProvider>();
             _tempDataSerializer = new TempDataSerializer();
             _chunkingCookieManager = new ChunkingCookieManager();
             _options = options.Value;
@@ -41,15 +48,39 @@ public IDictionary<string, object> LoadTempData(HttpContext context)
 
             if (context.Request.Cookies.ContainsKey(_options.Cookie.Name))
             {
-                var encodedValue = _chunkingCookieManager.GetRequestCookie(context, _options.Cookie.Name);
-                if (!string.IsNullOrEmpty(encodedValue))
+                // The cookie we use for temp data is user input, and might be invalid in many ways.
+                //
+                // Since TempData is a best-effort system, we don't want to throw and get a 500 if the cookie is
+                // bad, we will just clear it and ignore the exception. The common case that we've identified for
+                // this is misconfigured data protection settings, which can cause the key used to create the 
+                // cookie to no longer be available.
+                try
                 {
-                    var protectedData = Base64UrlTextEncoder.Decode(encodedValue);
-                    var unprotectedData = _dataProtector.Unprotect(protectedData);
-                    return _tempDataSerializer.Deserialize(unprotectedData);
+                    var encodedValue = _chunkingCookieManager.GetRequestCookie(context, _options.Cookie.Name);
+                    if (!string.IsNullOrEmpty(encodedValue))
+                    {
+                        var protectedData = Base64UrlTextEncoder.Decode(encodedValue);
+                        var unprotectedData = _dataProtector.Unprotect(protectedData);
+                        var tempData = _tempDataSerializer.Deserialize(unprotectedData);
+
+                        _logger.TempDataCookieLoadSuccess(_options.Cookie.Name);
+                        return tempData;
+                    }
+                }
+                catch (Exception ex)
+                {
+                    _logger.TempDataCookieLoadFailure(_options.Cookie.Name, ex);
+
+                    // If we've failed, we want to try and clear the cookie so that this won't keep happening
+                    // over and over.
+                    if (!context.Response.HasStarted)
+                    {
+                        _chunkingCookieManager.DeleteCookie(context, _options.Cookie.Name, _options.Cookie.Build(context));
+                    }
                 }
             }
 
+            _logger.TempDataCookieNotFound(_options.Cookie.Name);
             return new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
         }
 

test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewFeatures/CookieTempDataProviderTest.cs

@@ -9,7 +9,9 @@
 using Microsoft.AspNetCore.Http.Internal;
 using Microsoft.AspNetCore.Mvc.ViewFeatures.Internal;
 using Microsoft.AspNetCore.WebUtilities;
+using Microsoft.Extensions.Logging.Abstractions;
 using Microsoft.Extensions.Options;
+using Microsoft.Net.Http.Headers;
 using Moq;
 using Xunit;
 
@@ -66,6 +68,40 @@ public void LoadTempData_ReturnsEmptyDictionary_WhenNoCookieDataIsAvailable()
             Assert.Empty(tempDataDictionary);
         }
 
+        [Fact]
+        public void LoadTempData_ReturnsEmptyDictionary_AndClearsCookie_WhenDataIsInvalid()
+        {
+            // Arrange
+            var dataProtector = new Mock<IDataProtector>(MockBehavior.Strict);
+            dataProtector
+                .Setup(d => d.Unprotect(It.IsAny<byte[]>()))
+                .Throws(new Exception());
+
+            var tempDataProvider = GetProvider(dataProtector.Object);
+
+            var inputData = new Dictionary<string, object>();
+            inputData.Add("int", 10);
+            var tempDataProviderSerializer = new TempDataSerializer();
+            var expectedDataToUnprotect = tempDataProviderSerializer.Serialize(inputData);
+            var base64AndUrlEncodedDataInCookie = Base64UrlTextEncoder.Encode(expectedDataToUnprotect);
+
+            var context = new DefaultHttpContext();
+            context.Request.Cookies = new RequestCookieCollection(new Dictionary<string, string>()
+            {
+                { CookieTempDataProvider.CookieName, base64AndUrlEncodedDataInCookie }
+            });
+
+            // Act
+            var tempDataDictionary = tempDataProvider.LoadTempData(context);
+
+            // Assert
+            Assert.Empty(tempDataDictionary);
+
+            var setCookieHeader = SetCookieHeaderValue.Parse(context.Response.Headers["Set-Cookie"].ToString());
+            Assert.Equal(CookieTempDataProvider.CookieName, setCookieHeader.Name.ToString());
+            Assert.Equal(string.Empty, setCookieHeader.Value.ToString());
+        }
+
         [Fact]
         public void LoadTempData_Base64UrlDecodesAnd_UnprotectsData_FromCookie()
         {
@@ -219,7 +255,11 @@ public void SaveTempData_DefaultProviderOptions_SetsCookie_WithAppropriateCookie
         [InlineData("/", "/vdir1", ".abc.com", "/vdir1", ".abc.com")]
         [InlineData("/vdir1", "/", ".abc.com", "/", ".abc.com")]
         public void SaveTempData_CustomProviderOptions_SetsCookie_WithAppropriateCookieOptions(
-            string requestPathBase, string optionsPath, string optionsDomain, string expectedCookiePath, string expectedDomain)
+            string requestPathBase,
+            string optionsPath,
+            string optionsDomain,
+            string expectedCookiePath,
+            string expectedDomain)
         {
             // Arrange
             var values = new Dictionary<string, object>();
@@ -637,7 +677,7 @@ private CookieTempDataProvider GetProvider(IDataProtector dataProtector = null,
             var testOptions = new Mock<IOptions<CookieTempDataProviderOptions>>();
             testOptions.SetupGet(o => o.Value).Returns(options);
 
-            return new CookieTempDataProvider(new PassThroughDataProtectionProvider(dataProtector), testOptions.Object);
+            return new CookieTempDataProvider(new PassThroughDataProtectionProvider(dataProtector), NullLoggerFactory.Instance, testOptions.Object);
         }
 
         private class PassThroughDataProtectionProvider : IDataProtectionProvider