Skip to content

Commit

Permalink
Merged PR 9014: [2.1] Cancel SendFile copy loops
Browse files Browse the repository at this point in the history 
There are two places in 2.1 where SendFileAsync falls back to a copy loop. These loops should short circuit when the client disconnects, or else the server will sit there and burn resources reading the whole file from disk.

Fix: If you passed in your own active CT we'll use it. Otherwise we'll use the RequestAborted token.
  • Loading branch information
@Tratcher
Tratcher committed Jul 16, 2020
1 parent cb4e0ce commit bccd1ea
Show file tree
Hide file tree
Showing 9 changed files with 200 additions and 33 deletions.
2 changes: 2 additions & 0 deletions eng/PatchConfig.props
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ Later on, this will be checked using this condition:
</PropertyGroup>
<PropertyGroup Condition=" '$(VersionPrefix)' == '2.1.21' ">
<PackagesInPatch>
Microsoft.AspNetCore.Http.Extensions;
Microsoft.AspNetCore.ResponseCompression;
</PackagesInPatch>
</PropertyGroup>
</Project>
71 changes: 44 additions & 27 deletions src/Http/Http.Extensions/src/SendFileResponseExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,41 +107,28 @@ public static Task SendFileAsync(this HttpResponse response, string fileName, lo

private static async Task SendFileAsyncCore(HttpResponse response, IFileInfo file, long offset, long? count, CancellationToken cancellationToken)
{
if (string.IsNullOrEmpty(file.PhysicalPath))
if (!string.IsNullOrEmpty(file.PhysicalPath))
{
CheckRange(offset, count, file.Length);

using (var fileContent = file.CreateReadStream())
{
if (offset > 0)
{
fileContent.Seek(offset, SeekOrigin.Begin);
}
await StreamCopyOperation.CopyToAsync(fileContent, response.Body, count, cancellationToken);
}
await response.SendFileAsync(file.PhysicalPath, offset, count, cancellationToken);
return;
}
else

CheckRange(offset, count, file.Length);
using (var fileContent = file.CreateReadStream())
{
await response.SendFileAsync(file.PhysicalPath, offset, count, cancellationToken);
await SendStreamAsync(fileContent, response, offset, count, cancellationToken);
}
}

private static Task SendFileAsyncCore(HttpResponse response, string fileName, long offset, long? count, CancellationToken cancellationToken = default)
private static async Task SendFileAsyncCore(HttpResponse response, string fileName, long offset, long? count, CancellationToken cancellationToken = default)
{
var sendFile = response.HttpContext.Features.Get<IHttpSendFileFeature>();
if (sendFile == null)
if (sendFile != null)
{
return SendFileAsyncCore(response.Body, fileName, offset, count, cancellationToken);
await sendFile.SendFileAsync(fileName, offset, count, cancellationToken);
return;
}

return sendFile.SendFileAsync(fileName, offset, count, cancellationToken);
}

// Not safe for overlapped writes.
private static async Task SendFileAsyncCore(Stream outputStream, string fileName, long offset, long? count, CancellationToken cancel = default)
{
cancel.ThrowIfCancellationRequested();

var fileInfo = new FileInfo(fileName);
CheckRange(offset, count, fileInfo.Length);

Expand All @@ -155,14 +142,44 @@ private static async Task SendFileAsyncCore(Stream outputStream, string fileName
options: FileOptions.Asynchronous | FileOptions.SequentialScan);

using (fileStream)
{
await SendStreamAsync(fileStream, response, offset, count, cancellationToken);
}
}

private static Task SendStreamAsync(Stream source, HttpResponse response, long offset, long? count, CancellationToken cancellationToken)
{
if (!cancellationToken.CanBeCanceled)
{
return SendStreamQuietAsync(source, response, offset, count, response.HttpContext.RequestAborted);
}

cancellationToken.ThrowIfCancellationRequested();
if (offset > 0)
{
source.Seek(offset, SeekOrigin.Begin);
}

return StreamCopyOperation.CopyToAsync(source, response.Body, count, cancellationToken);
}

private static async Task SendStreamQuietAsync(Stream source, HttpResponse response, long offset, long? count, CancellationToken cancellationToken)
{
if (cancellationToken.IsCancellationRequested)
{
return;
}

try
{
if (offset > 0)
{
fileStream.Seek(offset, SeekOrigin.Begin);
source.Seek(offset, SeekOrigin.Begin);
}

await StreamCopyOperation.CopyToAsync(fileStream, outputStream, count, cancel);
await StreamCopyOperation.CopyToAsync(source, response.Body, count, cancellationToken);
}
catch (OperationCanceledException) { }
}

private static void CheckRange(long offset, long? count, long fileLength)
Expand All @@ -178,4 +195,4 @@ private static void CheckRange(long offset, long? count, long fileLength)
}
}
}
}
}
4 changes: 4 additions & 0 deletions src/Http/Http.Extensions/test/Microsoft.AspNetCore.Http.Extensions.Tests.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
<TargetFrameworks>netcoreapp2.1;net461</TargetFrameworks>
</PropertyGroup>

<ItemGroup>
<Content Include="testfile1kb.txt" CopyToOutputDirectory="PreserveNewest" />
</ItemGroup>

<ItemGroup>
<Reference Include="Microsoft.AspNetCore.Http" />
<Reference Include="Microsoft.AspNetCore.Http.Extensions" />
Expand Down
42 changes: 42 additions & 0 deletions src/Http/Http.Extensions/test/SendFileResponseExtensionsTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright (c) .NET Foundation. All rights reserved. See License.txt in the project root for license information.

using System;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -49,5 +50,46 @@ public Task SendFileAsync(string path, long offset, long? length, CancellationTo
return Task.FromResult(0);
}
}

[Fact]
public async Task SendFile_FallsBackToBodyStream()
{
var body = new MemoryStream();
var context = new DefaultHttpContext();
var response = context.Response;
response.Body = body;

await response.SendFileAsync("testfile1kb.txt", 1, 3, CancellationToken.None);

Assert.Equal(3, body.Length);
}

[Fact]
public async Task SendFile_ThrowsWhenCanceled()
{
var body = new MemoryStream();
var context = new DefaultHttpContext();
var response = context.Response;
response.Body = body;

await Assert.ThrowsAsync<OperationCanceledException>(
() => response.SendFileAsync("testfile1kb.txt", 1, 3, new CancellationToken(canceled: true)));

Assert.Equal(0, body.Length);
}

[Fact]
public async Task SendFile_AbortsSilentlyWhenRequestCanceled()
{
var body = new MemoryStream();
var context = new DefaultHttpContext();
context.RequestAborted = new CancellationToken(canceled: true);
var response = context.Response;
response.Body = body;

await response.SendFileAsync("testfile1kb.txt", 1, 3, CancellationToken.None);

Assert.Equal(0, body.Length);
}
}
}
1 change: 1 addition & 0 deletions src/Http/Http.Extensions/test/testfile1kb.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
30 changes: 30 additions & 0 deletions src/Http/HttpAbstractions.sln
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "RoutingSample.Web", "Routin
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "dependencies", "dependencies", "{793FFE24-138A-4C3D-81AB-18D625E36230}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Server.Kestrel", "..\Servers\Kestrel\Kestrel\src\Microsoft.AspNetCore.Server.Kestrel.csproj", "{C2608BEB-0C4C-4EDB-A0E4-E29AE59B4566}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Server.IISIntegration", "..\Servers\IIS\IISIntegration\src\Microsoft.AspNetCore.Server.IISIntegration.csproj", "{11E37916-24DF-48A3-AFC9-9E9BB76588E4}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -403,6 +407,30 @@ Global
{F4F5D8AF-FBD1-463F-9473-B63AA820A6C4}.Release|x64.Build.0 = Release|Any CPU
{F4F5D8AF-FBD1-463F-9473-B63AA820A6C4}.Release|x86.ActiveCfg = Release|Any CPU
{F4F5D8AF-FBD1-463F-9473-B63AA820A6C4}.Release|x86.Build.0 = Release|Any CPU
{C2608BEB-0C4C-4EDB-A0E4-E29AE59B4566}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{C2608BEB-0C4C-4EDB-A0E4-E29AE59B4566}.Debug|Any CPU.Build.0 = Debug|Any CPU
{C2608BEB-0C4C-4EDB-A0E4-E29AE59B4566}.Debug|x64.ActiveCfg = Debug|Any CPU
{C2608BEB-0C4C-4EDB-A0E4-E29AE59B4566}.Debug|x64.Build.0 = Debug|Any CPU
{C2608BEB-0C4C-4EDB-A0E4-E29AE59B4566}.Debug|x86.ActiveCfg = Debug|Any CPU
{C2608BEB-0C4C-4EDB-A0E4-E29AE59B4566}.Debug|x86.Build.0 = Debug|Any CPU
{C2608BEB-0C4C-4EDB-A0E4-E29AE59B4566}.Release|Any CPU.ActiveCfg = Release|Any CPU
{C2608BEB-0C4C-4EDB-A0E4-E29AE59B4566}.Release|Any CPU.Build.0 = Release|Any CPU
{C2608BEB-0C4C-4EDB-A0E4-E29AE59B4566}.Release|x64.ActiveCfg = Release|Any CPU
{C2608BEB-0C4C-4EDB-A0E4-E29AE59B4566}.Release|x64.Build.0 = Release|Any CPU
{C2608BEB-0C4C-4EDB-A0E4-E29AE59B4566}.Release|x86.ActiveCfg = Release|Any CPU
{C2608BEB-0C4C-4EDB-A0E4-E29AE59B4566}.Release|x86.Build.0 = Release|Any CPU
{11E37916-24DF-48A3-AFC9-9E9BB76588E4}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{11E37916-24DF-48A3-AFC9-9E9BB76588E4}.Debug|Any CPU.Build.0 = Debug|Any CPU
{11E37916-24DF-48A3-AFC9-9E9BB76588E4}.Debug|x64.ActiveCfg = Debug|Any CPU
{11E37916-24DF-48A3-AFC9-9E9BB76588E4}.Debug|x64.Build.0 = Debug|Any CPU
{11E37916-24DF-48A3-AFC9-9E9BB76588E4}.Debug|x86.ActiveCfg = Debug|Any CPU
{11E37916-24DF-48A3-AFC9-9E9BB76588E4}.Debug|x86.Build.0 = Debug|Any CPU
{11E37916-24DF-48A3-AFC9-9E9BB76588E4}.Release|Any CPU.ActiveCfg = Release|Any CPU
{11E37916-24DF-48A3-AFC9-9E9BB76588E4}.Release|Any CPU.Build.0 = Release|Any CPU
{11E37916-24DF-48A3-AFC9-9E9BB76588E4}.Release|x64.ActiveCfg = Release|Any CPU
{11E37916-24DF-48A3-AFC9-9E9BB76588E4}.Release|x64.Build.0 = Release|Any CPU
{11E37916-24DF-48A3-AFC9-9E9BB76588E4}.Release|x86.ActiveCfg = Release|Any CPU
{11E37916-24DF-48A3-AFC9-9E9BB76588E4}.Release|x86.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down Expand Up @@ -435,6 +463,8 @@ Global
{E4AC79A3-625B-421B-9F91-EFCBD9BEB37F} = {24D19E8E-25FD-4C0B-8865-697878B67BE0}
{BF8DC0FF-96F9-4705-8CFA-F42BE989AB6A} = {793FFE24-138A-4C3D-81AB-18D625E36230}
{F4F5D8AF-FBD1-463F-9473-B63AA820A6C4} = {14A7B3DE-46C8-4245-B0BD-9AFF3795C163}
{C2608BEB-0C4C-4EDB-A0E4-E29AE59B4566} = {793FFE24-138A-4C3D-81AB-18D625E36230}
{11E37916-24DF-48A3-AFC9-9E9BB76588E4} = {793FFE24-138A-4C3D-81AB-18D625E36230}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {85B5E151-2E9D-419C-83DD-0DDCF446C83A}
Expand Down
15 changes: 15 additions & 0 deletions src/Middleware/Middleware.sln
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Server
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "_dependencies", "_dependencies", "{ACA6DDB9-7592-47CE-A740-D15BF307E9E0}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Server.IISIntegration", "..\Servers\IIS\IISIntegration\src\Microsoft.AspNetCore.Server.IISIntegration.csproj", "{F05215CF-F754-47BF-ACED-259C53C8B223}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -1005,6 +1007,18 @@ Global
{260E77CB-800F-4A13-BE92-9CAA097705C2}.Release|x64.Build.0 = Release|Any CPU
{260E77CB-800F-4A13-BE92-9CAA097705C2}.Release|x86.ActiveCfg = Release|Any CPU
{260E77CB-800F-4A13-BE92-9CAA097705C2}.Release|x86.Build.0 = Release|Any CPU
{F05215CF-F754-47BF-ACED-259C53C8B223}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{F05215CF-F754-47BF-ACED-259C53C8B223}.Debug|Any CPU.Build.0 = Debug|Any CPU
{F05215CF-F754-47BF-ACED-259C53C8B223}.Debug|x64.ActiveCfg = Debug|Any CPU
{F05215CF-F754-47BF-ACED-259C53C8B223}.Debug|x64.Build.0 = Debug|Any CPU
{F05215CF-F754-47BF-ACED-259C53C8B223}.Debug|x86.ActiveCfg = Debug|Any CPU
{F05215CF-F754-47BF-ACED-259C53C8B223}.Debug|x86.Build.0 = Debug|Any CPU
{F05215CF-F754-47BF-ACED-259C53C8B223}.Release|Any CPU.ActiveCfg = Release|Any CPU
{F05215CF-F754-47BF-ACED-259C53C8B223}.Release|Any CPU.Build.0 = Release|Any CPU
{F05215CF-F754-47BF-ACED-259C53C8B223}.Release|x64.ActiveCfg = Release|Any CPU
{F05215CF-F754-47BF-ACED-259C53C8B223}.Release|x64.Build.0 = Release|Any CPU
{F05215CF-F754-47BF-ACED-259C53C8B223}.Release|x86.ActiveCfg = Release|Any CPU
{F05215CF-F754-47BF-ACED-259C53C8B223}.Release|x86.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down Expand Up @@ -1086,6 +1100,7 @@ Global
{0186A5D0-6D05-4C19-BB81-E49A51745FFF} = {ACA6DDB9-7592-47CE-A740-D15BF307E9E0}
{17B7BFF6-4E72-410C-B690-02741505500A} = {ACA6DDB9-7592-47CE-A740-D15BF307E9E0}
{260E77CB-800F-4A13-BE92-9CAA097705C2} = {ACA6DDB9-7592-47CE-A740-D15BF307E9E0}
{F05215CF-F754-47BF-ACED-259C53C8B223} = {ACA6DDB9-7592-47CE-A740-D15BF307E9E0}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {83786312-A93B-4BB4-AB06-7C6913A59AFA}
Expand Down
50 changes: 45 additions & 5 deletions src/Middleware/ResponseCompression/src/BodyWrapperStream.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
Expand Down Expand Up @@ -286,10 +286,8 @@ public Task SendFileAsync(string path, long offset, long? count, CancellationTok
return _innerSendFileFeature.SendFileAsync(path, offset, count, cancellation);
}

private async Task InnerSendFileAsync(string path, long offset, long? count, CancellationToken cancellation)
private Task InnerSendFileAsync(string path, long offset, long? count, CancellationToken cancellation)
{
cancellation.ThrowIfCancellationRequested();

var fileInfo = new FileInfo(path);
if (offset < 0 || offset > fileInfo.Length)
{
Expand All @@ -311,9 +309,25 @@ private async Task InnerSendFileAsync(string path, long offset, long? count, Can
bufferSize: bufferSize,
options: FileOptions.Asynchronous | FileOptions.SequentialScan);

if (cancellation.CanBeCanceled)
{
return InnerSendFileLoudAsync(fileStream, offset, count, cancellation);
}

return InnerSendFileQuietAsync(fileStream, offset, count, _context.RequestAborted);
}

private async Task InnerSendFileLoudAsync(Stream fileStream, long offset, long? count, CancellationToken cancellation)
{
using (fileStream)
{
fileStream.Seek(offset, SeekOrigin.Begin);
cancellation.ThrowIfCancellationRequested();

if (offset > 0)
{
fileStream.Seek(offset, SeekOrigin.Begin);
}

await StreamCopyOperation.CopyToAsync(fileStream, _compressionStream, count, cancellation);

if (_autoFlush)
Expand All @@ -322,5 +336,31 @@ private async Task InnerSendFileAsync(string path, long offset, long? count, Can
}
}
}

private async Task InnerSendFileQuietAsync(Stream fileStream, long offset, long? count, CancellationToken cancellation)
{
try
{
if (!cancellation.IsCancellationRequested)
{
if (offset > 0)
{
fileStream.Seek(offset, SeekOrigin.Begin);
}

await StreamCopyOperation.CopyToAsync(fileStream, _compressionStream, count, cancellation);

if (_autoFlush)
{
await _compressionStream.FlushAsync(cancellation);
}
}
}
catch (OperationCanceledException) { }
finally
{
fileStream.Dispose();
}
}
}
}
18 changes: 17 additions & 1 deletion src/Middleware/ResponseCompression/test/BodyWrapperStreamTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
Expand Down Expand Up @@ -90,6 +90,22 @@ public async Task SendFileAsync_IsPassedToUnderlyingStream_WhenDisableResponseBu
Assert.Equal(File.ReadAllBytes(path), memoryStream.ToArray());
}

[Fact]
public async Task SendFileAsync_SkipsSilently_WhenRequestAborted()
{
var memoryStream = new MemoryStream();

var context = new DefaultHttpContext();
context.RequestAborted = new CancellationToken(canceled: true);
var stream = new BodyWrapperStream(context, memoryStream, new MockResponseCompressionProvider(true), null, null);

var path = "testfile1kb.txt";
await stream.SendFileAsync(path, 0, null, CancellationToken.None);
stream.Flush();

Assert.Equal(0, memoryStream.Length);
}

[Theory]
[InlineData(true)]
[InlineData(false)]
Expand Down

0 comments on commit bccd1ea

Please sign in to comment.