Skip to content

Commit 07b8318

Browse files
TratcherTratcher
committed
Merged PR 9042: [3.1] Pass RequestAborted to SendFileAsync
This is the 3.1 version of https://dev.azure.com/dnceng/internal/_git/dotnet-aspnetcore/pullrequest/9014. The concerns in 3.1 are slightly different than prior versions due to a significant redesign in how the response body was handled. In 2.1 very few components implemented IHttpSendFileFeature, and most of those that did would eagerly terminate if the request aborted (e.g. HttpSys server). We mainly had to be concerned about the fallback code that did a copy loop when IHttpSendFileFeature wasn't available. In 3.x the response body Stream, PipeWriter, and SendFileAsync were consolidated onto the new IHttpResponseBodyFeature. Now all servers and component shims support all three ways to send data and we can't make any assumptions about how eagerly they terminate. E.g. many components implemented SendFileAsync using a fallback copy loop, and these components don't have access to RequestAborted to eagerly terminate. This means that in 3.1 we need to pass the RequestAborted token when calling IHttpSendFileFeature.SendFileAsync, as well as any copy loops that have access to the token. I've primarily fixed the HttpResponse.SendFileAsync extension methods and made sure the other affected components call through here. [Infrastructure side note] This commit needs to be rebased on internal/release/3.1 before merging. That branch can't be built locally so I developed this fix based on release/3.1 instead.
1 parent b8a0fa7 commit 07b8318

File tree

6 files changed

+144
-53
lines changed

6 files changed

+144
-53
lines changed

‎src/Http/Http.Extensions/src/SendFileResponseExtensions.cs

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ namespace Microsoft.AspNetCore.Http
1616
/// </summary>
1717
public static class SendFileResponseExtensions
1818
{
19+
private const int StreamCopyBufferSize = 64 * 1024;
20+
1921
/// <summary>
2022
/// Sends the given file using the SendFile extension.
2123
/// </summary>
@@ -110,26 +112,39 @@ private static async Task SendFileAsyncCore(HttpResponse response, IFileInfo fil
110112
if (string.IsNullOrEmpty(file.PhysicalPath))
111113
{
112114
CheckRange(offset, count, file.Length);
115+
using var fileContent = file.CreateReadStream();
116+
117+
var useRequestAborted = !cancellationToken.CanBeCanceled;
118+
var localCancel = useRequestAborted ? response.HttpContext.RequestAborted : cancellationToken;
113119

114-
using (var fileContent = file.CreateReadStream())
120+
try
115121
{
122+
localCancel.ThrowIfCancellationRequested();
116123
if (offset > 0)
117124
{
118125
fileContent.Seek(offset, SeekOrigin.Begin);
119126
}
120-
await StreamCopyOperation.CopyToAsync(fileContent, response.Body, count, cancellationToken);
127+
await StreamCopyOperation.CopyToAsync(fileContent, response.Body, count, StreamCopyBufferSize, localCancel);
121128
}
129+
catch (OperationCanceledException) when (useRequestAborted) { }
122130
}
123131
else
124132
{
125133
await response.SendFileAsync(file.PhysicalPath, offset, count, cancellationToken);
126134
}
127135
}
128136

129-
private static Task SendFileAsyncCore(HttpResponse response, string fileName, long offset, long? count, CancellationToken cancellationToken = default)
137+
private static async Task SendFileAsyncCore(HttpResponse response, string fileName, long offset, long? count, CancellationToken cancellationToken = default)
130138
{
139+
var useRequestAborted = !cancellationToken.CanBeCanceled;
140+
var localCancel = useRequestAborted ? response.HttpContext.RequestAborted : cancellationToken;
131141
var sendFile = response.HttpContext.Features.Get<IHttpResponseBodyFeature>();
132-
return sendFile.SendFileAsync(fileName, offset, count, cancellationToken);
142+
143+
try
144+
{
145+
await sendFile.SendFileAsync(fileName, offset, count, localCancel);
146+
}
147+
catch (OperationCanceledException) when (useRequestAborted) { }
133148
}
134149

135150
private static void CheckRange(long offset, long? count, long fileLength)

‎src/Http/Http.Extensions/test/Microsoft.AspNetCore.Http.Extensions.Tests.csproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
55
</PropertyGroup>
66

7+
<ItemGroup>
8+
<Content Include="testfile1kb.txt" CopyToOutputDirectory="PreserveNewest" />
9+
</ItemGroup>
10+
711
<ItemGroup>
812
<Reference Include="Microsoft.AspNetCore.Http" />
913
<Reference Include="Microsoft.AspNetCore.Http.Extensions" />

‎src/Http/Http.Extensions/test/SendFileResponseExtensionsTests.cs

Lines changed: 83 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Copyright (c) .NET Foundation. All rights reserved. See License.txt in the project root for license information.
22

3+
using System;
34
using System.IO;
45
using System.IO.Pipelines;
56
using System.Threading;
@@ -28,18 +29,86 @@ public async Task SendFileWorks()
2829

2930
await response.SendFileAsync("bob", 1, 3, CancellationToken.None);
3031

31-
Assert.Equal("bob", fakeFeature.name);
32-
Assert.Equal(1, fakeFeature.offset);
33-
Assert.Equal(3, fakeFeature.length);
34-
Assert.Equal(CancellationToken.None, fakeFeature.token);
32+
Assert.Equal("bob", fakeFeature.Name);
33+
Assert.Equal(1, fakeFeature.Offset);
34+
Assert.Equal(3, fakeFeature.Length);
35+
Assert.Equal(CancellationToken.None, fakeFeature.Token);
36+
}
37+
38+
[Fact]
39+
public async Task SendFile_FallsBackToBodyStream()
40+
{
41+
var body = new MemoryStream();
42+
var context = new DefaultHttpContext();
43+
var response = context.Response;
44+
response.Body = body;
45+
46+
await response.SendFileAsync("testfile1kb.txt", 1, 3, CancellationToken.None);
47+
48+
Assert.Equal(3, body.Length);
49+
}
50+
51+
[Fact]
52+
public async Task SendFile_Stream_ThrowsWhenCanceled()
53+
{
54+
var body = new MemoryStream();
55+
var context = new DefaultHttpContext();
56+
var response = context.Response;
57+
response.Body = body;
58+
59+
await Assert.ThrowsAnyAsync<OperationCanceledException>(
60+
() => response.SendFileAsync("testfile1kb.txt", 1, 3, new CancellationToken(canceled: true)));
61+
62+
Assert.Equal(0, body.Length);
63+
}
64+
65+
[Fact]
66+
public async Task SendFile_Feature_ThrowsWhenCanceled()
67+
{
68+
var context = new DefaultHttpContext();
69+
var fakeFeature = new FakeResponseBodyFeature();
70+
context.Features.Set<IHttpResponseBodyFeature>(fakeFeature);
71+
var response = context.Response;
72+
73+
await Assert.ThrowsAsync<OperationCanceledException>(
74+
() => response.SendFileAsync("testfile1kb.txt", 1, 3, new CancellationToken(canceled: true)));
75+
}
76+
77+
[Fact]
78+
public async Task SendFile_Stream_AbortsSilentlyWhenRequestCanceled()
79+
{
80+
var body = new MemoryStream();
81+
var context = new DefaultHttpContext();
82+
context.RequestAborted = new CancellationToken(canceled: true);
83+
var response = context.Response;
84+
response.Body = body;
85+
86+
await response.SendFileAsync("testfile1kb.txt", 1, 3, CancellationToken.None);
87+
88+
Assert.Equal(0, body.Length);
89+
}
90+
91+
[Fact]
92+
public async Task SendFile_Feature_AbortsSilentlyWhenRequestCanceled()
93+
{
94+
var context = new DefaultHttpContext();
95+
var fakeFeature = new FakeResponseBodyFeature();
96+
context.Features.Set<IHttpResponseBodyFeature>(fakeFeature);
97+
var token = new CancellationToken(canceled: true);
98+
context.RequestAborted = token;
99+
var response = context.Response;
100+
101+
await response.SendFileAsync("testfile1kb.txt", 1, 3, CancellationToken.None);
102+
103+
Assert.Equal(token, fakeFeature.Token);
35104
}
36105

37106
private class FakeResponseBodyFeature : IHttpResponseBodyFeature
38107
{
39-
public string name = null;
40-
public long offset = 0;
41-
public long? length = null;
42-
public CancellationToken token;
108+
public string Name { get; set; } = null;
109+
public long Offset { get; set; } = 0;
110+
public long? Length { get; set; } = null;
111+
public CancellationToken Token { get; set; }
43112

44113
public Stream Stream => throw new System.NotImplementedException();
45114

@@ -57,10 +126,12 @@ public void DisableBuffering()
57126

58127
public Task SendFileAsync(string path, long offset, long? length, CancellationToken cancellation)
59128
{
60-
this.name = path;
61-
this.offset = offset;
62-
this.length = length;
63-
this.token = cancellation;
129+
Name = path;
130+
Offset = offset;
131+
Length = length;
132+
Token = cancellation;
133+
134+
cancellation.ThrowIfCancellationRequested();
64135
return Task.FromResult(0);
65136
}
66137

‎src/Http/Http.Extensions/test/testfile1kb.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

‎src/Middleware/StaticFiles/src/StaticFileContext.cs

Lines changed: 6 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,9 @@
55
using System.Diagnostics;
66
using System.IO;
77
using System.Linq;
8-
using System.Threading;
98
using System.Threading.Tasks;
109
using Microsoft.AspNetCore.Builder;
1110
using Microsoft.AspNetCore.Http;
12-
using Microsoft.AspNetCore.Http.Extensions;
1311
using Microsoft.AspNetCore.Http.Features;
1412
using Microsoft.AspNetCore.Http.Headers;
1513
using Microsoft.AspNetCore.Internal;
@@ -21,8 +19,6 @@ namespace Microsoft.AspNetCore.StaticFiles
2119
{
2220
internal struct StaticFileContext
2321
{
24-
private const int StreamCopyBufferSize = 64 * 1024;
25-
2622
private readonly HttpContext _context;
2723
private readonly StaticFileOptions _options;
2824
private readonly HttpRequest _request;
@@ -345,28 +341,15 @@ public async Task SendAsync()
345341
{
346342
SetCompressionMode();
347343
ApplyResponseHeaders(Constants.Status200Ok);
348-
string physicalPath = _fileInfo.PhysicalPath;
349-
var sendFile = _context.Features.Get<IHttpResponseBodyFeature>();
350-
if (sendFile != null && !string.IsNullOrEmpty(physicalPath))
351-
{
352-
// We don't need to directly cancel this, if the client disconnects it will fail silently.
353-
await sendFile.SendFileAsync(physicalPath, 0, _length, CancellationToken.None);
354-
return;
355-
}
356-
357344
try
358345
{
359-
using (var readStream = _fileInfo.CreateReadStream())
360-
{
361-
// Larger StreamCopyBufferSize is required because in case of FileStream readStream isn't going to be buffering
362-
await StreamCopyOperation.CopyToAsync(readStream, _response.Body, _length, StreamCopyBufferSize, _context.RequestAborted);
363-
}
346+
await _context.Response.SendFileAsync(_fileInfo, 0, _length, _context.RequestAborted);
364347
}
365348
catch (OperationCanceledException ex)
366349
{
367350
_logger.WriteCancelled(ex);
368351
// Don't throw this exception, it's most likely caused by the client disconnecting.
369-
// However, if it was cancelled for any other reason we need to prevent empty responses.
352+
// However, if it was canceled for any other reason we need to prevent empty responses.
370353
_context.Abort();
371354
}
372355
}
@@ -390,31 +373,17 @@ internal async Task SendRangeAsync()
390373
_response.ContentLength = length;
391374
SetCompressionMode();
392375
ApplyResponseHeaders(Constants.Status206PartialContent);
393-
394-
string physicalPath = _fileInfo.PhysicalPath;
395-
var sendFile = _context.Features.Get<IHttpResponseBodyFeature>();
396-
if (sendFile != null && !string.IsNullOrEmpty(physicalPath))
397-
{
398-
_logger.SendingFileRange(_response.Headers[HeaderNames.ContentRange], physicalPath);
399-
// We don't need to directly cancel this, if the client disconnects it will fail silently.
400-
await sendFile.SendFileAsync(physicalPath, start, length, CancellationToken.None);
401-
return;
402-
}
403-
404376
try
405377
{
406-
using (var readStream = _fileInfo.CreateReadStream())
407-
{
408-
readStream.Seek(start, SeekOrigin.Begin); // TODO: What if !CanSeek?
409-
_logger.CopyingFileRange(_response.Headers[HeaderNames.ContentRange], SubPath);
410-
await StreamCopyOperation.CopyToAsync(readStream, _response.Body, length, _context.RequestAborted);
411-
}
378+
var logPath = !string.IsNullOrEmpty(_fileInfo.PhysicalPath) ? _fileInfo.PhysicalPath : SubPath;
379+
_logger.SendingFileRange(_response.Headers[HeaderNames.ContentRange], logPath);
380+
await _context.Response.SendFileAsync(_fileInfo, start, length, _context.RequestAborted);
412381
}
413382
catch (OperationCanceledException ex)
414383
{
415384
_logger.WriteCancelled(ex);
416385
// Don't throw this exception, it's most likely caused by the client disconnecting.
417-
// However, if it was cancelled for any other reason we need to prevent empty responses.
386+
// However, if it was canceled for any other reason we need to prevent empty responses.
418387
_context.Abort();
419388
}
420389
}

‎src/Middleware/StaticFiles/test/UnitTests/StaticFileContextTest.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Collections.Generic;
66
using System.IO;
7+
using System.Threading;
78
using System.Threading.Tasks;
89
using Microsoft.AspNetCore.Builder;
910
using Microsoft.AspNetCore.Http;
@@ -120,6 +121,36 @@ public void SkipsHttpsCompression_IfNotMatched()
120121
Assert.Equal(HttpsCompressionMode.Default, httpsCompressionFeature.Mode);
121122
}
122123

124+
[Fact]
125+
public async Task RequestAborted_DoesntThrow()
126+
{
127+
var options = new StaticFileOptions();
128+
var fileProvider = new TestFileProvider();
129+
fileProvider.AddFile("/foo.txt", new TestFileInfo
130+
{
131+
LastModified = new DateTimeOffset(2014, 1, 2, 3, 4, 5, TimeSpan.Zero)
132+
});
133+
var pathString = new PathString("/test");
134+
var httpContext = new DefaultHttpContext();
135+
httpContext.Request.Path = new PathString("/test/foo.txt");
136+
httpContext.RequestAborted = new CancellationToken(canceled: true);
137+
var body = new MemoryStream();
138+
httpContext.Response.Body = body;
139+
var validateResult = StaticFileMiddleware.ValidatePath(httpContext, pathString, out var subPath);
140+
var contentTypeResult = StaticFileMiddleware.LookupContentType(new FileExtensionContentTypeProvider(), options, subPath, out var contentType);
141+
142+
var context = new StaticFileContext(httpContext, options, NullLogger.Instance, fileProvider, contentType, subPath);
143+
144+
var result = context.LookupFileInfo();
145+
Assert.True(validateResult);
146+
Assert.True(contentTypeResult);
147+
Assert.True(result);
148+
149+
await context.SendAsync();
150+
151+
Assert.Equal(0, body.Length);
152+
}
153+
123154
private sealed class TestFileProvider : IFileProvider
124155
{
125156
private readonly Dictionary<string, IFileInfo> _files = new Dictionary<string, IFileInfo>(StringComparer.Ordinal);

0 commit comments

Comments
 (0)