diff --git a/src/Mvc/Mvc.ViewFeatures/test/CookieTempDataProviderTest.cs b/src/Mvc/Mvc.ViewFeatures/test/CookieTempDataProviderTest.cs index 2cbb4e8902b6..b7b3e8d95612 100644 --- a/src/Mvc/Mvc.ViewFeatures/test/CookieTempDataProviderTest.cs +++ b/src/Mvc/Mvc.ViewFeatures/test/CookieTempDataProviderTest.cs @@ -1,10 +1,11 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Collections; using System.Text; using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc.ViewFeatures.Infrastructure; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Logging.Abstractions; @@ -35,16 +36,12 @@ public void SaveTempData_UsesCookieName_FromOptions() }); var responseCookies = new MockResponseCookieCollection(); - var httpContext = new Mock(); - httpContext - .SetupGet(hc => hc.Request.PathBase) - .Returns("/"); - httpContext - .Setup(hc => hc.Response.Cookies) - .Returns(responseCookies); + var httpContext = new DefaultHttpContext(); + httpContext.Request.PathBase = "/"; + httpContext.Features.Set(new FakeResponseCookiesFeature(responseCookies)); // Act - tempDataProvider.SaveTempData(httpContext.Object, Dictionary); + tempDataProvider.SaveTempData(httpContext, Dictionary); // Assert Assert.Contains(responseCookies, (cookie) => cookie.Key == expectedCookieName); @@ -125,16 +122,12 @@ public void SaveTempData_ProtectsAnd_Base64UrlEncodesDataAnd_SetsCookie() var dataProtector = new PassThroughDataProtector(); var tempDataProvider = GetProvider(dataProtector); var responseCookies = new MockResponseCookieCollection(); - var httpContext = new Mock(); - httpContext - .SetupGet(hc => hc.Request.PathBase) - .Returns("/"); - httpContext - .Setup(hc => hc.Response.Cookies) - .Returns(responseCookies); + var httpContext = new DefaultHttpContext(); + httpContext.Request.PathBase = "/"; + httpContext.Features.Set(new FakeResponseCookiesFeature(responseCookies)); // Act - tempDataProvider.SaveTempData(httpContext.Object, Dictionary); + tempDataProvider.SaveTempData(httpContext, Dictionary); // Assert Assert.Equal(1, responseCookies.Count); @@ -164,19 +157,13 @@ public void SaveTempData_HonorsCookieSecurePolicy_OnOptions( options.Cookie.SecurePolicy = cookieSecurePolicy; var tempDataProvider = GetProvider(dataProtector, options); var responseCookies = new MockResponseCookieCollection(); - var httpContext = new Mock(); - httpContext - .SetupGet(hc => hc.Request.PathBase) - .Returns("/"); - httpContext - .SetupGet(hc => hc.Request.IsHttps) - .Returns(isRequestSecure); - httpContext - .Setup(hc => hc.Response.Cookies) - .Returns(responseCookies); + var httpContext = new DefaultHttpContext(); + httpContext.Request.PathBase = "/"; + httpContext.Features.Set(new FakeResponseCookiesFeature(responseCookies)); + httpContext.Request.IsHttps = isRequestSecure; // Act - tempDataProvider.SaveTempData(httpContext.Object, Dictionary); + tempDataProvider.SaveTempData(httpContext, Dictionary); // Assert Assert.Equal(1, responseCookies.Count); @@ -206,19 +193,12 @@ public void SaveTempData_DefaultProviderOptions_SetsCookie_WithAppropriateCookie var dataProtector = new PassThroughDataProtector(); var tempDataProvider = GetProvider(dataProtector); var responseCookies = new MockResponseCookieCollection(); - var httpContext = new Mock(); - httpContext - .SetupGet(hc => hc.Request.PathBase) - .Returns(pathBase); - httpContext - .SetupGet(hc => hc.Request.IsHttps) - .Returns(false); - httpContext - .Setup(hc => hc.Response.Cookies) - .Returns(responseCookies); + var httpContext = new DefaultHttpContext(); + httpContext.Request.PathBase = pathBase; + httpContext.Features.Set(new FakeResponseCookiesFeature(responseCookies)); // Act - tempDataProvider.SaveTempData(httpContext.Object, Dictionary); + tempDataProvider.SaveTempData(httpContext, Dictionary); // Assert Assert.Equal(1, responseCookies.Count); @@ -262,19 +242,12 @@ public void SaveTempData_CustomProviderOptions_SetsCookie_WithAppropriateCookieO } }); var responseCookies = new MockResponseCookieCollection(); - var httpContext = new Mock(); - httpContext - .SetupGet(hc => hc.Request.IsHttps) - .Returns(false); - httpContext - .SetupGet(hc => hc.Request.PathBase) - .Returns(requestPathBase); - httpContext - .Setup(hc => hc.Response.Cookies) - .Returns(responseCookies); + var httpContext = new DefaultHttpContext(); + httpContext.Request.PathBase = requestPathBase; + httpContext.Features.Set(new FakeResponseCookiesFeature(responseCookies)); // Act - tempDataProvider.SaveTempData(httpContext.Object, Dictionary); + tempDataProvider.SaveTempData(httpContext, Dictionary); // Assert Assert.Equal(1, responseCookies.Count); @@ -495,4 +468,9 @@ public override byte[] Serialize(IDictionary values) return Bytes; } } + + private class FakeResponseCookiesFeature(IResponseCookies cookies) : IResponseCookiesFeature + { + public IResponseCookies Cookies => cookies; + } } diff --git a/src/Security/CookiePolicy/test/CookieChunkingTests.cs b/src/Security/CookiePolicy/test/CookieChunkingTests.cs index c02fef621f88..c0a841fe4f85 100644 --- a/src/Security/CookiePolicy/test/CookieChunkingTests.cs +++ b/src/Security/CookiePolicy/test/CookieChunkingTests.cs @@ -80,6 +80,68 @@ public void AppendLargeCookieWithExtensions_Chunked() }, values); } + [Fact] + public void AppendSmallerCookieThanPriorValue_SingleChunk_DeletesOldChunks() + { + HttpContext context = new DefaultHttpContext(); + context.Request.Headers["Cookie"] = new[] + { + "TestCookie=chunks-7", + "TestCookieC1=abcdefghi", + "TestCookieC2=jklmnopqr", + "TestCookieC3=stuvwxyz0", + "TestCookieC4=123456789", + "TestCookieC5=ABCDEFGHI", + "TestCookieC6=JKLMNOPQR", + "TestCookieC7=STUVWXYZ" + }; + + new ChunkingCookieManager() { ChunkSize = 31 }.AppendResponseCookie(context, "TestCookie", "ShortValue", new CookieOptions()); + var values = context.Response.Headers["Set-Cookie"]; + Assert.Equal( + [ + "TestCookieC1=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/", + "TestCookieC2=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/", + "TestCookieC3=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/", + "TestCookieC4=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/", + "TestCookieC5=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/", + "TestCookieC6=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/", + "TestCookieC7=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/", + "TestCookie=ShortValue; path=/", + ], values); + } + + [Fact] + public void AppendSmallerCookieThanPriorValue_MultipleChunks_DeletesOldChunks() + { + HttpContext context = new DefaultHttpContext(); + context.Request.Headers["Cookie"] = new[] + { + "TestCookie=chunks-7", + "TestCookieC1=abcdefghi", + "TestCookieC2=jklmnopqr", + "TestCookieC3=stuvwxyz0", + "TestCookieC4=123456789", + "TestCookieC5=ABCDEFGHI", + "TestCookieC6=JKLMNOPQR", + "TestCookieC7=STUVWXYZ" + }; + + new ChunkingCookieManager() { ChunkSize = 31 }.AppendResponseCookie(context, "TestCookie", "abcdefghijklmnopqr", new CookieOptions()); + var values = context.Response.Headers["Set-Cookie"]; + Assert.Equal( + [ + "TestCookieC3=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/", + "TestCookieC4=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/", + "TestCookieC5=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/", + "TestCookieC6=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/", + "TestCookieC7=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/", + "TestCookie=chunks-2; path=/", + "TestCookieC1=abcdefghi; path=/", + "TestCookieC2=jklmnopqr; path=/", + ], values); + } + [Fact] public void GetLargeChunkedCookie_Reassembled() { diff --git a/src/Shared/ChunkingCookieManager/ChunkingCookieManager.cs b/src/Shared/ChunkingCookieManager/ChunkingCookieManager.cs index e67666749dbb..f2e2efcc6399 100644 --- a/src/Shared/ChunkingCookieManager/ChunkingCookieManager.cs +++ b/src/Shared/ChunkingCookieManager/ChunkingCookieManager.cs @@ -9,6 +9,7 @@ using System.Linq; using System.Text; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; @@ -157,9 +158,19 @@ public void AppendResponseCookie(HttpContext context, string key, string? value, var templateLength = options.CreateCookieHeader(key, string.Empty).ToString().Length; + var requestCookies = context.Request.Cookies; + var requestCookie = requestCookies[key]; + var requestChunks = ParseChunksCount(requestCookie); + // Normal cookie if (!ChunkSize.HasValue || ChunkSize.Value > templateLength + value.Length) { + if (requestChunks > 0) + { + // If the cookie was previously chunked but no longer is, delete the chunks. + DeleteChunks(context, requestCookies, options, key, startChunk: 1, endChunk: requestChunks); + } + responseCookies.Append(key, value, options); } else if (ChunkSize.Value < templateLength + 10) @@ -179,6 +190,12 @@ public void AppendResponseCookie(HttpContext context, string key, string? value, var dataSizePerCookie = ChunkSize.Value - templateLength - 3; // Budget 3 chars for the chunkid. var cookieChunkCount = (int)Math.Ceiling(value.Length * 1.0 / dataSizePerCookie); + if (requestChunks > cookieChunkCount) + { + // If the cookie was previously chunked but is now smaller, delete the chunks. + DeleteChunks(context, requestCookies, options, key, startChunk: cookieChunkCount + 1, endChunk: requestChunks); + } + var keyValuePairs = new KeyValuePair[cookieChunkCount + 1]; keyValuePairs[0] = KeyValuePair.Create(key, ChunkCountPrefix + cookieChunkCount.ToString(CultureInfo.InvariantCulture)); @@ -289,5 +306,33 @@ public void DeleteCookie(HttpContext context, string key, CookieOptions options) Expires = DateTimeOffset.UnixEpoch, }); } + + // Deletes unneeded cookie chunks, but not the primary cookie. + private static void DeleteChunks(HttpContext context, IRequestCookieCollection requestCookies, CookieOptions options, string key, int startChunk, int endChunk) + { + // Don't pre-allocate, we don't trust the input. + var keyValuePairs = new List>(); + + for (var i = startChunk; i <= endChunk; i++) + { + var subkey = key + ChunkKeySuffix + i.ToString(CultureInfo.InvariantCulture); + + // Only delete cookies we received. We received the chunk count cookie so we should have received the others too. + if (string.IsNullOrEmpty(requestCookies[subkey])) + { + break; + } + + keyValuePairs.Add(KeyValuePair.Create(subkey, string.Empty)); + } + + if (keyValuePairs.Count > 0) + { + context.Response.Cookies.Append(keyValuePairs.ToArray(), new CookieOptions(options) + { + Expires = DateTimeOffset.UnixEpoch, + }); + } + } }