Cache-Control header: What should the default or easier opt-ins be?
- 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
Assessment
This issue has not been assessed yet.