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 134b882b99..f20675f9be 100644 --- a/src/Http/Http.Extensions/test/SendFileResponseExtensionsTests.cs +++ b/src/Http/Http.Extensions/test/SendFileResponseExtensionsTests.cs @@ -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.IO.Pipelines; using System.Threading; @@ -28,18 +29,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(); @@ -57,10 +126,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 3de3576e76..f3864324bf 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,28 +341,15 @@ namespace Microsoft.AspNetCore.StaticFiles { SetCompressionMode(); ApplyResponseHeaders(Constants.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. + // However, if it was canceled for any other reason we need to prevent empty responses. _context.Abort(); } } @@ -390,31 +373,17 @@ namespace Microsoft.AspNetCore.StaticFiles _response.ContentLength = length; SetCompressionMode(); ApplyResponseHeaders(Constants.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. + // However, if it was canceled for any other reason we need to prevent empty responses. _context.Abort(); } } 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);