[Fixes #2086] FilePathResult WriteFileAsync uses SendFile feature incorrectly

src/Microsoft.AspNet.Mvc.Core/ActionResults/FilePathResult.cs

@@ -80,27 +80,23 @@ namespace Microsoft.AspNet.Mvc
         /// <inheritdoc />
         protected override Task WriteFileAsync(HttpResponse response, CancellationToken cancellation)
         {
-            var sendFile = response.HttpContext.GetFeature<IHttpSendFileFeature>();
-
             var fileProvider = GetFileProvider(response.HttpContext.RequestServices);
 
-            var filePath = ResolveFilePath(fileProvider);
+            var resolveFilePathResult = ResolveFilePath(fileProvider);
 
-            if (sendFile != null)
+            if (resolveFilePathResult.PhysicalFilePath != null)
             {
-                return sendFile.SendFileAsync(
+                return CopyPhysicalFileToResponseAsync(response, resolveFilePathResult.PhysicalFilePath, cancellation);
-                    filePath,
-                    offset: 0,
-                    length: null,
-                    cancellation: 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(
+        private Task CopyPhysicalFileToResponseAsync(
-            string fileName,
+            HttpResponse response,
+            string physicalFilePath,
+            CancellationToken cancellationToken)
+        {
+            var sendFile = response.HttpContext.GetFeature<IHttpSendFileFeature>();
+            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(
+            using (sourceStream)
-                fileName, FileMode.Open,
-                FileAccess.Read,
-                FileShare.ReadWrite,
-                DefaultBufferSize,
-                FileOptions.Asynchronous | FileOptions.SequentialScan);
-
-            using (fileStream)
             {
-                await fileStream.CopyToAsync(response.Body, DefaultBufferSize, cancellation);
+                await sourceStream.CopyToAsync(response.Body, DefaultBufferSize, cancellation);
             }
         }
     }
-}
+
+    internal class ResolveFilePathResult
+    {
+        public IFileInfo FileInfo { get; set; }
+
+        public string PhysicalFilePath { get; set; }
+    }
+}

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<IFileInfo>();
+            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<IFileProvider>();
+            nonDiskFileProvider.Setup(fp => fp.GetFileInfo(It.IsAny<string>())).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]

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);
+        }
     }
 }

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"
+            };
+        }
+    }
+}

test/WebSites/FilesWebSite/EmbeddedResources/Greetings.txt

@@ -0,0 +1 @@
+Sample text file as embedded resource.

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-*",