From d9778252d0d499b458206dfcea09683b0c71b66f Mon Sep 17 00:00:00 2001 From: John Luo Date: Fri, 23 Feb 2018 15:09:50 -0800 Subject: [PATCH] Sort header and query values --- .../Internal/ResponseCachingKeyProvider.cs | 49 +++++++++++++++---- .../ResponseCachingKeyProviderTests.cs | 34 +++++++++++++ 2 files changed, 73 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.AspNetCore.ResponseCaching/Internal/ResponseCachingKeyProvider.cs b/src/Microsoft.AspNetCore.ResponseCaching/Internal/ResponseCachingKeyProvider.cs index 5524d8f687..d69d9008eb 100644 --- a/src/Microsoft.AspNetCore.ResponseCaching/Internal/ResponseCachingKeyProvider.cs +++ b/src/Microsoft.AspNetCore.ResponseCaching/Internal/ResponseCachingKeyProvider.cs @@ -82,7 +82,7 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal } } - // BaseKeyHHeaderName=HeaderValueQQueryName=QueryValue + // BaseKeyHHeaderName=HeaderValueQQueryName=QueryValue1QueryValue2 public string CreateStorageVaryByKey(ResponseCachingContext context) { if (context == null) @@ -124,9 +124,12 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal .Append(header) .Append("="); - for (var j = 0; j < headerValues.Count; j++) + var headerValuesArray = headerValues.ToArray(); + Array.Sort(headerValuesArray, StringComparer.Ordinal); + + for (var j = 0; j < headerValuesArray.Length; j++) { - builder.Append(headerValues[j]); + builder.Append(headerValuesArray[j]); } } } @@ -141,19 +144,27 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal if (varyByRules.QueryKeys.Count == 1 && string.Equals(varyByRules.QueryKeys[0], "*", StringComparison.Ordinal)) { // Vary by all available query keys - foreach (var query in context.HttpContext.Request.Query.OrderBy(q => q.Key, StringComparer.OrdinalIgnoreCase)) + var queryArray = context.HttpContext.Request.Query.ToArray(); + // Query keys are aggregated case-insensitively whereas the query values are compared ordinally. + Array.Sort(queryArray, QueryKeyComparer.OrdinalIgnoreCase); + + for (var i = 0; i < queryArray.Length; i++) { builder.Append(KeyDelimiter) - .AppendUpperInvariant(query.Key) + .AppendUpperInvariant(queryArray[i].Key) .Append("="); - for (var i = 0; i < query.Value.Count; i++) + var queryValueArray = queryArray[i].Value.ToArray(); + Array.Sort(queryValueArray, StringComparer.Ordinal); + + for (var j = 0; j < queryValueArray.Length; j++) { - if (i > 0) + if (j > 0) { builder.Append(KeySubDelimiter); } - builder.Append(query.Value[i]); + + builder.Append(queryValueArray[j]); } } } @@ -167,13 +178,17 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal .Append(queryKey) .Append("="); - for (var j = 0; j < queryKeyValues.Count; j++) + var queryValueArray = queryKeyValues.ToArray(); + Array.Sort(queryValueArray, StringComparer.Ordinal); + + for (var j = 0; j < queryValueArray.Length; j++) { if (j > 0) { builder.Append(KeySubDelimiter); } - builder.Append(queryKeyValues[j]); + + builder.Append(queryValueArray[j]); } } } @@ -186,5 +201,19 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal _builderPool.Return(builder); } } + + private class QueryKeyComparer : IComparer> + { + private StringComparer _stringComparer; + + public static QueryKeyComparer OrdinalIgnoreCase { get; } = new QueryKeyComparer(StringComparer.OrdinalIgnoreCase); + + public QueryKeyComparer(StringComparer stringComparer) + { + _stringComparer = stringComparer; + } + + public int Compare(KeyValuePair x, KeyValuePair y) => _stringComparer.Compare(x.Key, y.Key); + } } } diff --git a/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachingKeyProviderTests.cs b/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachingKeyProviderTests.cs index 1c3cd0e485..36bd3da0c8 100644 --- a/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachingKeyProviderTests.cs +++ b/test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachingKeyProviderTests.cs @@ -94,6 +94,22 @@ namespace Microsoft.AspNetCore.ResponseCaching.Tests cacheKeyProvider.CreateStorageVaryByKey(context)); } + [Fact] + public void ResponseCachingKeyProvider_CreateStorageVaryKey_HeaderValuesAreSorted() + { + var cacheKeyProvider = TestUtils.CreateTestKeyProvider(); + var context = TestUtils.CreateTestContext(); + context.HttpContext.Request.Headers["HeaderA"] = "ValueB"; + context.HttpContext.Request.Headers.Append("HeaderA", "ValueA"); + context.CachedVaryByRules = new CachedVaryByRules() + { + Headers = new string[] { "HeaderA", "HeaderC" } + }; + + Assert.Equal($"{context.CachedVaryByRules.VaryByKeyPrefix}{KeyDelimiter}H{KeyDelimiter}HeaderA=ValueAValueB{KeyDelimiter}HeaderC=", + cacheKeyProvider.CreateStorageVaryByKey(context)); + } + [Fact] public void ResponseCachingKeyProvider_CreateStorageVaryKey_IncludesListedQueryKeysOnly() { @@ -162,6 +178,24 @@ namespace Microsoft.AspNetCore.ResponseCaching.Tests cacheKeyProvider.CreateStorageVaryByKey(context)); } + [Fact] + public void ResponseCachingKeyProvider_CreateStorageVaryKey_QueryKeysValuesAreSorted() + { + var cacheKeyProvider = TestUtils.CreateTestKeyProvider(); + var context = TestUtils.CreateTestContext(); + context.HttpContext.Request.QueryString = new QueryString("?QueryA=ValueB&QueryA=ValueA"); + context.CachedVaryByRules = new CachedVaryByRules() + { + VaryByKeyPrefix = FastGuid.NewGuid().IdString, + QueryKeys = new string[] { "*" } + }; + + // To support case insensitivity, all query keys are converted to upper case. + // Explicit query keys uses the casing specified in the setting. + Assert.Equal($"{context.CachedVaryByRules.VaryByKeyPrefix}{KeyDelimiter}Q{KeyDelimiter}QUERYA=ValueA{KeySubDelimiter}ValueB", + cacheKeyProvider.CreateStorageVaryByKey(context)); + } + [Fact] public void ResponseCachingKeyProvider_CreateStorageVaryKey_IncludesListedHeadersAndQueryKeys() {