dotnet / dotnet/aspnetcore

Cache-Control header: What should the default or easier opt-ins be?

Open
#19,782 7 comments 10 reactions 1 assignee Claimed by @LadyNaggaga View on GitHub
affected-medium area-mvc enhancement feature-mvc-execution-pipeline migration severity-minor
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 5h
Merged PRs (30d)
276

Description

In our port from .NET 4.7.2 to .NET Core 3.1.2 we didn't notice a critical difference until very late in the game: the `Cache-Control` headers, by default, are very different. It's my understanding from @blowdart that some common middlewares (anti-forgery and auth) change this behavior, so you may only see this difference if _not_ using one of those.

#### Overall
- In the System.Web & ASP.NET MVC 5 world, [the default cache header is `Cache-Control: private`](https://referencesource.microsoft.com/#System.Web/HttpCachePolicy.cs,812).
- In .NET Core, the default is no `Cache-Control` header at all.

#### User Story
We found out about this in a fairly odd way. We redirect users who don't meet a certain criteria on a certain route with a 302, just a normal `Redirect()`. It took us a little while to realize our CDN/DDoS proxy was caching the response. This proxy is Fastly, not some random setup. I think it's worth noting the cache driving the developer crazy was _over an hour old_ - which is perfectly legal with `private` and no TTL specified.

After asking on Twitter ([thread here](https://twitter.com/Nick_Craver/status/1237795895359483905)), we were surprised to find out this just wasn't considered (as far as anyone knows) in the .NET Core ground-up design. But for anyone migrating from full framework to .NET Core, this is going to be a fun surprise. It took us several apps before we noticed it. _Now_ we have tests in place but this just isn't something we thought to be checking and took that old .NET 4.x behavior for granted.

Logically my brain says _of course_ you wouldn't cache a 302 or a 401, or a multitude of other things but...that's not actually a universal and the fact it would surprises me. I bet it surprises others.

It can easily introduce a security issue for anyone porting as well, as with anything going from default private to default public. I'm not saying this makes it a critical factor, but it should be considered.

#### Suggestions
I'm not arguing for a default change...that's potentially breaking too. I'm just here to poke the proverbial bear and make suggestions.

We just wrote middleware for adding the header in Kestrel - **perhaps such a thing should be in-box?** I think an easy way to opt-in to what was such a fundamental behavior for compat/expectation reasons is a low-ish non-breaking bar. It'd make migration easier for others doing this.

At the very least, I think this should be called out in the migration guidance. If it is already, I apologize - I looked for a while today and couldn't find it. I hope our upcoming blog posts on migration will help relay some of these issues and add to the docs for the better, but I thought this issue in particular deserved it's own discussion since it was a surprise or unknown to so many people.

cc @rynowak @GrabYourPitchforks @DamianEdwards @davidfowl

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.