From 3d247ec028703513ef19c869e018946cdf947dbe Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Thu, 12 Mar 2015 13:46:58 -0700 Subject: [PATCH] [Fixes #2086] FilePathResult WriteFileAsync uses SendFile feature incorrectly --- .../ActionResults/FilePathResult.cs | 84 +++++++++++++------ .../ActionResults/FilePathResultTest.cs | 40 ++++++++- .../FileResultTests.cs | 26 ++++++ .../Controllers/EmbeddedFilesController.cs | 21 +++++ .../EmbeddedResources/Greetings.txt | 1 + test/WebSites/FilesWebSite/project.json | 1 + 6 files changed, 144 insertions(+), 29 deletions(-) create mode 100644 test/WebSites/FilesWebSite/Controllers/EmbeddedFilesController.cs create mode 100644 test/WebSites/FilesWebSite/EmbeddedResources/Greetings.txt diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/FilePathResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/FilePathResult.cs index 5ea501cd90..fcaaabb4e6 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/FilePathResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/FilePathResult.cs @@ -80,27 +80,23 @@ namespace Microsoft.AspNet.Mvc /// protected override Task WriteFileAsync(HttpResponse response, CancellationToken cancellation) { - var sendFile = response.HttpContext.GetFeature(); - var fileProvider = GetFileProvider(response.HttpContext.RequestServices); - var filePath = ResolveFilePath(fileProvider); + var resolveFilePathResult = ResolveFilePath(fileProvider); - if (sendFile != null) + if (resolveFilePathResult.PhysicalFilePath != null) { - return sendFile.SendFileAsync( - filePath, - offset: 0, - length: null, - cancellation: cancellation); + return CopyPhysicalFileToResponseAsync(response, resolveFilePathResult.PhysicalFilePath, cancellation); } else { - return CopyStreamToResponse(filePath, response, cancellation); + // Example: An embedded resource + var sourceStream = resolveFilePathResult.FileInfo.CreateReadStream(); + return CopyStreamToResponseAsync(sourceStream, response, cancellation); } } - internal string ResolveFilePath(IFileProvider fileProvider) + internal ResolveFilePathResult ResolveFilePath(IFileProvider fileProvider) { // Let the file system try to get the file and if it can't, // fallback to trying the path directly unless the path starts with '/'. @@ -109,12 +105,17 @@ namespace Microsoft.AspNet.Mvc var path = NormalizePath(FileName); + // Note that we cannot use 'File.Exists' check as the file could be a non-physical + // file. For example, an embedded resource. if (IsPathRooted(path)) { // The path is absolute // C:\...\file.ext // C:/.../file.ext - return path; + return new ResolveFilePathResult() + { + PhysicalFilePath = path + }; } var fileInfo = fileProvider.GetFileInfo(path); @@ -122,7 +123,12 @@ namespace Microsoft.AspNet.Mvc { // The path is relative and IFileProvider found the file, so return the full // path. - return fileInfo.PhysicalPath; + return new ResolveFilePathResult() + { + // Note that physical path could be null in case of non-disk file (ex: embedded resource) + PhysicalFilePath = fileInfo.PhysicalPath, + FileInfo = fileInfo + }; } // We are absolutely sure the path is relative, and couldn't find the file @@ -208,22 +214,50 @@ namespace Microsoft.AspNet.Mvc return FileProvider; } - private static async Task CopyStreamToResponse( - string fileName, + private Task CopyPhysicalFileToResponseAsync( + HttpResponse response, + string physicalFilePath, + CancellationToken cancellationToken) + { + var sendFile = response.HttpContext.GetFeature(); + if (sendFile != null) + { + return sendFile.SendFileAsync( + physicalFilePath, + offset: 0, + length: null, + cancellation: cancellationToken); + } + else + { + var fileStream = new FileStream( + physicalFilePath, + FileMode.Open, + FileAccess.Read, + FileShare.ReadWrite, + DefaultBufferSize, + FileOptions.Asynchronous | FileOptions.SequentialScan); + + return CopyStreamToResponseAsync(fileStream, response, cancellationToken); + } + } + + private static async Task CopyStreamToResponseAsync( + Stream sourceStream, HttpResponse response, CancellationToken cancellation) { - var fileStream = new FileStream( - fileName, FileMode.Open, - FileAccess.Read, - FileShare.ReadWrite, - DefaultBufferSize, - FileOptions.Asynchronous | FileOptions.SequentialScan); - - using (fileStream) + using (sourceStream) { - await fileStream.CopyToAsync(response.Body, DefaultBufferSize, cancellation); + await sourceStream.CopyToAsync(response.Body, DefaultBufferSize, cancellation); } } } -} \ No newline at end of file + + internal class ResolveFilePathResult + { + public IFileInfo FileInfo { get; set; } + + public string PhysicalFilePath { get; set; } + } +} diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FilePathResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FilePathResultTest.cs index 1089903a9c..e7aefceefb 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FilePathResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FilePathResultTest.cs @@ -12,6 +12,7 @@ using Microsoft.AspNet.Routing; using Microsoft.Framework.DependencyInjection; using Moq; using Xunit; +using System.Text; namespace Microsoft.AspNet.Mvc { @@ -171,6 +172,35 @@ namespace Microsoft.AspNet.Mvc Assert.Equal("FilePathResultTestFile contents", contents); } + [Fact] + public async Task ExecuteResultAsync_WorksWithNonDiskBasedFiles() + { + // Arrange + var httpContext = new DefaultHttpContext(); + httpContext.Response.Body = new MemoryStream(); + var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); + var expectedData = "This is an embedded resource"; + var sourceStream = new MemoryStream(Encoding.UTF8.GetBytes(expectedData)); + var nonDiskFileInfo = new Mock(); + nonDiskFileInfo.SetupGet(fi => fi.Exists).Returns(true); + nonDiskFileInfo.SetupGet(fi => fi.PhysicalPath).Returns(() => null); // set null to indicate non-disk file + nonDiskFileInfo.Setup(fi => fi.CreateReadStream()).Returns(sourceStream); + var nonDiskFileProvider = new Mock(); + nonDiskFileProvider.Setup(fp => fp.GetFileInfo(It.IsAny())).Returns(nonDiskFileInfo.Object); + var filePathResult = new FilePathResult("/SampleEmbeddedFile.txt") + { + FileProvider = nonDiskFileProvider.Object + }; + + // Act + await filePathResult.ExecuteResultAsync(actionContext); + + // Assert + httpContext.Response.Body.Position = 0; + var contents = await new StreamReader(httpContext.Response.Body).ReadToEndAsync(); + Assert.Equal(expectedData, contents); + } + [Theory] // Root of the file system, forward slash and back slash [InlineData("FilePathResultTestFile.txt", "TestFiles/FilePathResultTestFile.txt")] @@ -202,18 +232,20 @@ namespace Microsoft.AspNet.Mvc public void GetFilePath_Resolves_RelativePaths(string path, string relativePathToFile) { // Arrange - var fileProvider = new PhysicalFileProvider(Path.GetFullPath("./TestFiles")); var expectedPath = Path.GetFullPath(Path.Combine(Directory.GetCurrentDirectory(), relativePathToFile)); + var fileProvider = new PhysicalFileProvider(Path.GetFullPath("./TestFiles")); var filePathResult = new FilePathResult(path, "text/plain") { FileProvider = fileProvider, }; - + // Act - var result = filePathResult.ResolveFilePath(fileProvider); + var resolveFilePathResult = filePathResult.ResolveFilePath(fileProvider); // Assert - Assert.Equal(expectedPath, result); + Assert.NotNull(resolveFilePathResult); + Assert.NotNull(resolveFilePathResult.FileInfo); + Assert.Equal(expectedPath, resolveFilePathResult.PhysicalFilePath); } [Theory] diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/FileResultTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/FileResultTests.cs index 09bf80ed31..6ad4a9de4c 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/FileResultTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/FileResultTests.cs @@ -151,5 +151,31 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests Assert.NotNull(contentDisposition); Assert.Equal("attachment; filename=downloadName.txt; filename*=UTF-8''downloadName.txt", contentDisposition); } + + [Fact] + public async Task FileFromEmbeddedResources_ReturnsFileWithFileName() + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName); + var client = server.CreateClient(); + var expectedBody = "Sample text file as embedded resource."; + + // Act + var response = await client.GetAsync("http://localhost/EmbeddedFiles/DownloadFileWithFileName"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + Assert.NotNull(response.Content.Headers.ContentType); + Assert.Equal("text/plain", response.Content.Headers.ContentType.ToString()); + + var body = await response.Content.ReadAsStringAsync(); + Assert.NotNull(body); + Assert.Equal(expectedBody, body); + + var contentDisposition = response.Content.Headers.ContentDisposition.ToString(); + Assert.NotNull(contentDisposition); + Assert.Equal("attachment; filename=downloadName.txt; filename*=UTF-8''downloadName.txt", contentDisposition); + } } } \ No newline at end of file diff --git a/test/WebSites/FilesWebSite/Controllers/EmbeddedFilesController.cs b/test/WebSites/FilesWebSite/Controllers/EmbeddedFilesController.cs new file mode 100644 index 0000000000..e7628f2898 --- /dev/null +++ b/test/WebSites/FilesWebSite/Controllers/EmbeddedFilesController.cs @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.AspNet.FileProviders; +using Microsoft.AspNet.Mvc; + +namespace FilesWebSite +{ + public class EmbeddedFilesController : Controller + { + public IActionResult DownloadFileWithFileName() + { + return new FilePathResult("/Greetings.txt", "text/plain") + { + FileProvider = new EmbeddedFileProvider(GetType().Assembly, "EmbeddedResources"), + FileDownloadName = "downloadName.txt" + }; + } + } +} \ No newline at end of file diff --git a/test/WebSites/FilesWebSite/EmbeddedResources/Greetings.txt b/test/WebSites/FilesWebSite/EmbeddedResources/Greetings.txt new file mode 100644 index 0000000000..6b5d08138b --- /dev/null +++ b/test/WebSites/FilesWebSite/EmbeddedResources/Greetings.txt @@ -0,0 +1 @@ +Sample text file as embedded resource. \ No newline at end of file diff --git a/test/WebSites/FilesWebSite/project.json b/test/WebSites/FilesWebSite/project.json index 3f37089db9..f99d40075e 100644 --- a/test/WebSites/FilesWebSite/project.json +++ b/test/WebSites/FilesWebSite/project.json @@ -3,6 +3,7 @@ "web": "Microsoft.AspNet.Hosting server=Microsoft.AspNet.Server.WebListener server.urls=http://localhost:5001", "kestrel": "Microsoft.AspNet.Hosting --server Kestrel --server.urls http://localhost:5000" }, + "resources": "EmbeddedResources/**", "dependencies": { "Kestrel": "1.0.0-*", "Microsoft.AspNet.Mvc": "6.0.0-*",