Cherry pick preview8 changes into master (#24849)

* Pass RequestAborted to SendFileAsync

* Fix HttpSys tests
This commit is contained in:
Chris Ross 2020-08-12 14:30:04 -07:00 committed by GitHub
parent ffeeb034f5
commit 3b9c16ce28
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 156 additions and 60 deletions

View File

@ -16,6 +16,8 @@ namespace Microsoft.AspNetCore.Http
/// </summary>
public static class SendFileResponseExtensions
{
private const int StreamCopyBufferSize = 64 * 1024;
/// <summary>
/// Sends the given file using the SendFile extension.
/// </summary>
@ -110,15 +112,21 @@ namespace Microsoft.AspNetCore.Http
if (string.IsNullOrEmpty(file.PhysicalPath))
{
CheckRange(offset, count, file.Length);
using var fileContent = file.CreateReadStream();
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, cancellationToken);
await StreamCopyOperation.CopyToAsync(fileContent, response.Body, count, StreamCopyBufferSize, localCancel);
}
catch (OperationCanceledException) when (useRequestAborted) { }
}
else
{
@ -126,10 +134,17 @@ namespace Microsoft.AspNetCore.Http
}
}
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 useRequestAborted = !cancellationToken.CanBeCanceled;
var localCancel = useRequestAborted ? response.HttpContext.RequestAborted : cancellationToken;
var sendFile = response.HttpContext.Features.Get<IHttpResponseBodyFeature>();
return sendFile.SendFileAsync(fileName, offset, count, cancellationToken);
try
{
await sendFile.SendFileAsync(fileName, offset, count, localCancel);
}
catch (OperationCanceledException) when (useRequestAborted) { }
}
private static void CheckRange(long offset, long? count, long fileLength)

View File

@ -4,6 +4,10 @@
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
</PropertyGroup>
<ItemGroup>
<Content Include="testfile1kb.txt" CopyToOutputDirectory="PreserveNewest" />
</ItemGroup>
<ItemGroup>
<Reference Include="Microsoft.AspNetCore.Http" />
<Reference Include="Microsoft.AspNetCore.Http.Extensions" />

View File

@ -1,6 +1,7 @@
// 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;
using System.IO;
using System.IO.Pipelines;
using System.Threading;
@ -29,18 +30,86 @@ namespace Microsoft.AspNetCore.Http.Extensions.Tests
await response.SendFileAsync("bob", 1, 3, CancellationToken.None);
Assert.Equal("bob", fakeFeature.name);
Assert.Equal(1, fakeFeature.offset);
Assert.Equal(3, fakeFeature.length);
Assert.Equal(CancellationToken.None, fakeFeature.token);
Assert.Equal("bob", fakeFeature.Name);
Assert.Equal(1, fakeFeature.Offset);
Assert.Equal(3, fakeFeature.Length);
Assert.Equal(CancellationToken.None, fakeFeature.Token);
}
[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_Stream_ThrowsWhenCanceled()
{
var body = new MemoryStream();
var context = new DefaultHttpContext();
var response = context.Response;
response.Body = body;
await Assert.ThrowsAnyAsync<OperationCanceledException>(
() => response.SendFileAsync("testfile1kb.txt", 1, 3, new CancellationToken(canceled: true)));
Assert.Equal(0, body.Length);
}
[Fact]
public async Task SendFile_Feature_ThrowsWhenCanceled()
{
var context = new DefaultHttpContext();
var fakeFeature = new FakeResponseBodyFeature();
context.Features.Set<IHttpResponseBodyFeature>(fakeFeature);
var response = context.Response;
await Assert.ThrowsAsync<OperationCanceledException>(
() => response.SendFileAsync("testfile1kb.txt", 1, 3, new CancellationToken(canceled: true)));
}
[Fact]
public async Task SendFile_Stream_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);
}
[Fact]
public async Task SendFile_Feature_AbortsSilentlyWhenRequestCanceled()
{
var context = new DefaultHttpContext();
var fakeFeature = new FakeResponseBodyFeature();
context.Features.Set<IHttpResponseBodyFeature>(fakeFeature);
var token = new CancellationToken(canceled: true);
context.RequestAborted = token;
var response = context.Response;
await response.SendFileAsync("testfile1kb.txt", 1, 3, CancellationToken.None);
Assert.Equal(token, fakeFeature.Token);
}
private class FakeResponseBodyFeature : IHttpResponseBodyFeature
{
public string name = null;
public long offset = 0;
public long? length = null;
public CancellationToken token;
public string Name { get; set; } = null;
public long Offset { get; set; } = 0;
public long? Length { get; set; } = null;
public CancellationToken Token { get; set; }
public Stream Stream => throw new System.NotImplementedException();
@ -58,10 +127,12 @@ namespace Microsoft.AspNetCore.Http.Extensions.Tests
public Task SendFileAsync(string path, long offset, long? length, CancellationToken cancellation)
{
this.name = path;
this.offset = offset;
this.length = length;
this.token = cancellation;
Name = path;
Offset = offset;
Length = length;
Token = cancellation;
cancellation.ThrowIfCancellationRequested();
return Task.FromResult(0);
}

View File

@ -0,0 +1 @@
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

View File

@ -5,11 +5,9 @@ using System;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Http.Headers;
using Microsoft.AspNetCore.Internal;
@ -21,8 +19,6 @@ namespace Microsoft.AspNetCore.StaticFiles
{
internal struct StaticFileContext
{
private const int StreamCopyBufferSize = 64 * 1024;
private readonly HttpContext _context;
private readonly StaticFileOptions _options;
private readonly HttpRequest _request;
@ -345,29 +341,14 @@ namespace Microsoft.AspNetCore.StaticFiles
{
SetCompressionMode();
ApplyResponseHeaders(StatusCodes.Status200OK);
string physicalPath = _fileInfo.PhysicalPath;
var sendFile = _context.Features.Get<IHttpResponseBodyFeature>();
if (sendFile != null && !string.IsNullOrEmpty(physicalPath))
{
// We don't need to directly cancel this, if the client disconnects it will fail silently.
await sendFile.SendFileAsync(physicalPath, 0, _length, CancellationToken.None);
return;
}
try
{
using (var readStream = _fileInfo.CreateReadStream())
{
// Larger StreamCopyBufferSize is required because in case of FileStream readStream isn't going to be buffering
await StreamCopyOperation.CopyToAsync(readStream, _response.Body, _length, StreamCopyBufferSize, _context.RequestAborted);
}
await _context.Response.SendFileAsync(_fileInfo, 0, _length, _context.RequestAborted);
}
catch (OperationCanceledException ex)
{
_logger.WriteCancelled(ex);
// Don't throw this exception, it's most likely caused by the client disconnecting.
// However, if it was cancelled for any other reason we need to prevent empty responses.
_context.Abort();
_logger.WriteCancelled(ex);
}
}
@ -391,31 +372,16 @@ namespace Microsoft.AspNetCore.StaticFiles
SetCompressionMode();
ApplyResponseHeaders(StatusCodes.Status206PartialContent);
string physicalPath = _fileInfo.PhysicalPath;
var sendFile = _context.Features.Get<IHttpResponseBodyFeature>();
if (sendFile != null && !string.IsNullOrEmpty(physicalPath))
{
_logger.SendingFileRange(_response.Headers[HeaderNames.ContentRange], physicalPath);
// We don't need to directly cancel this, if the client disconnects it will fail silently.
await sendFile.SendFileAsync(physicalPath, start, length, CancellationToken.None);
return;
}
try
{
using (var readStream = _fileInfo.CreateReadStream())
{
readStream.Seek(start, SeekOrigin.Begin); // TODO: What if !CanSeek?
_logger.CopyingFileRange(_response.Headers[HeaderNames.ContentRange], SubPath);
await StreamCopyOperation.CopyToAsync(readStream, _response.Body, length, _context.RequestAborted);
}
var logPath = !string.IsNullOrEmpty(_fileInfo.PhysicalPath) ? _fileInfo.PhysicalPath : SubPath;
_logger.SendingFileRange(_response.Headers[HeaderNames.ContentRange], logPath);
await _context.Response.SendFileAsync(_fileInfo, start, length, _context.RequestAborted);
}
catch (OperationCanceledException ex)
{
_logger.WriteCancelled(ex);
// Don't throw this exception, it's most likely caused by the client disconnecting.
// However, if it was cancelled for any other reason we need to prevent empty responses.
_context.Abort();
_logger.WriteCancelled(ex);
}
}

View File

@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http;
@ -120,6 +121,36 @@ namespace Microsoft.AspNetCore.StaticFiles
Assert.Equal(HttpsCompressionMode.Default, httpsCompressionFeature.Mode);
}
[Fact]
public async Task RequestAborted_DoesntThrow()
{
var options = new StaticFileOptions();
var fileProvider = new TestFileProvider();
fileProvider.AddFile("/foo.txt", new TestFileInfo
{
LastModified = new DateTimeOffset(2014, 1, 2, 3, 4, 5, TimeSpan.Zero)
});
var pathString = new PathString("/test");
var httpContext = new DefaultHttpContext();
httpContext.Request.Path = new PathString("/test/foo.txt");
httpContext.RequestAborted = new CancellationToken(canceled: true);
var body = new MemoryStream();
httpContext.Response.Body = body;
var validateResult = StaticFileMiddleware.ValidatePath(httpContext, pathString, out var subPath);
var contentTypeResult = StaticFileMiddleware.LookupContentType(new FileExtensionContentTypeProvider(), options, subPath, out var contentType);
var context = new StaticFileContext(httpContext, options, NullLogger.Instance, fileProvider, contentType, subPath);
var result = context.LookupFileInfo();
Assert.True(validateResult);
Assert.True(contentTypeResult);
Assert.True(result);
await context.SendAsync();
Assert.Equal(0, body.Length);
}
private sealed class TestFileProvider : IFileProvider
{
private readonly Dictionary<string, IFileInfo> _files = new Dictionary<string, IFileInfo>(StringComparer.Ordinal);

View File

@ -487,17 +487,21 @@ namespace Microsoft.AspNetCore.Server.HttpSys
try
{
// Note Response.SendFileAsync uses RequestAborted by default. This can cause the response to be disposed
// before it throws an IOException, but there's a race depending on when the disconnect is noticed.
// Passing our own token to skip that.
using var cts = new CancellationTokenSource();
await Assert.ThrowsAsync<IOException>(async () =>
{
// It can take several tries before Send notices the disconnect.
for (int i = 0; i < Utilities.WriteRetryLimit; i++)
{
await httpContext.Response.SendFileAsync(AbsoluteFilePath, 0, null);
await httpContext.Response.SendFileAsync(AbsoluteFilePath, 0, null, cts.Token);
}
});
await Assert.ThrowsAsync<ObjectDisposedException>(() =>
httpContext.Response.SendFileAsync(AbsoluteFilePath, 0, null));
httpContext.Response.SendFileAsync(AbsoluteFilePath, 0, null, cts.Token));
testComplete.SetResult(0);
}
@ -573,9 +577,13 @@ namespace Microsoft.AspNetCore.Server.HttpSys
var testComplete = new TaskCompletionSource<int>(TaskCreationOptions.RunContinuationsAsynchronously);
using (Utilities.CreateHttpServer(out var address, async httpContext =>
{
// Note Response.SendFileAsync uses RequestAborted by default. This can cause the response to be disposed
// before it throws an IOException, but there's a race depending on when the disconnect is noticed.
// Passing our own token to skip that.
using var cts = new CancellationTokenSource();
httpContext.RequestAborted.Register(() => cancellationReceived.SetResult(0));
// First write sends headers
await httpContext.Response.SendFileAsync(AbsoluteFilePath, 0, null);
await httpContext.Response.SendFileAsync(AbsoluteFilePath, 0, null, cts.Token);
firstSendComplete.SetResult(0);
await clientDisconnected.Task;
@ -586,7 +594,7 @@ namespace Microsoft.AspNetCore.Server.HttpSys
// It can take several tries before Write notices the disconnect.
for (int i = 0; i < Utilities.WriteRetryLimit; i++)
{
await httpContext.Response.SendFileAsync(AbsoluteFilePath, 0, null, CancellationToken.None);
await httpContext.Response.SendFileAsync(AbsoluteFilePath, 0, null, cts.Token);
}
});