Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
aspnet-hello opened this issue Jan 2, 2018 · 8 comments
Closed

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

aspnet-hello opened this issue Jan 2, 2018 · 8 comments
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares feature-response-caching Needs: Design This issue requires design work before implementating.
Milestone

Comments

@aspnet-hello
Copy link

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

@aspnet-hello
Copy link
Author

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

@aspnet-hello aspnet-hello added this to the Backlog milestone Jan 2, 2018
@aspnet-hello aspnet-hello added Needs: Design This issue requires design work before implementating. feature-response-caching labels Jan 2, 2018
@aspnet-hello
Copy link
Author

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

Any news?

@aspnet-hello
Copy link
Author

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.

@speige
Copy link
Contributor

speige commented Feb 14, 2018

+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();

@speige
Copy link
Contributor

speige commented Feb 16, 2018

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

@garyapps
Copy link

garyapps commented Jun 12, 2018

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.

@hannespreishuber
Copy link

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

@JunTaoLuo
Copy link
Contributor

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.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares and removed area-runtime labels Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares feature-response-caching Needs: Design This issue requires design work before implementating.
Projects
None yet
Development

No branches or pull requests

7 participants