dotnet/aspnetcore

Make request cacheable when it has "no-cache" header

Closed this issue · 8 comments

From @beltoev on Wednesday, June 7, 2017 1:48:39 PM

Why do we trust the client?

For example, "no-cache" header can be used for the ddos attacks. Besides OutputCacheAttribute (ASP.NET MVC 5) doesn't pay attention to this header.

Can we change the behavior of the attribute or add it to the settings?

Copied from original issue: aspnet/ResponseCaching#125

From @Tratcher on Wednesday, June 7, 2017 10:00:57 PM

Yes, this is a basic design question we haven't reconciled yet. The cache was built as a real HTTP cache (that honors client headers), not an output cache (that is only beholden to the server). The only additional feature it has is VaryByQueryKeys.

We either need to pivot the whole component to become a true output cache, or to provide a separate middleware that fulfills that role.

@glennc

From @GokGokalp on Thursday, November 9, 2017 9:22:31 PM

Any news?

From @JunTaoLuo on Monday, November 13, 2017 11:06:31 AM

We have not prioritized this work item for the next release. I'm putting this in backlog for now but @muratg may decide to move it out if he disagrees.

+1

There are currently two ways I've found to work around this:

Option#1:
replace

services.AddResponseCaching();

with

  public class CustomResponseCachingPolicyProvider : ResponseCachingPolicyProvider
  {
    public override bool AllowCacheLookup(ResponseCachingContext context)
    {
      return true;
    }

    public override bool AllowCacheStorage(ResponseCachingContext context)
    {
      return true;
    }
  }

      services.AddMemoryCache();
      services.TryAdd(ServiceDescriptor.Singleton<IResponseCachingPolicyProvider, CustomResponseCachingPolicyProvider>());
      services.TryAdd(ServiceDescriptor.Singleton<IResponseCachingKeyProvider, ResponseCachingKeyProvider>());

Option#2:

            app.Use(async (context, next) =>
            {
              List<KeyValuePair<string, StringValues>> cacheControlHeaders = context.Request.Headers.Where(x => x.Key.Trim().ToLower() == "cache-control" && x.Value.Count > 0 && (x.Value.Aggregate((y, z) => y + z).Trim().ToLower() == "no-cache" || x.Value.Aggregate((y, z) => y + z).Trim().ToLower() == "no-store")).ToList();
              foreach (var cacheControlHeader in cacheControlHeaders)
              {
                context.Request.Headers.Remove(cacheControlHeader);
              }
              List<KeyValuePair<string, StringValues>> pragmaNoCacheHeaders = context.Request.Headers.Where(x => x.Key.Trim().ToLower() == "pragma" && x.Value.Count > 0 && x.Value.Aggregate((y, z) => y + z).Trim().ToLower() == "no-cache").ToList();
              foreach (var pragmaNoCacheHeader in pragmaNoCacheHeaders)
              {
                context.Request.Headers.Remove(pragmaNoCacheHeader);
              }

              await next();
            });
      app.UseResponseCaching();

I've created a repo which extends the default ResponseCache implementation. It has an option to ignore the browser's no-cache/no-store headers.

https://github.com/speige/AspNetCore.ResponseCaching.Extensions
https://www.nuget.org/packages/AspNetCore.ResponseCaching.Extensions

I'd like to add that this doesn't just happen with a "no-cache" header, it also happens with a "cache-control: max-age=0" header. You can confirm this by loading a page that gets cached on the server, and then clicking the browser's refresh button to reload the page (no need to press Ctrl+F5, just use the browser's refresh button). This max-age=0 cache-control header is automatically placed by the browser in the headers, and because of that the ASP Core app processes the request rather than serving up a cached page.

Suggestion for dev team: If, for philosophical reasons, you insist on keeping the functionality as is (meaning that ASP Core honors the client http header over a server setting), then I would suggest improving the trace/log message to a more descriptive one for us developers. What I currently see in the Visual Studio output pane is "No cached response available for this request". Maybe it should say "Cached response will not be used because browser sent cache-control:max-age=0 header. (Tip: Try opening the page in a new browser tab rather than clicking the refresh button, as browsers automatically send this header when Refresh is clicked)". I have been tearing my hair out for the past few hours thinking that the request caching middleware app.UseResponseCaching() in ASP Core 2.1 wasn't working, when in fact it was behaving exactly as designed.

I agree to the Need for forcing Response Cache from Server side. It's a developers decision not a Browsers.

This is one of the main motivation for our work on Output Caching: This will be considered as part of the work on OutputCaching: #27387. The design and discussion will continue there.