Skip to content
This repository was archived by the owner on Nov 22, 2018. It is now read-only.

Commit a00d0d8

Browse files
committed
Feedback
1 parent 36a247c commit a00d0d8

File tree

5 files changed

+27
-18
lines changed

5 files changed

+27
-18
lines changed

src/Microsoft.AspNetCore.ResponseCaching/Internal/DefaultResponseCacheEntrySerializer.cs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,17 @@ private static CachedResponse ReadCachedResponse(BinaryReader reader)
103103
private static CachedVaryBy ReadCachedVaryBy(BinaryReader reader)
104104
{
105105
var headerCount = reader.ReadInt32();
106-
var headers = headerCount > 0 ? reader.ReadString().Split(',') : null;
106+
var headers = new string[headerCount];
107+
for (var index = 0; index < headerCount; index++)
108+
{
109+
headers[index] = reader.ReadString();
110+
}
107111
var paramCount = reader.ReadInt32();
108-
var param = paramCount > 0 ? reader.ReadString().Split(',') : null;
112+
var param = new string[paramCount];
113+
for (var index = 0; index < paramCount; index++)
114+
{
115+
param[index] = reader.ReadString();
116+
}
109117

110118
return new CachedVaryBy { Headers = headers, Params = param };
111119
}
@@ -162,15 +170,15 @@ private static void WriteCachedVaryBy(BinaryWriter writer, CachedVaryBy entry)
162170
writer.Write(nameof(CachedVaryBy));
163171

164172
writer.Write(entry.Headers.Count);
165-
if (entry.Headers.Count > 0)
173+
foreach (var header in entry.Headers)
166174
{
167-
writer.Write(entry.Headers);
175+
writer.Write(header);
168176
}
169177

170178
writer.Write(entry.Params.Count);
171-
if (entry.Params.Count > 0)
179+
foreach (var param in entry.Params)
172180
{
173-
writer.Write(entry.Params);
181+
writer.Write(param);
174182
}
175183
}
176184
}

src/Microsoft.AspNetCore.ResponseCaching/ResponseCachingContext.cs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ namespace Microsoft.AspNetCore.ResponseCaching
2020
internal class ResponseCachingContext
2121
{
2222
private static readonly CacheControlHeaderValue EmptyCacheControl = new CacheControlHeaderValue();
23-
private static readonly char KeyDelimiter = '\x1e'; // Use the record separator for delimiting components of the cache key
23+
// Use the record separator for delimiting components of the cache key to avoid possible collisions
24+
private static readonly char KeyDelimiter = '\x1e';
2425

2526
private readonly HttpContext _httpContext;
2627
private readonly IResponseCache _cache;
@@ -161,7 +162,7 @@ internal string CreateCacheKey(CachedVaryBy varyBy)
161162
// Vary by headers
162163
if (varyBy?.Headers.Count > 0)
163164
{
164-
// Append a value denoting the start of the header segment of the cache key
165+
// Append a group separator for the header segment of the cache key
165166
builder.Append(KeyDelimiter)
166167
.Append('H');
167168

@@ -187,7 +188,7 @@ internal string CreateCacheKey(CachedVaryBy varyBy)
187188
// Vary by query params
188189
if (varyBy?.Params.Count > 0)
189190
{
190-
// Append a value denoting the start of the query parameter segment of the cache key
191+
// Append a group separator for the query parameter segment of the cache key
191192
builder.Append(KeyDelimiter)
192193
.Append('Q');
193194

@@ -197,7 +198,7 @@ internal string CreateCacheKey(CachedVaryBy varyBy)
197198
foreach (var query in _httpContext.Request.Query.OrderBy(q => q.Key))
198199
{
199200
builder.Append(KeyDelimiter)
200-
.Append(query.Key.ToLower())
201+
.Append(query.Key.ToUpperInvariant())
201202
.Append("=")
202203
.Append(query.Value);
203204
}
@@ -228,7 +229,7 @@ internal string CreateCacheKey(CachedVaryBy varyBy)
228229
var customKey = _cacheKeySuffixProvider.CreateCustomKeySuffix(_httpContext);
229230
if (!string.IsNullOrEmpty(customKey))
230231
{
231-
// Append a value denoting the start of the query parameter segment of the cache key
232+
// Append a group separator for the custom segment of the cache key
232233
builder.Append(KeyDelimiter)
233234
.Append('C');
234235

src/Microsoft.AspNetCore.ResponseCaching/ResponseCachingOptionsFeature.cs renamed to src/Microsoft.AspNetCore.ResponseCaching/ResponseCachingFeature.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
namespace Microsoft.AspNetCore.ResponseCaching
77
{
88
// TODO: Temporary interface for endpoints to specify options for response caching
9-
public class ResponseCachingOptionsFeature
9+
public class ResponseCachingFeature
1010
{
11-
public StringValues Params;
11+
public StringValues Params { get; set; }
1212
}
1313
}

src/Microsoft.AspNetCore.ResponseCaching/ResponseCachingHttpContextExtensions.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,17 @@ public static class ResponseCachingHttpContextExtensions
1010
{
1111
public static void AddResponseCachingOptions(this HttpContext httpContext)
1212
{
13-
httpContext.Features.Set(new ResponseCachingOptionsFeature());
13+
httpContext.Features.Set(new ResponseCachingFeature());
1414
}
1515

1616
public static void RemoveResponseCachingOptions(this HttpContext httpContext)
1717
{
18-
httpContext.Features.Set<ResponseCachingOptionsFeature>(null);
18+
httpContext.Features.Set<ResponseCachingFeature>(null);
1919
}
2020

21-
public static ResponseCachingOptionsFeature GetResponseCachingOptions(this HttpContext httpContext)
21+
public static ResponseCachingFeature GetResponseCachingOptions(this HttpContext httpContext)
2222
{
23-
return httpContext.Features.Get<ResponseCachingOptionsFeature>() ?? new ResponseCachingOptionsFeature();
23+
return httpContext.Features.Get<ResponseCachingFeature>() ?? new ResponseCachingFeature();
2424
}
2525
}
2626
}

test/Microsoft.AspNetCore.ResponseCaching.Tests/ResponseCachingContextTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ public void CreateCacheKey_Includes_AllQueryParamsGivenAsterisk()
233233

234234
// To support case insensitivity, all param keys are converted to lower case.
235235
// Explicit VaryBy uses the casing specified in the setting.
236-
Assert.Equal($"GET{KeyDelimiter}/{KeyDelimiter}Q{KeyDelimiter}parama=ValueA{KeyDelimiter}paramb=ValueB", context.CreateCacheKey(new CachedVaryBy()
236+
Assert.Equal($"GET{KeyDelimiter}/{KeyDelimiter}Q{KeyDelimiter}PARAMA=ValueA{KeyDelimiter}PARAMB=ValueB", context.CreateCacheKey(new CachedVaryBy()
237237
{
238238
Params = new string[] { "*" }
239239
}));

0 commit comments

Comments
 (0)