From 80c7f1bfebbdf4fdfb668d2795724ee29bb47db6 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Wed, 2 Dec 2015 21:29:38 -0800 Subject: [PATCH 1/4] Change SendFileAsync to use a fallback implementation instead of throwing - If the user wants to use the SendFile API directly then they can access the feature explicitly. #496 --- .../project.json | 1 + .../Properties/Resources.Designer.cs | 46 ------- .../Resources.resx | 123 ------------------ .../SendFileResponseExtensions.cs | 52 +++++++- .../StreamCopyOperation.cs | 61 +++++++++ .../SendFileResponseExtensionsTests.cs | 5 +- 6 files changed, 114 insertions(+), 174 deletions(-) delete mode 100644 src/Microsoft.AspNet.Http.Extensions/Properties/Resources.Designer.cs delete mode 100644 src/Microsoft.AspNet.Http.Extensions/Resources.resx create mode 100644 src/Microsoft.AspNet.Http.Extensions/StreamCopyOperation.cs diff --git a/src/Microsoft.AspNet.Http.Abstractions/project.json b/src/Microsoft.AspNet.Http.Abstractions/project.json index 1316a951..d76ed1ff 100644 --- a/src/Microsoft.AspNet.Http.Abstractions/project.json +++ b/src/Microsoft.AspNet.Http.Abstractions/project.json @@ -36,6 +36,7 @@ "System.Net.Primitives": "4.0.11-*", "System.Net.WebSockets": "4.0.0-*", "System.Reflection.TypeExtensions": "4.0.1-*", + "System.IO": "4.0.11-*", "System.Runtime": "4.0.21-*", "System.Runtime.InteropServices": "4.0.21-*", "System.Security.Claims": "4.0.1-*", diff --git a/src/Microsoft.AspNet.Http.Extensions/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Http.Extensions/Properties/Resources.Designer.cs deleted file mode 100644 index a0d5a215..00000000 --- a/src/Microsoft.AspNet.Http.Extensions/Properties/Resources.Designer.cs +++ /dev/null @@ -1,46 +0,0 @@ -// -namespace Microsoft.AspNet.Http.Extensions -{ - using System.Globalization; - using System.Reflection; - using System.Resources; - - internal static class Resources - { - private static readonly ResourceManager _resourceManager - = new ResourceManager("Microsoft.AspNet.Http.Extensions.Resources", typeof(Resources).GetTypeInfo().Assembly); - - /// - /// This server does not support the sendfile.SendAsync extension. - /// - internal static string Exception_SendFileNotSupported - { - get { return GetString("Exception_SendFileNotSupported"); } - } - - /// - /// This server does not support the sendfile.SendAsync extension. - /// - internal static string FormatException_SendFileNotSupported() - { - return GetString("Exception_SendFileNotSupported"); - } - - private static string GetString(string name, params string[] formatterNames) - { - var value = _resourceManager.GetString(name); - - System.Diagnostics.Debug.Assert(value != null); - - if (formatterNames != null) - { - for (var i = 0; i < formatterNames.Length; i++) - { - value = value.Replace("{" + formatterNames[i] + "}", "{" + i + "}"); - } - } - - return value; - } - } -} diff --git a/src/Microsoft.AspNet.Http.Extensions/Resources.resx b/src/Microsoft.AspNet.Http.Extensions/Resources.resx deleted file mode 100644 index 2059135d..00000000 --- a/src/Microsoft.AspNet.Http.Extensions/Resources.resx +++ /dev/null @@ -1,123 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - text/microsoft-resx - - - 2.0 - - - System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - - - System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - - - This server does not support the sendfile.SendAsync extension. - - \ No newline at end of file diff --git a/src/Microsoft.AspNet.Http.Extensions/SendFileResponseExtensions.cs b/src/Microsoft.AspNet.Http.Extensions/SendFileResponseExtensions.cs index 680c77e6..2b91b8a6 100644 --- a/src/Microsoft.AspNet.Http.Extensions/SendFileResponseExtensions.cs +++ b/src/Microsoft.AspNet.Http.Extensions/SendFileResponseExtensions.cs @@ -2,9 +2,9 @@ // 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.Threading; using System.Threading.Tasks; -using Microsoft.AspNet.Http.Extensions; using Microsoft.AspNet.Http.Features; namespace Microsoft.AspNet.Http @@ -33,7 +33,7 @@ public static bool SupportsSendFile(this HttpResponse response) /// Sends the given file using the SendFile extension. /// /// - /// + /// The full or relative path to the file. /// public static Task SendFileAsync(this HttpResponse response, string fileName) { @@ -74,10 +74,56 @@ public static Task SendFileAsync(this HttpResponse response, string fileName, lo var sendFile = response.HttpContext.Features.Get(); if (sendFile == null) { - throw new NotSupportedException(Resources.Exception_SendFileNotSupported); + return SendFileAsync(response.Body, fileName, offset, count, cancellationToken); } return sendFile.SendFileAsync(fileName, offset, count, cancellationToken); } + + // Not safe for overlapped writes. + private static async Task SendFileAsync(Stream outputStream, string fileName, long offset, long? length, CancellationToken cancel) + { + cancel.ThrowIfCancellationRequested(); + + if (string.IsNullOrWhiteSpace(fileName)) + { + throw new ArgumentNullException(nameof(fileName)); + } + if (!File.Exists(fileName)) + { + throw new FileNotFoundException(string.Empty, fileName); + } + + var fileInfo = new FileInfo(fileName); + if (offset < 0 || offset > fileInfo.Length) + { + throw new ArgumentOutOfRangeException(nameof(offset), offset, string.Empty); + } + + if (length.HasValue && + (length.Value < 0 || length.Value > fileInfo.Length - offset)) + { + throw new ArgumentOutOfRangeException(nameof(length), length, string.Empty); + } + + var fileStream = new FileStream( + fileName, + FileMode.Open, + FileAccess.Read, + FileShare.ReadWrite, + bufferSize: 1024 * 64, + options: FileOptions.Asynchronous | FileOptions.SequentialScan); + + try + { + fileStream.Seek(offset, SeekOrigin.Begin); + + await StreamCopyOperation.CopyToAsync(fileStream, outputStream, length, cancel); + } + finally + { + fileStream.Dispose(); + } + } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Http.Extensions/StreamCopyOperation.cs b/src/Microsoft.AspNet.Http.Extensions/StreamCopyOperation.cs new file mode 100644 index 00000000..09d85865 --- /dev/null +++ b/src/Microsoft.AspNet.Http.Extensions/StreamCopyOperation.cs @@ -0,0 +1,61 @@ +// 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.Diagnostics; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.AspNet.Http +{ + // FYI: In most cases the source will be a FileStream and the destination will be to the network. + internal static class StreamCopyOperation + { + private const int DefaultBufferSize = 1024 * 16; + + internal static async Task CopyToAsync(Stream source, Stream destination, long? length, CancellationToken cancel) + { + long? bytesRemaining = length; + byte[] buffer = new byte[DefaultBufferSize]; + + Debug.Assert(source != null); + Debug.Assert(destination != null); + Debug.Assert(!bytesRemaining.HasValue || bytesRemaining.Value >= 0); + Debug.Assert(buffer != null); + + while (true) + { + // The natural end of the range. + if (bytesRemaining.HasValue && bytesRemaining.Value <= 0) + { + return; + } + + cancel.ThrowIfCancellationRequested(); + + int readLength = buffer.Length; + if (bytesRemaining.HasValue) + { + readLength = (int)Math.Min(bytesRemaining.Value, (long)readLength); + } + int count = await source.ReadAsync(buffer, 0, readLength, cancel); + + if (bytesRemaining.HasValue) + { + bytesRemaining -= count; + } + + // End of the source stream. + if (count == 0) + { + return; + } + + cancel.ThrowIfCancellationRequested(); + + await destination.WriteAsync(buffer, 0, count, cancel); + } + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Http.Extensions.Tests/SendFileResponseExtensionsTests.cs b/test/Microsoft.AspNet.Http.Extensions.Tests/SendFileResponseExtensionsTests.cs index cd16acb0..5f4ef3a9 100644 --- a/test/Microsoft.AspNet.Http.Extensions.Tests/SendFileResponseExtensionsTests.cs +++ b/test/Microsoft.AspNet.Http.Extensions.Tests/SendFileResponseExtensionsTests.cs @@ -1,6 +1,7 @@ // 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; using Microsoft.AspNet.Http.Features; @@ -22,10 +23,10 @@ public void SendFileSupport() } [Fact] - public Task SendFileWhenNotSupported() + public Task SendFileWhenFileNotFoundThrows() { var response = new DefaultHttpContext().Response; - return Assert.ThrowsAsync(() => response.SendFileAsync("foo")); + return Assert.ThrowsAsync(() => response.SendFileAsync("foo")); } [Fact] From 00c342d71f1189fb765cb363421423b42ad52323 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Wed, 2 Dec 2015 21:52:13 -0800 Subject: [PATCH 2/4] CR Feedback - Removed SupportsSendFile - Don't check for existence, let FileStream throw - Updated Doc comments --- .../SendFileResponseExtensions.cs | 23 ++----------------- .../SendFileResponseExtensionsTests.cs | 10 -------- 2 files changed, 2 insertions(+), 31 deletions(-) diff --git a/src/Microsoft.AspNet.Http.Extensions/SendFileResponseExtensions.cs b/src/Microsoft.AspNet.Http.Extensions/SendFileResponseExtensions.cs index 2b91b8a6..87bddee0 100644 --- a/src/Microsoft.AspNet.Http.Extensions/SendFileResponseExtensions.cs +++ b/src/Microsoft.AspNet.Http.Extensions/SendFileResponseExtensions.cs @@ -14,26 +14,11 @@ namespace Microsoft.AspNet.Http /// public static class SendFileResponseExtensions { - /// - /// Checks if the SendFile extension is supported. - /// - /// - /// True if sendfile feature exists in the response. - public static bool SupportsSendFile(this HttpResponse response) - { - if (response == null) - { - throw new ArgumentNullException(nameof(response)); - } - - return response.HttpContext.Features.Get() != null; - } - /// /// Sends the given file using the SendFile extension. /// /// - /// The full or relative path to the file. + /// The full to the file. /// public static Task SendFileAsync(this HttpResponse response, string fileName) { @@ -54,7 +39,7 @@ public static Task SendFileAsync(this HttpResponse response, string fileName) /// Sends the given file using the SendFile extension. /// /// - /// The full or relative path to the file. + /// The full to the file. /// The offset in the file. /// The number of types to send, or null to send the remainder of the file. /// @@ -89,10 +74,6 @@ private static async Task SendFileAsync(Stream outputStream, string fileName, lo { throw new ArgumentNullException(nameof(fileName)); } - if (!File.Exists(fileName)) - { - throw new FileNotFoundException(string.Empty, fileName); - } var fileInfo = new FileInfo(fileName); if (offset < 0 || offset > fileInfo.Length) diff --git a/test/Microsoft.AspNet.Http.Extensions.Tests/SendFileResponseExtensionsTests.cs b/test/Microsoft.AspNet.Http.Extensions.Tests/SendFileResponseExtensionsTests.cs index 5f4ef3a9..53c80517 100644 --- a/test/Microsoft.AspNet.Http.Extensions.Tests/SendFileResponseExtensionsTests.cs +++ b/test/Microsoft.AspNet.Http.Extensions.Tests/SendFileResponseExtensionsTests.cs @@ -12,16 +12,6 @@ namespace Microsoft.AspNet.Http.Extensions.Tests { public class SendFileResponseExtensionsTests { - [Fact] - public void SendFileSupport() - { - var context = new DefaultHttpContext(); - var response = context.Response; - Assert.False(response.SupportsSendFile()); - context.Features.Set(new FakeSendFileFeature()); - Assert.True(response.SupportsSendFile()); - } - [Fact] public Task SendFileWhenFileNotFoundThrows() { From cc1b6c083f8bf61e4967f954c57bff85a8b982c3 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Wed, 2 Dec 2015 21:57:33 -0800 Subject: [PATCH 3/4] Removed error checking in private method --- .../SendFileResponseExtensions.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Microsoft.AspNet.Http.Extensions/SendFileResponseExtensions.cs b/src/Microsoft.AspNet.Http.Extensions/SendFileResponseExtensions.cs index 87bddee0..0177f8c0 100644 --- a/src/Microsoft.AspNet.Http.Extensions/SendFileResponseExtensions.cs +++ b/src/Microsoft.AspNet.Http.Extensions/SendFileResponseExtensions.cs @@ -70,11 +70,6 @@ private static async Task SendFileAsync(Stream outputStream, string fileName, lo { cancel.ThrowIfCancellationRequested(); - if (string.IsNullOrWhiteSpace(fileName)) - { - throw new ArgumentNullException(nameof(fileName)); - } - var fileInfo = new FileInfo(fileName); if (offset < 0 || offset > fileInfo.Length) { From c60c3f0c12f7332eec3e80b3c46b9c3b521bd994 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Wed, 2 Dec 2015 22:18:57 -0800 Subject: [PATCH 4/4] More cleanup - Pass the buffer into StreamCopyOperation - Using a real using instead of try finally. --- .../SendFileResponseExtensions.cs | 15 ++++++++------- .../StreamCopyOperation.cs | 6 +----- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.AspNet.Http.Extensions/SendFileResponseExtensions.cs b/src/Microsoft.AspNet.Http.Extensions/SendFileResponseExtensions.cs index 0177f8c0..7218adde 100644 --- a/src/Microsoft.AspNet.Http.Extensions/SendFileResponseExtensions.cs +++ b/src/Microsoft.AspNet.Http.Extensions/SendFileResponseExtensions.cs @@ -82,23 +82,24 @@ private static async Task SendFileAsync(Stream outputStream, string fileName, lo throw new ArgumentOutOfRangeException(nameof(length), length, string.Empty); } + int bufferSize = 1024 * 16; + var fileStream = new FileStream( fileName, FileMode.Open, FileAccess.Read, FileShare.ReadWrite, - bufferSize: 1024 * 64, + bufferSize: bufferSize, options: FileOptions.Asynchronous | FileOptions.SequentialScan); - try + using (fileStream) { fileStream.Seek(offset, SeekOrigin.Begin); - await StreamCopyOperation.CopyToAsync(fileStream, outputStream, length, cancel); - } - finally - { - fileStream.Dispose(); + // TODO: Use buffer pool + var buffer = new byte[bufferSize]; + + await StreamCopyOperation.CopyToAsync(fileStream, buffer, outputStream, length, cancel); } } } diff --git a/src/Microsoft.AspNet.Http.Extensions/StreamCopyOperation.cs b/src/Microsoft.AspNet.Http.Extensions/StreamCopyOperation.cs index 09d85865..f7e6734c 100644 --- a/src/Microsoft.AspNet.Http.Extensions/StreamCopyOperation.cs +++ b/src/Microsoft.AspNet.Http.Extensions/StreamCopyOperation.cs @@ -12,13 +12,9 @@ namespace Microsoft.AspNet.Http // FYI: In most cases the source will be a FileStream and the destination will be to the network. internal static class StreamCopyOperation { - private const int DefaultBufferSize = 1024 * 16; - - internal static async Task CopyToAsync(Stream source, Stream destination, long? length, CancellationToken cancel) + internal static async Task CopyToAsync(Stream source, byte[] buffer, Stream destination, long? length, CancellationToken cancel) { long? bytesRemaining = length; - byte[] buffer = new byte[DefaultBufferSize]; - Debug.Assert(source != null); Debug.Assert(destination != null); Debug.Assert(!bytesRemaining.HasValue || bytesRemaining.Value >= 0);