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.
This commit is contained in:
parent
e09a0f5b7c
commit
f80f7cefa5
|
|
@ -30,6 +30,10 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
|
|||
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 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
|
|||
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 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
|
|||
_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;
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ using Microsoft.AspNetCore.WebUtilities;
|
|||
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 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
|
|||
{
|
||||
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 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
|
|||
|
||||
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);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -9,7 +9,9 @@ using Microsoft.AspNetCore.Http;
|
|||
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 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
|
|||
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 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
|
|||
[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 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
|
|||
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
|
||||
|
|
|
|||
Loading…
Reference in New Issue