Double-check of FileInfo in SendFileFallback causes issues in templating

[nibblesnbits]

I've written a custom IFileProvider that does in-line replacements of file contents based on the incoming request. This worked back in .NET Core 2.2, but has since broken due to #12328.

The offending lines are below.

aspnetcore/src/Http/Http/src/SendFileFallback.cs

Lines 30 to 39 in 62e157b

	var fileInfo = new FileInfo(filePath);
	if (offset < 0 || offset > fileInfo.Length)
	{
	throw new ArgumentOutOfRangeException(nameof(offset), offset, string.Empty);
	}
	if (count.HasValue &&
	(count.Value < 0 || count.Value > fileInfo.Length - offset))
	{
	throw new ArgumentOutOfRangeException(nameof(count), count, string.Empty);
	}

Is there a new suggested approach if I need to send a modified or different file from a custom IFileProvider? Also, is it stricly necessary to have this check at all?

[Tratcher]

Are you using your custom IFileProvider with the StaticFile middleware? If the file length has been modified then your response has a mismatched Content-Length which can lead to protocol violations and failed responses.

Note the above code path is only hit if your IFileInfo preserves the original PhysicalPath property. If you're modifying the contents in the Stream then you'll want to remove the PhysicalPath property and update the length.

aspnetcore/src/Http/Http.Extensions/src/SendFileResponseExtensions.cs

Lines 115 to 138 in 62e157b

	private static async Task SendFileAsyncCore(HttpResponse response, IFileInfo file, long offset, long? count, CancellationToken cancellationToken)
	{
	if (string.IsNullOrEmpty(file.PhysicalPath))
	{
	CheckRange(offset, count, file.Length);
	using var fileContent = file.CreateReadStream();
	var useRequestAborted = !cancellationToken.CanBeCanceled;
	var localCancel = useRequestAborted ? response.HttpContext.RequestAborted : cancellationToken;
	try
	{
	localCancel.ThrowIfCancellationRequested();
	if (offset > 0)
	{
	fileContent.Seek(offset, SeekOrigin.Begin);
	}
	await StreamCopyOperation.CopyToAsync(fileContent, response.Body, count, StreamCopyBufferSize, localCancel);
	}
	catch (OperationCanceledException) when (useRequestAborted) { }
	}
	else
	{
	await response.SendFileAsync(file.PhysicalPath, offset, count, cancellationToken);

Have you considered doing this kind of inline processing in an MVC controller or custom middleware instead?

[nibblesnbits]

Yes, I am feeding that custom IFileProvider to the StaticFile middleware. I had no idea I could just not return a PhysicalPath! I'll update that and get back to you.

[nibblesnbits]

That totally works!

Would it make sense to put together something for the docs on a task like this? I can throw up a small example repo of my current code to ensure it's not a totally off-the-rails approach if need be.

[Tratcher]

Feel free, I doubt we have much coverage for custom file providers.

[nibblesnbits]

Will do! I'll link my repo here tonight and work on the docs soon.

[nibblesnbits]

Here's a quick-and-dirty demo: https://github.com/nibblesnbits/TemplateFileProviderDemo

[Tratcher]

404? Is it private?

[ghost]

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

[nibblesnbits]

I've updated the visibility. It is public now; my mistake.

[Tratcher]

That's an interesting setup, I'm glad it works for you. I don't know that I'd recommend to the general public integrating dynamic content into the static file provider like that, it breaks some of the assumptions about immutability, cachability, range support, etc.. I hope your files aren't very big or else this would be really expensive to do on demand. You still might want to consider some caching logic like:
A) caching the original file in memory so you don't have to read it off the disk every time.
B) caching some of the templated results by host name so you don't have to keep regenerating them.

One alternative to consider is razor views. If everything is HTML anyways then it might be simpler to turn them into cshtml files and dynamically insert the content that way. That engine should be more efficient for templating than what you're using here.

[nibblesnbits]

That makes sense. The impetus behind this originally was some really strange requirements due to a WordPress setup that's calling a .net core API that also serves up some JavaScript files that I need to update dynamically. It's kind of a mess all the way around, but I learned a lot in the process. So if I tweak this to just be static replacements and maybe throw some minimal caching in front of it with the relevant parts be acceptable for a documentation page?

…

________________________________ From: Chris Ross ***@***.***> Sent: Wednesday, July 21, 2021 5:08:45 PM To: dotnet/aspnetcore ***@***.***> Cc: Stephen Collins ***@***.***>; Author ***@***.***> Subject: Re: [dotnet/aspnetcore] Double-check of FileInfo in SendFileFallback causes issues in templating (#34575) That's an interesting setup, I'm glad it works for you. I don't know that I'd recommend to the general public integrating dynamic content into the static file provider like that, it breaks some of the assumptions about immutability, cachability, range support, etc.. I hope your files aren't very big or else this would be really expensive to do on demand. You still might want to consider some caching logic like: A) caching the original file in memory so you don't have to read it off the disk every time. B) caching some of the templated results by host name so you don't have to keep regenerating them. One alternative to consider is razor views. If everything is HTML anyways then it might be simpler to turn them into cshtml files and dynamically insert the content that way. That engine should be more efficient for templating than what you're using here. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#34575 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABODKJNBXCZ6HPWOGOYO6V3TY4ZN3ANCNFSM5AW6W4JA>.

[ghost]

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.