From 54610b8fe4cc988e9542227225d807be88efb82d Mon Sep 17 00:00:00 2001 From: John Luo Date: Thu, 6 Oct 2016 19:06:02 -0700 Subject: [PATCH] Handle null or empty vary rules --- .../ResponseCacheFeature.cs | 25 +++++++- .../ResponseCacheFeatureTests.cs | 64 +++++++++++++++++++ .../ResponseCacheMiddlewareTests.cs | 45 ++++++++++++- .../ResponseCachePolicyProviderTests.cs | 1 - 4 files changed, 131 insertions(+), 4 deletions(-) create mode 100644 test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCacheFeatureTests.cs diff --git a/src/Microsoft.AspNetCore.ResponseCaching/ResponseCacheFeature.cs b/src/Microsoft.AspNetCore.ResponseCaching/ResponseCacheFeature.cs index 374338a688..4c0b129ff4 100644 --- a/src/Microsoft.AspNetCore.ResponseCaching/ResponseCacheFeature.cs +++ b/src/Microsoft.AspNetCore.ResponseCaching/ResponseCacheFeature.cs @@ -1,12 +1,35 @@ // 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 Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.ResponseCaching { public class ResponseCacheFeature : IResponseCacheFeature { - public StringValues VaryByQueryKeys { get; set; } + private StringValues _varyByQueryKeys; + + public StringValues VaryByQueryKeys + { + get + { + return _varyByQueryKeys; + } + set + { + if (value.Count > 1) + { + for (var i = 0; i < value.Count; i++) + { + if (string.IsNullOrEmpty(value[i])) + { + throw new ArgumentException($"When {nameof(value)} contains more than one value, it cannot contain a null or empty value.", nameof(value)); + } + } + } + _varyByQueryKeys = value; + } + } } } diff --git a/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCacheFeatureTests.cs b/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCacheFeatureTests.cs new file mode 100644 index 0000000000..5b2fa7016f --- /dev/null +++ b/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCacheFeatureTests.cs @@ -0,0 +1,64 @@ +// 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 Microsoft.Extensions.Primitives; +using Xunit; + +namespace Microsoft.AspNetCore.ResponseCaching.Tests +{ + public class ResponseCacheFeatureTests + { + public static TheoryData ValidNullOrEmptyVaryRules + { + get + { + return new TheoryData + { + default(StringValues), + StringValues.Empty, + new StringValues((string)null), + new StringValues(string.Empty), + new StringValues((string[])null), + new StringValues(new string[0]), + new StringValues(new string[] { null }), + new StringValues(new string[] { string.Empty }) + }; + } + } + + [Theory] + [MemberData(nameof(ValidNullOrEmptyVaryRules))] + public void VaryByQueryKeys_Set_ValidEmptyValues_Succeeds(StringValues value) + { + // Does not throw + new ResponseCacheFeature().VaryByQueryKeys = value; + } + + public static TheoryData InvalidVaryRules + { + get + { + return new TheoryData + { + new StringValues(new string[] { null, null }), + new StringValues(new string[] { null, string.Empty }), + new StringValues(new string[] { string.Empty, null }), + new StringValues(new string[] { string.Empty, "Valid" }), + new StringValues(new string[] { "Valid", string.Empty }), + new StringValues(new string[] { null, "Valid" }), + new StringValues(new string[] { "Valid", null }) + }; + } + } + + + [Theory] + [MemberData(nameof(InvalidVaryRules))] + public void VaryByQueryKeys_Set_InValidEmptyValues_Throws(StringValues value) + { + // Throws + Assert.Throws(() => new ResponseCacheFeature().VaryByQueryKeys = value); + } + } +} diff --git a/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCacheMiddlewareTests.cs b/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCacheMiddlewareTests.cs index f2005ff54f..402c3a722f 100644 --- a/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCacheMiddlewareTests.cs +++ b/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCacheMiddlewareTests.cs @@ -7,8 +7,6 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Headers; using Microsoft.AspNetCore.ResponseCaching.Internal; -using Microsoft.AspNetCore.WebUtilities; -using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; @@ -488,6 +486,49 @@ namespace Microsoft.AspNetCore.ResponseCaching.Tests LoggedMessage.VaryByRulesUpdated); } + public static TheoryData NullOrEmptyVaryRules + { + get + { + return new TheoryData + { + default(StringValues), + StringValues.Empty, + new StringValues((string)null), + new StringValues(string.Empty), + new StringValues((string[])null), + new StringValues(new string[0]), + new StringValues(new string[] { null }), + new StringValues(new string[] { string.Empty }) + }; + } + } + + [Theory] + [MemberData(nameof(NullOrEmptyVaryRules))] + public async Task FinalizeCacheHeaders_UpdateCachedVaryByRules_NullOrEmptyRules(StringValues vary) + { + var store = new TestResponseCacheStore(); + var sink = new TestSink(); + var middleware = TestUtils.CreateTestMiddleware(testSink: sink, store: store); + var context = TestUtils.CreateTestContext(); + + context.HttpContext.Response.Headers[HeaderNames.Vary] = vary; + context.HttpContext.Features.Set(new ResponseCacheFeature() + { + VaryByQueryKeys = vary + }); + + await middleware.TryServeFromCacheAsync(context); + await middleware.FinalizeCacheHeadersAsync(context); + + // Vary rules should not be updated + Assert.Equal(0, store.SetCount); + TestUtils.AssertLoggedMessages( + sink.Writes, + LoggedMessage.NoResponseServed); + } + [Fact] public async Task FinalizeCacheHeaders_DoNotAddDate_IfSpecified() { diff --git a/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachePolicyProviderTests.cs b/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachePolicyProviderTests.cs index 2aefdb287e..5aeda6b24a 100644 --- a/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachePolicyProviderTests.cs +++ b/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachePolicyProviderTests.cs @@ -8,7 +8,6 @@ using Microsoft.AspNetCore.ResponseCaching.Internal; using Microsoft.Extensions.Logging.Testing; using Microsoft.Net.Http.Headers; using Xunit; -using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.ResponseCaching.Tests {