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

Commit 2812291

Browse files
BrennanConroyJunTaoLuo
BrennanConroy
authored and
JunTaoLuo
committed
feedback
1 parent d4dbc7f commit 2812291

File tree

5 files changed

+104
-119
lines changed

5 files changed

+104
-119
lines changed

‎src/Microsoft.AspNetCore.ResponseCaching/Internal/ResponseCachingContext.cs

Lines changed: 8 additions & 26 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.TryStringToDate(HttpContext.Response.Headers[HeaderNames.Date], out date))
68+
if (ParsingHelpers.TryParseDate(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.TryStringToDate(HttpContext.Response.Headers[HeaderNames.Expires], out expires))
95+
if (ParsingHelpers.TryParseDate(HttpContext.Response.Headers[HeaderNames.Expires], out expires))
9696
{
9797
_responseExpires = expires;
9898
}
@@ -113,19 +113,10 @@ internal TimeSpan? ResponseSharedMaxAge
113113
{
114114
_parsedResponseSharedMaxAge = true;
115115
_responseSharedMaxAge = null;
116-
foreach (var header in HttpContext.Response.Headers[HeaderNames.CacheControl])
116+
int seconds;
117+
if (ParsingHelpers.TryGetHeaderValue(HttpContext.Response.Headers[HeaderNames.CacheControl], CacheControlValues.SharedMaxAgeString, out seconds))
117118
{
118-
var index = header.IndexOf(CacheControlValues.SharedMaxAgeString, StringComparison.OrdinalIgnoreCase);
119-
if (index != -1)
120-
{
121-
index += CacheControlValues.SharedMaxAgeString.Length;
122-
int seconds;
123-
if (ParsingHelpers.TryParseHeaderValue(index, header, out seconds))
124-
{
125-
_responseSharedMaxAge = TimeSpan.FromSeconds(seconds);
126-
}
127-
break;
128-
}
119+
_responseSharedMaxAge = TimeSpan.FromSeconds(seconds);
129120
}
130121
}
131122
return _responseSharedMaxAge;
@@ -140,19 +131,10 @@ internal TimeSpan? ResponseMaxAge
140131
{
141132
_parsedResponseMaxAge = true;
142133
_responseMaxAge = null;
143-
foreach (var header in HttpContext.Response.Headers[HeaderNames.CacheControl])
134+
int seconds;
135+
if (ParsingHelpers.TryGetHeaderValue(HttpContext.Response.Headers[HeaderNames.CacheControl], CacheControlValues.MaxAgeString, out seconds))
144136
{
145-
var index = header.IndexOf(CacheControlValues.MaxAgeString, StringComparison.OrdinalIgnoreCase);
146-
if (index != -1)
147-
{
148-
index += CacheControlValues.MaxAgeString.Length;
149-
int seconds;
150-
if (ParsingHelpers.TryParseHeaderValue(index, header, out seconds))
151-
{
152-
_responseMaxAge = TimeSpan.FromSeconds(seconds);
153-
}
154-
break;
155-
}
137+
_responseMaxAge = TimeSpan.FromSeconds(seconds);
156138
}
157139
}
158140
return _responseMaxAge;

‎src/Microsoft.AspNetCore.ResponseCaching/Internal/ResponseCachingPolicyProvider.cs

Lines changed: 18 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -190,38 +190,19 @@ public virtual bool IsCachedEntryFresh(ResponseCachingContext context)
190190
var requestCacheControlHeaders = context.HttpContext.Request.Headers[HeaderNames.CacheControl];
191191

192192
// Add min-fresh requirements
193-
foreach (var header in requestCacheControlHeaders)
193+
int seconds;
194+
if (ParsingHelpers.TryGetHeaderValue(requestCacheControlHeaders, CacheControlValues.MinFreshString, out seconds))
194195
{
195-
var index = header.IndexOf(CacheControlValues.MinFreshString, StringComparison.OrdinalIgnoreCase);
196-
if (index != -1)
197-
{
198-
index += CacheControlValues.MinFreshString.Length;
199-
int seconds;
200-
if (ParsingHelpers.TryParseHeaderValue(index, header, out seconds))
201-
{
202-
var minFresh = TimeSpan.FromSeconds(seconds);
203-
age += minFresh;
204-
context.Logger.LogExpirationMinFreshAdded(minFresh);
205-
}
206-
break;
207-
}
196+
var minFresh = TimeSpan.FromSeconds(seconds);
197+
age += minFresh;
198+
context.Logger.LogExpirationMinFreshAdded(minFresh);
208199
}
209200

210201
// Validate shared max age, this overrides any max age settings for shared caches
211202
TimeSpan? cachedSharedMaxAge = null;
212-
foreach (var header in cachedControlHeaders)
203+
if (ParsingHelpers.TryGetHeaderValue(cachedControlHeaders, CacheControlValues.SharedMaxAgeString, out seconds))
213204
{
214-
var index = header.IndexOf(CacheControlValues.SharedMaxAgeString, StringComparison.OrdinalIgnoreCase);
215-
if (index != -1)
216-
{
217-
index += CacheControlValues.SharedMaxAgeString.Length;
218-
int seconds;
219-
if (ParsingHelpers.TryParseHeaderValue(index, header, out seconds))
220-
{
221-
cachedSharedMaxAge = TimeSpan.FromSeconds(seconds);
222-
}
223-
break;
224-
}
205+
cachedSharedMaxAge = TimeSpan.FromSeconds(seconds);
225206
}
226207

227208
if (age >= cachedSharedMaxAge)
@@ -233,35 +214,15 @@ public virtual bool IsCachedEntryFresh(ResponseCachingContext context)
233214
else if (!cachedSharedMaxAge.HasValue)
234215
{
235216
TimeSpan? requestMaxAge = null;
236-
foreach (var header in requestCacheControlHeaders)
217+
if (ParsingHelpers.TryGetHeaderValue(requestCacheControlHeaders, CacheControlValues.MaxAgeString, out seconds))
237218
{
238-
var index = header.IndexOf(CacheControlValues.MaxAgeString, StringComparison.OrdinalIgnoreCase);
239-
if (index != -1)
240-
{
241-
index += CacheControlValues.MaxAgeString.Length;
242-
int seconds;
243-
if (ParsingHelpers.TryParseHeaderValue(index, header, out seconds))
244-
{
245-
requestMaxAge = TimeSpan.FromSeconds(seconds);
246-
}
247-
break;
248-
}
219+
requestMaxAge = TimeSpan.FromSeconds(seconds);
249220
}
250221

251222
TimeSpan? cachedMaxAge = null;
252-
foreach (var header in cachedControlHeaders)
223+
if (ParsingHelpers.TryGetHeaderValue(cachedControlHeaders, CacheControlValues.MaxAgeString, out seconds))
253224
{
254-
var index = header.IndexOf(CacheControlValues.MaxAgeString, StringComparison.OrdinalIgnoreCase);
255-
if (index != -1)
256-
{
257-
index += CacheControlValues.MaxAgeString.Length;
258-
int seconds;
259-
if (ParsingHelpers.TryParseHeaderValue(index, header, out seconds))
260-
{
261-
cachedMaxAge = TimeSpan.FromSeconds(seconds);
262-
}
263-
break;
264-
}
225+
cachedMaxAge = TimeSpan.FromSeconds(seconds);
265226
}
266227

267228
var lowestMaxAge = cachedMaxAge < requestMaxAge ? cachedMaxAge : requestMaxAge ?? cachedMaxAge;
@@ -279,19 +240,9 @@ public virtual bool IsCachedEntryFresh(ResponseCachingContext context)
279240
}
280241

281242
TimeSpan? requestMaxStale = null;
282-
foreach (var header in requestCacheControlHeaders)
243+
if (ParsingHelpers.TryGetHeaderValue(requestCacheControlHeaders, CacheControlValues.MaxStaleString, out seconds))
283244
{
284-
var index = header.IndexOf(CacheControlValues.MaxStaleString, StringComparison.OrdinalIgnoreCase);
285-
if (index != -1)
286-
{
287-
index += CacheControlValues.MaxStaleString.Length;
288-
int seconds;
289-
if (ParsingHelpers.TryParseHeaderValue(index, header, out seconds))
290-
{
291-
requestMaxStale = TimeSpan.FromSeconds(seconds);
292-
}
293-
break;
294-
}
245+
requestMaxStale = TimeSpan.FromSeconds(seconds);
295246
}
296247

297248
// Request allows stale values
@@ -308,15 +259,13 @@ public virtual bool IsCachedEntryFresh(ResponseCachingContext context)
308259
{
309260
// Validate expiration
310261
DateTimeOffset expires;
311-
if (context.CachedResponseHeaders[HeaderNames.Expires].Count > 0)
262+
if (context.CachedResponseHeaders[HeaderNames.Expires].Count > 0 &&
263+
ParsingHelpers.TryParseDate(context.CachedResponseHeaders[HeaderNames.Expires], out expires))
312264
{
313-
if (ParsingHelpers.TryStringToDate(context.CachedResponseHeaders[HeaderNames.Expires], out expires))
265+
if (context.ResponseTime.Value >= expires)
314266
{
315-
if (context.ResponseTime.Value >= expires)
316-
{
317-
context.Logger.LogExpirationExpiresExceeded(context.ResponseTime.Value, expires);
318-
return false;
319-
}
267+
context.Logger.LogExpirationExpiresExceeded(context.ResponseTime.Value, expires);
268+
return false;
320269
}
321270
}
322271
}

‎src/Microsoft.AspNetCore.ResponseCaching/ParsingHelpers.cs

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Globalization;
6+
using Microsoft.Extensions.Primitives;
67

78
namespace Microsoft.AspNetCore.ResponseCaching.Internal
89
{
@@ -29,30 +30,48 @@ internal static class ParsingHelpers
2930
"d MMM yyyy H:m:s", // RFC 5322 no day-of-week, no zone
3031
};
3132

32-
internal static bool TryStringToDate(string input, out DateTimeOffset result)
33+
// Try the various date formats in the order listed above.
34+
// 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,
36+
DateTimeStyles.AllowWhiteSpaces | DateTimeStyles.AssumeUniversal, out result);
37+
38+
internal static bool TryGetHeaderValue(StringValues headers, string headerName, out int value)
3339
{
34-
// Try the various date formats in the order listed above.
35-
// We should accept a wide verity of common formats, but only output RFC 1123 style dates.
36-
if (DateTimeOffset.TryParseExact(input, DateFormats, DateTimeFormatInfo.InvariantInfo,
37-
DateTimeStyles.AllowWhiteSpaces | DateTimeStyles.AssumeUniversal, out result))
40+
value = 0;
41+
foreach (var header in headers)
3842
{
39-
return true;
43+
var index = header.IndexOf(headerName, StringComparison.OrdinalIgnoreCase);
44+
if (index != -1)
45+
{
46+
index += headerName.Length;
47+
if (!TryParseHeaderValue(index, header, out value))
48+
{
49+
break;
50+
}
51+
return true;
52+
}
4053
}
41-
4254
return false;
4355
}
4456

45-
internal static bool TryParseHeaderValue(int startIndex, string header, out int value)
57+
private static bool TryParseHeaderValue(int startIndex, string header, out int value)
4658
{
59+
var found = false;
4760
while (startIndex != header.Length)
4861
{
49-
if (header[startIndex] == '=')
62+
var c = header[startIndex];
63+
if (c == '=')
64+
{
65+
found = true;
66+
}
67+
else if (c != ' ')
5068
{
69+
--startIndex;
5170
break;
5271
}
5372
++startIndex;
5473
}
55-
if (startIndex != header.Length)
74+
if (found && startIndex != header.Length)
5675
{
5776
var endIndex = startIndex + 1;
5877
while (endIndex < header.Length)
@@ -67,8 +86,12 @@ internal static bool TryParseHeaderValue(int startIndex, string header, out int
6786
break;
6887
}
6988
}
70-
value = int.Parse(header.Substring(startIndex + 1, endIndex - (startIndex + 1)), NumberStyles.None, NumberFormatInfo.InvariantInfo);
71-
return true;
89+
var length = endIndex - (startIndex + 1);
90+
if (length > 0)
91+
{
92+
value = int.Parse(header.Substring(startIndex + 1, length), NumberStyles.None, NumberFormatInfo.InvariantInfo);
93+
return true;
94+
}
7295
}
7396
value = 0;
7497
return false;

‎src/Microsoft.AspNetCore.ResponseCaching/ResponseCachingMiddleware.cs

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -376,13 +376,11 @@ internal static bool ContentIsNotModified(ResponseCachingContext context)
376376
foreach (var tag in ifNoneMatchHeader)
377377
{
378378
EntityTagHeaderValue requestETag;
379-
if (EntityTagHeaderValue.TryParse(tag, out requestETag))
379+
if (EntityTagHeaderValue.TryParse(tag, out requestETag) &&
380+
eTag.Compare(requestETag, useStrongComparison: false))
380381
{
381-
if (eTag.Compare(requestETag, useStrongComparison: false))
382-
{
383-
context.Logger.LogNotModifiedIfNoneMatchMatched(requestETag);
384-
return true;
385-
}
382+
context.Logger.LogNotModifiedIfNoneMatchMatched(requestETag);
383+
return true;
386384
}
387385
}
388386
}
@@ -394,16 +392,14 @@ internal static bool ContentIsNotModified(ResponseCachingContext context)
394392
if (!StringValues.IsNullOrEmpty(ifUnmodifiedSince))
395393
{
396394
DateTimeOffset modified;
397-
if (!ParsingHelpers.TryStringToDate(cachedResponseHeaders[HeaderNames.LastModified], out modified))
395+
if (!ParsingHelpers.TryParseDate(cachedResponseHeaders[HeaderNames.LastModified], out modified) &&
396+
!ParsingHelpers.TryParseDate(cachedResponseHeaders[HeaderNames.Date], out modified))
398397
{
399-
if (!ParsingHelpers.TryStringToDate(cachedResponseHeaders[HeaderNames.Date], out modified))
400-
{
401-
return false;
402-
}
398+
return false;
403399
}
404400

405401
DateTimeOffset unmodifiedSince;
406-
if (ParsingHelpers.TryStringToDate(ifUnmodifiedSince, out unmodifiedSince))
402+
if (ParsingHelpers.TryParseDate(ifUnmodifiedSince, out unmodifiedSince))
407403
{
408404
if (modified <= unmodifiedSince)
409405
{

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
using Microsoft.AspNetCore.ResponseCaching.Internal;
2+
using Microsoft.Extensions.Primitives;
3+
using Xunit;
4+
5+
namespace Microsoft.AspNetCore.ResponseCaching.Tests
6+
{
7+
public class ParsingHelpersTests
8+
{
9+
[Theory]
10+
[InlineData("h=1", "h", 1)]
11+
[InlineData("header1=3, header2=10", "header1", 3)]
12+
[InlineData("header1 =45, header2=80", "header1", 45)]
13+
[InlineData("header1= 89 , header2=22", "header1", 89)]
14+
[InlineData("header1= 89 , header2= 42", "header2", 42)]
15+
void TryGetHeaderValue_Succeeds(string headerValue, string headerName, int expectedValue)
16+
{
17+
int value;
18+
Assert.True(ParsingHelpers.TryGetHeaderValue(new StringValues(headerValue), headerName, out value));
19+
Assert.Equal(expectedValue, value);
20+
}
21+
22+
[Theory]
23+
[InlineData("h=", "h")]
24+
[InlineData("header1=, header2=10", "header1")]
25+
[InlineData("header1 , header2=80", "header1")]
26+
[InlineData("h=10", "header")]
27+
[InlineData("", "")]
28+
[InlineData(null, null)]
29+
void TryGetHeaderValue_Fails(string headerValue, string headerName)
30+
{
31+
int value;
32+
Assert.False(ParsingHelpers.TryGetHeaderValue(new StringValues(headerValue), headerName, out value));
33+
}
34+
}
35+
}

0 commit comments

Comments
 (0)