diff --git a/src/Http/Http.Extensions/src/SendFileResponseExtensions.cs b/src/Http/Http.Extensions/src/SendFileResponseExtensions.cs index e7fd608c05..8dfae44bdc 100644 --- a/src/Http/Http.Extensions/src/SendFileResponseExtensions.cs +++ b/src/Http/Http.Extensions/src/SendFileResponseExtensions.cs @@ -16,6 +16,8 @@ namespace Microsoft.AspNetCore.Http /// public static class SendFileResponseExtensions { + private const int StreamCopyBufferSize = 64 * 1024; + /// /// Sends the given file using the SendFile extension. /// @@ -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(); - 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) diff --git a/src/Http/Http.Extensions/test/Microsoft.AspNetCore.Http.Extensions.Tests.csproj b/src/Http/Http.Extensions/test/Microsoft.AspNetCore.Http.Extensions.Tests.csproj index 19bf5e753a..fffeeb054b 100644 --- a/src/Http/Http.Extensions/test/Microsoft.AspNetCore.Http.Extensions.Tests.csproj +++ b/src/Http/Http.Extensions/test/Microsoft.AspNetCore.Http.Extensions.Tests.csproj @@ -4,6 +4,10 @@ $(DefaultNetCoreTargetFramework) + + + + diff --git a/src/Http/Http.Extensions/test/SendFileResponseExtensionsTests.cs b/src/Http/Http.Extensions/test/SendFileResponseExtensionsTests.cs index 5f5a6341f8..54e22811c0 100644 --- a/src/Http/Http.Extensions/test/SendFileResponseExtensionsTests.cs +++ b/src/Http/Http.Extensions/test/SendFileResponseExtensionsTests.cs @@ -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( + () => 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(fakeFeature); + var response = context.Response; + + await Assert.ThrowsAsync( + () => 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(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); } diff --git a/src/Http/Http.Extensions/test/testfile1kb.txt b/src/Http/Http.Extensions/test/testfile1kb.txt new file mode 100644 index 0000000000..61d982da1a --- /dev/null +++ b/src/Http/Http.Extensions/test/testfile1kb.txt @@ -0,0 +1 @@ +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa diff --git a/src/Middleware/StaticFiles/src/StaticFileContext.cs b/src/Middleware/StaticFiles/src/StaticFileContext.cs index 1d39b42c2e..540088e5fc 100644 --- a/src/Middleware/StaticFiles/src/StaticFileContext.cs +++ b/src/Middleware/StaticFiles/src/StaticFileContext.cs @@ -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(); - 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(); - 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); } } diff --git a/src/Middleware/StaticFiles/test/UnitTests/StaticFileContextTest.cs b/src/Middleware/StaticFiles/test/UnitTests/StaticFileContextTest.cs index efea2a08e5..409cda5b5b 100644 --- a/src/Middleware/StaticFiles/test/UnitTests/StaticFileContextTest.cs +++ b/src/Middleware/StaticFiles/test/UnitTests/StaticFileContextTest.cs @@ -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 _files = new Dictionary(StringComparer.Ordinal); diff --git a/src/Servers/HttpSys/test/FunctionalTests/ResponseSendFileTests.cs b/src/Servers/HttpSys/test/FunctionalTests/ResponseSendFileTests.cs index 1bda0ddee1..8d54a67a53 100644 --- a/src/Servers/HttpSys/test/FunctionalTests/ResponseSendFileTests.cs +++ b/src/Servers/HttpSys/test/FunctionalTests/ResponseSendFileTests.cs @@ -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(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(() => - 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(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); } });