Handle null or empty vary rules

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;
+            }
+        }
     }
 }

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<StringValues> ValidNullOrEmptyVaryRules
+        {
+            get
+            {
+                return new TheoryData<StringValues>
+                {
+                    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<StringValues> InvalidVaryRules
+        {
+            get
+            {
+                return new TheoryData<StringValues>
+                {
+                    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<ArgumentException>(() => new ResponseCacheFeature().VaryByQueryKeys = value);
+        }
+    }
+}

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<StringValues> NullOrEmptyVaryRules
+        {
+            get
+            {
+                return new TheoryData<StringValues>
+                {
+                    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<IResponseCacheFeature>(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()
         {

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
 {