Restructure response caching middleware flow

- Always add IresponseCachingFeatu8re before calling the next middleware #81
- Use If-Modified-Since instead of the incorrect If-Unmodified-Since header #83
- Handle proxy-revalidate in the same way as must-revalidate #83
- Handle max-stale with no specified limit #83
- Bypass cache lookup for no-cache but store the response #83
- Bypass response capturing and buffering when no-store is specified #83
- Replace IsRequestCacheable cache policy with three new independent policy checks to reflect these changes

src/Microsoft.AspNetCore.ResponseCaching/Internal/Interfaces/IResponseCachingPolicyProvider.cs

@@ -6,21 +6,35 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
     public interface IResponseCachingPolicyProvider
     {
         /// <summary>
-        /// Determine wehther the response cache middleware should be executed for the incoming HTTP request.
+        /// Determine whether the response caching logic should be attempted for the incoming HTTP request.
         /// </summary>
         /// <param name="context">The <see cref="ResponseCachingContext"/>.</param>
-        /// <returns><c>true</c> if the request is cacheable; otherwise <c>false</c>.</returns>
-        bool IsRequestCacheable(ResponseCachingContext context);
+        /// <returns><c>true</c> if response caching logic should be attempted; otherwise <c>false</c>.</returns>
+        bool AttemptResponseCaching(ResponseCachingContext context);
 
         /// <summary>
-        /// Determine whether the response received by the middleware be cached for future requests.
+        /// Determine whether a cache lookup is allowed for the incoming HTTP request.
+        /// </summary>
+        /// <param name="context">The <see cref="ResponseCachingContext"/>.</param>
+        /// <returns><c>true</c> if cache lookup for this request is allowed; otherwise <c>false</c>.</returns>
+        bool AllowCacheLookup(ResponseCachingContext context);
+
+        /// <summary>
+        /// Determine whether storage of the response is allowed for the incoming HTTP request.
+        /// </summary>
+        /// <param name="context">The <see cref="ResponseCachingContext"/>.</param>
+        /// <returns><c>true</c> if storage of the response for this request is allowed; otherwise <c>false</c>.</returns>
+        bool AllowCacheStorage(ResponseCachingContext context);
+
+        /// <summary>
+        /// Determine whether the response received by the middleware can be cached for future requests.
         /// </summary>
         /// <param name="context">The <see cref="ResponseCachingContext"/>.</param>
         /// <returns><c>true</c> if the response is cacheable; otherwise <c>false</c>.</returns>
         bool IsResponseCacheable(ResponseCachingContext context);
 
         /// <summary>
-        /// Determine whether the response retrieved from the response cache is fresh and be served.
+        /// Determine whether the response retrieved from the response cache is fresh and can be served.
         /// </summary>
         /// <param name="context">The <see cref="ResponseCachingContext"/>.</param>
         /// <returns><c>true</c> if the cached entry is fresh; otherwise <c>false</c>.</returns>

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

@@ -31,7 +31,7 @@ internal static class LoggerExtensions
         private static Action<ILogger, int, Exception> _logResponseWithUnsuccessfulStatusCodeNotCacheable;
         private static Action<ILogger, Exception> _logNotModifiedIfNoneMatchStar;
         private static Action<ILogger, EntityTagHeaderValue, Exception> _logNotModifiedIfNoneMatchMatched;
-        private static Action<ILogger, DateTimeOffset, DateTimeOffset, Exception> _logNotModifiedIfUnmodifiedSinceSatisfied;
+        private static Action<ILogger, DateTimeOffset, DateTimeOffset, Exception> _logNotModifiedIfModifiedSinceSatisfied;
         private static Action<ILogger, Exception> _logNotModifiedServed;
         private static Action<ILogger, Exception> _logCachedResponseServed;
         private static Action<ILogger, Exception> _logGatewayTimeoutServed;
@@ -40,6 +40,7 @@ internal static class LoggerExtensions
         private static Action<ILogger, Exception> _logResponseCached;
         private static Action<ILogger, Exception> _logResponseNotCached;
         private static Action<ILogger, Exception> _logResponseContentLengthMismatchNotCached;
+        private static Action<ILogger, TimeSpan, TimeSpan, Exception> _logExpirationInfiniteMaxStaleSatisfied;
 
         static LoggerExtensions()
         {
@@ -70,7 +71,7 @@ static LoggerExtensions()
             _logExpirationMustRevalidate = LoggerMessage.Define<TimeSpan, TimeSpan>(
                 logLevel: LogLevel.Debug,
                 eventId: 7,
-                formatString: "The age of the entry is {Age} and has exceeded the maximum age of {MaxAge} specified by the 'max-age' cache directive. It must be revalidated because the 'must-revalidate' cache directive is specified.");
+                formatString: "The age of the entry is {Age} and has exceeded the maximum age of {MaxAge} specified by the 'max-age' cache directive. It must be revalidated because the 'must-revalidate' or 'proxy-revalidate' cache directive is specified.");
             _logExpirationMaxStaleSatisfied = LoggerMessage.Define<TimeSpan, TimeSpan, TimeSpan>(
                 logLevel: LogLevel.Debug,
                 eventId: 8,
@@ -119,10 +120,10 @@ static LoggerExtensions()
                 logLevel: LogLevel.Debug,
                 eventId: 19,
                 formatString: $"The ETag {{ETag}} in the '{HeaderNames.IfNoneMatch}' header matched the ETag of a cached entry.");
-            _logNotModifiedIfUnmodifiedSinceSatisfied = LoggerMessage.Define<DateTimeOffset, DateTimeOffset>(
+            _logNotModifiedIfModifiedSinceSatisfied = LoggerMessage.Define<DateTimeOffset, DateTimeOffset>(
                 logLevel: LogLevel.Debug,
                 eventId: 20,
-                formatString: $"The last modified date of {{LastModified}} is before the date {{IfUnmodifiedSince}} specified in the '{HeaderNames.IfUnmodifiedSince}' header.");
+                formatString: $"The last modified date of {{LastModified}} is before the date {{IfModifiedSince}} specified in the '{HeaderNames.IfModifiedSince}' header.");
             _logNotModifiedServed = LoggerMessage.Define(
                 logLevel: LogLevel.Information,
                 eventId: 21,
@@ -155,6 +156,10 @@ static LoggerExtensions()
                 logLevel: LogLevel.Warning,
                 eventId: 28,
                 formatString: $"The response could not be cached for this request because the '{HeaderNames.ContentLength}' did not match the body length.");
+            _logExpirationInfiniteMaxStaleSatisfied = LoggerMessage.Define<TimeSpan, TimeSpan>(
+                logLevel: LogLevel.Debug,
+                eventId: 29,
+                formatString: "The age of the entry is {Age} and has exceeded the maximum age of {MaxAge} specified by the 'max-age' cache directive. However, the 'max-stale' cache directive was specified without an assigned value and a stale response of any age is accepted.");
         }
 
         internal static void LogRequestMethodNotCacheable(this ILogger logger, string method)
@@ -252,9 +257,9 @@ internal static void LogNotModifiedIfNoneMatchMatched(this ILogger logger, Entit
             _logNotModifiedIfNoneMatchMatched(logger, etag, null);
         }
 
-        internal static void LogNotModifiedIfUnmodifiedSinceSatisfied(this ILogger logger, DateTimeOffset lastModified, DateTimeOffset ifUnmodifiedSince)
+        internal static void LogNotModifiedIfModifiedSinceSatisfied(this ILogger logger, DateTimeOffset lastModified, DateTimeOffset ifModifiedSince)
         {
-            _logNotModifiedIfUnmodifiedSinceSatisfied(logger, lastModified, ifUnmodifiedSince, null);
+            _logNotModifiedIfModifiedSinceSatisfied(logger, lastModified, ifModifiedSince, null);
         }
 
         internal static void LogNotModifiedServed(this ILogger logger)
@@ -296,5 +301,10 @@ internal static void LogResponseContentLengthMismatchNotCached(this ILogger logg
         {
             _logResponseContentLengthMismatchNotCached(logger, null);
         }
+
+        internal static void LogExpirationInfiniteMaxStaleSatisfied(this ILogger logger, TimeSpan age, TimeSpan maxAge)
+        {
+            _logExpirationInfiniteMaxStaleSatisfied(logger, age, maxAge, null);
+        }
     }
 }

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

@@ -37,7 +37,7 @@ internal ResponseCachingContext(HttpContext httpContext, ILogger logger)
 
         internal ILogger Logger { get; }
 
-        internal bool ShouldCacheResponse { get;  set; }
+        internal bool ShouldCacheResponse { get; set; }
 
         internal string BaseKey { get;  set; }
 

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

@@ -10,10 +10,11 @@ namespace Microsoft.AspNetCore.ResponseCaching.Internal
 {
     public class ResponseCachingPolicyProvider : IResponseCachingPolicyProvider
     {
-        public virtual bool IsRequestCacheable(ResponseCachingContext context)
+        public virtual bool AttemptResponseCaching(ResponseCachingContext context)
         {
-            // Verify the method
             var request = context.HttpContext.Request;
+
+            // Verify the method
             if (!HttpMethods.IsGet(request.Method) && !HttpMethods.IsHead(request.Method))
             {
                 context.Logger.LogRequestMethodNotCacheable(request.Method);
@@ -27,6 +28,13 @@ public virtual bool IsRequestCacheable(ResponseCachingContext context)
                 return false;
             }
 
+            return true;
+        }
+
+        public virtual bool AllowCacheLookup(ResponseCachingContext context)
+        {
+            var request = context.HttpContext.Request;
+
             // Verify request cache-control parameters
             if (!StringValues.IsNullOrEmpty(request.Headers[HeaderNames.CacheControl]))
             {
@@ -50,6 +58,12 @@ public virtual bool IsRequestCacheable(ResponseCachingContext context)
             return true;
         }
 
+        public virtual bool AllowCacheStorage(ResponseCachingContext context)
+        {
+            // Check request no-store
+            return !HeaderUtilities.ContainsCacheDirective(context.HttpContext.Request.Headers[HeaderNames.CacheControl], CacheControlHeaderValue.NoStoreString);
+        }
+
         public virtual bool IsResponseCacheable(ResponseCachingContext context)
         {
             var responseCacheControlHeader = context.HttpContext.Response.Headers[HeaderNames.CacheControl];
@@ -61,9 +75,8 @@ public virtual bool IsResponseCacheable(ResponseCachingContext context)
                 return false;
             }
 
-            // Check no-store
-            if (HeaderUtilities.ContainsCacheDirective(context.HttpContext.Request.Headers[HeaderNames.CacheControl], CacheControlHeaderValue.NoStoreString)
-                || HeaderUtilities.ContainsCacheDirective(responseCacheControlHeader, CacheControlHeaderValue.NoStoreString))
+            // Check response no-store
+            if (HeaderUtilities.ContainsCacheDirective(responseCacheControlHeader, CacheControlHeaderValue.NoStoreString))
             {
                 context.Logger.LogResponseWithNoStoreNotCacheable();
                 return false;
@@ -187,17 +200,26 @@ public virtual bool IsCachedEntryFresh(ResponseCachingContext context)
                 // Validate max age
                 if (age >= lowestMaxAge)
                 {
-                    // Must revalidate
-                    if (HeaderUtilities.ContainsCacheDirective(cachedCacheControlHeaders, CacheControlHeaderValue.MustRevalidateString))
+                    // Must revalidate or proxy revalidate
+                    if (HeaderUtilities.ContainsCacheDirective(cachedCacheControlHeaders, CacheControlHeaderValue.MustRevalidateString)
+                        || HeaderUtilities.ContainsCacheDirective(cachedCacheControlHeaders, CacheControlHeaderValue.ProxyRevalidateString))
                     {
                         context.Logger.LogExpirationMustRevalidate(age, lowestMaxAge.Value);
                         return false;
                     }
 
                     TimeSpan? requestMaxStale;
+                    var maxStaleExist = HeaderUtilities.ContainsCacheDirective(requestCacheControlHeaders, CacheControlHeaderValue.MaxStaleString);
                     HeaderUtilities.TryParseSeconds(requestCacheControlHeaders, CacheControlHeaderValue.MaxStaleString, out requestMaxStale);
 
-                    // Request allows stale values
+                    // Request allows stale values with no age limit
+                    if (maxStaleExist && !requestMaxStale.HasValue)
+                    {
+                        context.Logger.LogExpirationInfiniteMaxStaleSatisfied(age, lowestMaxAge.Value);
+                        return true;
+                    }
+
+                    // Request allows stale values with age limit
                     if (requestMaxStale.HasValue && age - lowestMaxAge < requestMaxStale)
                     {
                         context.Logger.LogExpirationMaxStaleSatisfied(age, lowestMaxAge.Value, requestMaxStale.Value);