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

Commit 24c9f8c

Browse files
committed
more feedback
1 parent 6af29ef commit 24c9f8c

File tree

5 files changed

+69
-88
lines changed

5 files changed

+69
-88
lines changed

src/Microsoft.AspNetCore.ResponseCaching/ParsingHelpers.cs renamed to src/Microsoft.AspNetCore.ResponseCaching/Internal/HttpHeaderParsingHelpers.cs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
namespace Microsoft.AspNetCore.ResponseCaching.Internal
99
{
10-
internal static class ParsingHelpers
10+
internal static class HttpHeaderParsingHelpers
1111
{
1212
private static readonly string[] DateFormats = new string[] {
1313
// "r", // RFC 1123, required output format but too strict for input
@@ -32,9 +32,11 @@ internal static class ParsingHelpers
3232

3333
// Try the various date formats in the order listed above.
3434
// We should accept a wide verity of common formats, but only output RFC 1123 style dates.
35-
internal static bool TryParseDate(string input, out DateTimeOffset result) => DateTimeOffset.TryParseExact(input, DateFormats, DateTimeFormatInfo.InvariantInfo,
35+
internal static bool TryParseHeaderDate(string input, out DateTimeOffset result) => DateTimeOffset.TryParseExact(input, DateFormats, DateTimeFormatInfo.InvariantInfo,
3636
DateTimeStyles.AllowWhiteSpaces | DateTimeStyles.AssumeUniversal, out result);
3737

38+
// Try to get the value of a specific header from a list of headers
39+
// e.g. "header1=10, header2=30"
3840
internal static bool TryGetHeaderValue(StringValues headers, string headerName, out int value)
3941
{
4042
value = 0;
@@ -54,6 +56,20 @@ internal static bool TryGetHeaderValue(StringValues headers, string headerName,
5456
return false;
5557
}
5658

59+
internal static bool HeaderContains(StringValues headers, string headerName)
60+
{
61+
foreach (var header in headers)
62+
{
63+
var index = header.IndexOf(headerName, StringComparison.OrdinalIgnoreCase);
64+
if (index != -1)
65+
{
66+
return true;
67+
}
68+
}
69+
70+
return false;
71+
}
72+
5773
private static bool TryParseHeaderValue(int startIndex, string header, out int value)
5874
{
5975
var found = false;

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ internal DateTimeOffset? ResponseDate
6565
{
6666
_parsedResponseDate = true;
6767
DateTimeOffset date;
68-
if (ParsingHelpers.TryParseDate(HttpContext.Response.Headers[HeaderNames.Date], out date))
68+
if (HttpHeaderParsingHelpers.TryParseHeaderDate(HttpContext.Response.Headers[HeaderNames.Date], out date))
6969
{
7070
_responseDate = date;
7171
}
@@ -92,7 +92,7 @@ internal DateTimeOffset? ResponseExpires
9292
{
9393
_parsedResponseExpires = true;
9494
DateTimeOffset expires;
95-
if (ParsingHelpers.TryParseDate(HttpContext.Response.Headers[HeaderNames.Expires], out expires))
95+
if (HttpHeaderParsingHelpers.TryParseHeaderDate(HttpContext.Response.Headers[HeaderNames.Expires], out expires))
9696
{
9797
_responseExpires = expires;
9898
}
@@ -114,7 +114,7 @@ internal TimeSpan? ResponseSharedMaxAge
114114
_parsedResponseSharedMaxAge = true;
115115
_responseSharedMaxAge = null;
116116
int seconds;
117-
if (ParsingHelpers.TryGetHeaderValue(HttpContext.Response.Headers[HeaderNames.CacheControl], CacheControlValues.SharedMaxAgeString, out seconds))
117+
if (HttpHeaderParsingHelpers.TryGetHeaderValue(HttpContext.Response.Headers[HeaderNames.CacheControl], CacheControlValues.SharedMaxAgeString, out seconds))
118118
{
119119
_responseSharedMaxAge = TimeSpan.FromSeconds(seconds);
120120
}
@@ -132,7 +132,7 @@ internal TimeSpan? ResponseMaxAge
132132
_parsedResponseMaxAge = true;
133133
_responseMaxAge = null;
134134
int seconds;
135-
if (ParsingHelpers.TryGetHeaderValue(HttpContext.Response.Headers[HeaderNames.CacheControl], CacheControlValues.MaxAgeString, out seconds))
135+
if (HttpHeaderParsingHelpers.TryGetHeaderValue(HttpContext.Response.Headers[HeaderNames.CacheControl], CacheControlValues.MaxAgeString, out seconds))
136136
{
137137
_responseMaxAge = TimeSpan.FromSeconds(seconds);
138138
}

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

Lines changed: 31 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -30,26 +30,20 @@ public virtual bool IsRequestCacheable(ResponseCacheContext context)
3030
// Verify request cache-control parameters
3131
if (!StringValues.IsNullOrEmpty(request.Headers[HeaderNames.CacheControl]))
3232
{
33-
foreach (var header in request.Headers[HeaderNames.CacheControl])
33+
if (HttpHeaderParsingHelpers.HeaderContains(request.Headers[HeaderNames.CacheControl], CacheControlValues.NoCacheString))
3434
{
35-
if (header.IndexOf(CacheControlValues.NoCacheString, StringComparison.OrdinalIgnoreCase) != -1)
36-
{
37-
context.Logger.LogRequestWithNoCacheNotCacheable();
38-
return false;
39-
}
35+
context.Logger.LogRequestWithNoCacheNotCacheable();
36+
return false;
4037
}
4138
}
4239
else
4340
{
4441
// Support for legacy HTTP 1.0 cache directive
4542
var pragmaHeaderValues = request.Headers[HeaderNames.Pragma];
46-
foreach (var directive in pragmaHeaderValues)
43+
if (HttpHeaderParsingHelpers.HeaderContains(request.Headers[HeaderNames.Pragma], CacheControlValues.NoCacheString))
4744
{
48-
if (string.Equals(CacheControlValues.NoCacheString, directive, StringComparison.OrdinalIgnoreCase))
49-
{
50-
context.Logger.LogRequestWithPragmaNoCacheNotCacheable();
51-
return false;
52-
}
45+
context.Logger.LogRequestWithPragmaNoCacheNotCacheable();
46+
return false;
5347
}
5448
}
5549

@@ -61,48 +55,30 @@ public virtual bool IsResponseCacheable(ResponseCacheContext context)
6155
var responseCacheControlHeader = context.HttpContext.Response.Headers[HeaderNames.CacheControl];
6256

6357
// Only cache pages explicitly marked with public
64-
var isPublic = false;
65-
foreach (var header in responseCacheControlHeader)
66-
{
67-
if (header.IndexOf(CacheControlValues.PublicString, StringComparison.OrdinalIgnoreCase) != -1)
68-
{
69-
isPublic = true;
70-
break;
71-
}
72-
}
73-
if (!isPublic)
58+
if (!HttpHeaderParsingHelpers.HeaderContains(responseCacheControlHeader, CacheControlValues.PublicString))
7459
{
7560
context.Logger.LogResponseWithoutPublicNotCacheable();
7661
return false;
7762
}
7863

7964
// Check no-store
80-
foreach (var header in context.HttpContext.Request.Headers[HeaderNames.CacheControl])
65+
if (HttpHeaderParsingHelpers.HeaderContains(context.HttpContext.Request.Headers[HeaderNames.CacheControl], CacheControlValues.NoStoreString))
8166
{
82-
if (header.IndexOf(CacheControlValues.NoStoreString, StringComparison.OrdinalIgnoreCase) != -1)
83-
{
84-
context.Logger.LogResponseWithNoStoreNotCacheable();
85-
return false;
86-
}
67+
context.Logger.LogResponseWithNoStoreNotCacheable();
68+
return false;
8769
}
8870

89-
foreach (var header in responseCacheControlHeader)
71+
if (HttpHeaderParsingHelpers.HeaderContains(responseCacheControlHeader, CacheControlValues.NoStoreString))
9072
{
91-
if (header.IndexOf(CacheControlValues.NoStoreString, StringComparison.OrdinalIgnoreCase) != -1)
92-
{
93-
context.Logger.LogResponseWithNoStoreNotCacheable();
94-
return false;
95-
}
73+
context.Logger.LogResponseWithNoStoreNotCacheable();
74+
return false;
9675
}
9776

9877
// Check no-cache
99-
foreach (var header in responseCacheControlHeader)
78+
if (HttpHeaderParsingHelpers.HeaderContains(responseCacheControlHeader, CacheControlValues.NoCacheString))
10079
{
101-
if (header.IndexOf(CacheControlValues.NoCacheString, StringComparison.OrdinalIgnoreCase) != -1)
102-
{
103-
context.Logger.LogResponseWithNoCacheNotCacheable();
104-
return false;
105-
}
80+
context.Logger.LogResponseWithNoCacheNotCacheable();
81+
return false;
10682
}
10783

10884
var response = context.HttpContext.Response;
@@ -123,13 +99,10 @@ public virtual bool IsResponseCacheable(ResponseCacheContext context)
12399
}
124100

125101
// Check private
126-
foreach (var header in responseCacheControlHeader)
102+
if (HttpHeaderParsingHelpers.HeaderContains(responseCacheControlHeader, CacheControlValues.PrivateString))
127103
{
128-
if (header.IndexOf(CacheControlValues.PrivateString, StringComparison.OrdinalIgnoreCase) != -1)
129-
{
130-
context.Logger.LogResponseWithPrivateNotCacheable();
131-
return false;
132-
}
104+
context.Logger.LogResponseWithPrivateNotCacheable();
105+
return false;
133106
}
134107

135108
// Check response code
@@ -191,7 +164,7 @@ public virtual bool IsCachedEntryFresh(ResponseCacheContext context)
191164

192165
// Add min-fresh requirements
193166
int seconds;
194-
if (ParsingHelpers.TryGetHeaderValue(requestCacheControlHeaders, CacheControlValues.MinFreshString, out seconds))
167+
if (HttpHeaderParsingHelpers.TryGetHeaderValue(requestCacheControlHeaders, CacheControlValues.MinFreshString, out seconds))
195168
{
196169
var minFresh = TimeSpan.FromSeconds(seconds);
197170
age += minFresh;
@@ -200,7 +173,7 @@ public virtual bool IsCachedEntryFresh(ResponseCacheContext context)
200173

201174
// Validate shared max age, this overrides any max age settings for shared caches
202175
TimeSpan? cachedSharedMaxAge = null;
203-
if (ParsingHelpers.TryGetHeaderValue(cachedControlHeaders, CacheControlValues.SharedMaxAgeString, out seconds))
176+
if (HttpHeaderParsingHelpers.TryGetHeaderValue(cachedControlHeaders, CacheControlValues.SharedMaxAgeString, out seconds))
204177
{
205178
cachedSharedMaxAge = TimeSpan.FromSeconds(seconds);
206179
}
@@ -214,13 +187,13 @@ public virtual bool IsCachedEntryFresh(ResponseCacheContext context)
214187
else if (!cachedSharedMaxAge.HasValue)
215188
{
216189
TimeSpan? requestMaxAge = null;
217-
if (ParsingHelpers.TryGetHeaderValue(requestCacheControlHeaders, CacheControlValues.MaxAgeString, out seconds))
190+
if (HttpHeaderParsingHelpers.TryGetHeaderValue(requestCacheControlHeaders, CacheControlValues.MaxAgeString, out seconds))
218191
{
219192
requestMaxAge = TimeSpan.FromSeconds(seconds);
220193
}
221194

222195
TimeSpan? cachedMaxAge = null;
223-
if (ParsingHelpers.TryGetHeaderValue(cachedControlHeaders, CacheControlValues.MaxAgeString, out seconds))
196+
if (HttpHeaderParsingHelpers.TryGetHeaderValue(cachedControlHeaders, CacheControlValues.MaxAgeString, out seconds))
224197
{
225198
cachedMaxAge = TimeSpan.FromSeconds(seconds);
226199
}
@@ -230,17 +203,14 @@ public virtual bool IsCachedEntryFresh(ResponseCacheContext context)
230203
if (age >= lowestMaxAge)
231204
{
232205
// Must revalidate
233-
foreach (var header in cachedControlHeaders)
206+
if (HttpHeaderParsingHelpers.HeaderContains(cachedControlHeaders, CacheControlValues.MustRevalidateString))
234207
{
235-
if (header.IndexOf(CacheControlValues.MustRevalidateString, StringComparison.OrdinalIgnoreCase) != -1)
236-
{
237-
context.Logger.LogExpirationMustRevalidate(age, lowestMaxAge.Value);
238-
return false;
239-
}
208+
context.Logger.LogExpirationMustRevalidate(age, lowestMaxAge.Value);
209+
return false;
240210
}
241211

242212
TimeSpan? requestMaxStale = null;
243-
if (ParsingHelpers.TryGetHeaderValue(requestCacheControlHeaders, CacheControlValues.MaxStaleString, out seconds))
213+
if (HttpHeaderParsingHelpers.TryGetHeaderValue(requestCacheControlHeaders, CacheControlValues.MaxStaleString, out seconds))
244214
{
245215
requestMaxStale = TimeSpan.FromSeconds(seconds);
246216
}
@@ -259,14 +229,11 @@ public virtual bool IsCachedEntryFresh(ResponseCacheContext context)
259229
{
260230
// Validate expiration
261231
DateTimeOffset expires;
262-
if (context.CachedResponseHeaders[HeaderNames.Expires].Count > 0 &&
263-
ParsingHelpers.TryParseDate(context.CachedResponseHeaders[HeaderNames.Expires], out expires))
232+
if (HttpHeaderParsingHelpers.TryParseHeaderDate(context.CachedResponseHeaders[HeaderNames.Expires], out expires) &&
233+
context.ResponseTime.Value >= expires)
264234
{
265-
if (context.ResponseTime.Value >= expires)
266-
{
267-
context.Logger.LogExpirationExpiresExceeded(context.ResponseTime.Value, expires);
268-
return false;
269-
}
235+
context.Logger.LogExpirationExpiresExceeded(context.ResponseTime.Value, expires);
236+
return false;
270237
}
271238
}
272239
}

src/Microsoft.AspNetCore.ResponseCaching/ResponseCacheMiddleware.cs

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -191,14 +191,11 @@ internal async Task<bool> TryServeFromCacheAsync(ResponseCacheContext context)
191191
return true;
192192
}
193193

194-
foreach (var header in context.HttpContext.Request.Headers[HeaderNames.CacheControl])
194+
if (HttpHeaderParsingHelpers.HeaderContains(context.HttpContext.Request.Headers[HeaderNames.CacheControl], CacheControlValues.OnlyIfCachedString))
195195
{
196-
if (header.IndexOf(CacheControlValues.OnlyIfCachedString, StringComparison.OrdinalIgnoreCase) != -1)
197-
{
198-
_logger.LogGatewayTimeoutServed();
199-
context.HttpContext.Response.StatusCode = StatusCodes.Status504GatewayTimeout;
200-
return true;
201-
}
196+
_logger.LogGatewayTimeoutServed();
197+
context.HttpContext.Response.StatusCode = StatusCodes.Status504GatewayTimeout;
198+
return true;
202199
}
203200

204201
_logger.LogNoResponseServed();
@@ -379,20 +376,18 @@ internal static bool ContentIsNotModified(ResponseCacheContext context)
379376
if (!StringValues.IsNullOrEmpty(ifUnmodifiedSince))
380377
{
381378
DateTimeOffset modified;
382-
if (!ParsingHelpers.TryParseDate(cachedResponseHeaders[HeaderNames.LastModified], out modified) &&
383-
!ParsingHelpers.TryParseDate(cachedResponseHeaders[HeaderNames.Date], out modified))
379+
if (!HttpHeaderParsingHelpers.TryParseHeaderDate(cachedResponseHeaders[HeaderNames.LastModified], out modified) &&
380+
!HttpHeaderParsingHelpers.TryParseHeaderDate(cachedResponseHeaders[HeaderNames.Date], out modified))
384381
{
385382
return false;
386383
}
387384

388385
DateTimeOffset unmodifiedSince;
389-
if (ParsingHelpers.TryParseDate(ifUnmodifiedSince, out unmodifiedSince))
386+
if (HttpHeaderParsingHelpers.TryParseHeaderDate(ifUnmodifiedSince, out unmodifiedSince) &&
387+
modified <= unmodifiedSince)
390388
{
391-
if (modified <= unmodifiedSince)
392-
{
393-
context.Logger.LogNotModifiedIfUnmodifiedSinceSatisfied(modified, unmodifiedSince);
394-
return true;
395-
}
389+
context.Logger.LogNotModifiedIfUnmodifiedSinceSatisfied(modified, unmodifiedSince);
390+
return true;
396391
}
397392
}
398393
}

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
using Microsoft.AspNetCore.ResponseCaching.Internal;
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using Microsoft.AspNetCore.ResponseCaching.Internal;
25
using Microsoft.Extensions.Primitives;
36
using Xunit;
47

@@ -15,7 +18,7 @@ public class ParsingHelpersTests
1518
void TryGetHeaderValue_Succeeds(string headerValue, string headerName, int expectedValue)
1619
{
1720
int value;
18-
Assert.True(ParsingHelpers.TryGetHeaderValue(new StringValues(headerValue), headerName, out value));
21+
Assert.True(HttpHeaderParsingHelpers.TryGetHeaderValue(new StringValues(headerValue), headerName, out value));
1922
Assert.Equal(expectedValue, value);
2023
}
2124

@@ -29,7 +32,7 @@ void TryGetHeaderValue_Succeeds(string headerValue, string headerName, int expec
2932
void TryGetHeaderValue_Fails(string headerValue, string headerName)
3033
{
3134
int value;
32-
Assert.False(ParsingHelpers.TryGetHeaderValue(new StringValues(headerValue), headerName, out value));
35+
Assert.False(HttpHeaderParsingHelpers.TryGetHeaderValue(new StringValues(headerValue), headerName, out value));
3336
}
3437
}
3538
}

0 commit comments

Comments
 (0)