dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.38k stars 9.99k forks source link

`Accept-Ranges` should not be returned when output caching is used #45999

Open jonpayne opened 1 year ago

jonpayne commented 1 year ago

Is there an existing issue for this?

Describe the bug

When output caching is used with static content, the Accept-Ranges header is returned, but requests with the Ranges header are not served correctly.

Expected Behavior

The Accept-Ranges header should not be set when output caching is enabled. The Ranges header in requests should be ignored when output caching is enabled.

Steps To Reproduce

This project has a simple repro: https://github.com/jonpayne/AspNetCacheIssue/tree/main/src

Using this example:

curl -s -D - https://localhost:5000/test.txt -H "Range: bytes=0-32"   // returns a range
curl -s -D - https://localhost:5000/test.txt -H "Range: bytes=0-40"   // returns a different range
curl -s -D - https://localhost:5000/test.txt                          // returns the full content
curl -s -D - https://localhost:5000/test.txt -H "Range: bytes=0-32"   // returns the full content (unexpected)
curl -s -D - https://localhost:5000/test.txt -H "Range: bytes=0-40"   // returns the full content (unexpected)

Exceptions (if any)

No response

.NET Version

7.0.101

Anything else?

No response

mitchdenny commented 1 year ago

I've confirmed the behavior that you are reporting here, but I think it is compliant. It is important to observe the response code for the requests above. Based on what you have there, and what I've observed it should look like:

The way that output caching works is that it won't even consider storing the response unless the response code is 200. So in the first two requests it returned a 206 so the output cache wasn't populated and the ranged responses were returned. Then you did a request to get the full file and it returned a 200, so the cache was populated. Then on subsequent requests output caching decided not to observe the Range header (which is compliant).

So whilst the server can process the range header, it decided not to respond with a partial response which the client can figure out by looking at the response code.

mitchdenny commented 1 year ago

@sebastienros I seem to recall an issue/pr related to output caching on static files ... something along the lines of not using output caching on static files becuase OS-level caching of files might be better than loading the static files into the web app memory?

jonpayne commented 1 year ago

I have two concerns here:

mitchdenny commented 1 year ago
  • A simple GET request can make a large change to the behavior and performance of a server. For example, a server could serve small range requests for many hours using very little bandwidth, then a single request for the complete content would prevent any future range requests.

I think this is a fair concern @jonpayne. I would like to get @sebastienros's input though since he has more domain expertise in this area and there may be an explicit reason for the current behavior.

sebastienros commented 1 year ago

Two options I see:

Another option would be to support accept-ranges but I don't think it's useful since the whole response would still be loaded before being trimmed to the client.

mitchdenny commented 1 year ago

I think option 1 makes the most sense. Because output caching of static files is a bit dubious anyway - for a few reasons. First of all we don't know that output caching is going to be that much more efficient than executing the code and relying on any OS level caching of file access.

The other thing is that files that tend to need range support to efficiently download probably need some kind of out of band integrity check. Two different requests would end up grabbing two different versions of the file resulting in a corrupted result. When this occurs, you don't want output caching getting in the road of you getting the updated chunks.

(drawing on some of my experience here from Azure Artifacts and importance of knowing what you are downloading when a single file across multiple requests).

sebastienros commented 1 year ago

Could also be part of the default policy to not cache accept-ranges requests.

jonpayne commented 1 year ago

I detailed my reasons for wanting to cache static content here: https://github.com/dotnet/aspnetcore/issues/45998#issuecomment-1380761392

ghost commented 1 year ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

adityamandaleeka commented 1 year ago

Triage: