From 31e42ee31286fe115c75336343a6cef68729f4d7 Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Tue, 17 Nov 2015 09:32:05 -0800 Subject: [PATCH] [Fixes #3433] Invalid media type 'text/plain; charset=utf-8' --- samples/MvcSandbox/project.json | 2 +- .../FileContentResult.cs | 2 +- src/Microsoft.AspNet.Mvc.Core/FileResult.cs | 16 +--- .../FileStreamResult.cs | 2 +- .../PhysicalFileResult.cs | 2 +- .../VirtualFileResult.cs | 2 +- .../Controller.cs | 14 +++- .../FileContentResultTest.cs | 16 ++++ .../FileResultTest.cs | 4 +- .../FileStreamResultTest.cs | 16 ++++ .../PhysicalFileResultTest.cs | 16 ++++ .../VirtualFileResultTest.cs | 16 ++++ .../ControllerTest.cs | 84 +++++++++++++++++++ 13 files changed, 166 insertions(+), 26 deletions(-) diff --git a/samples/MvcSandbox/project.json b/samples/MvcSandbox/project.json index 0f92301967..ffebb778fb 100644 --- a/samples/MvcSandbox/project.json +++ b/samples/MvcSandbox/project.json @@ -1,6 +1,6 @@ { "commands": { - "web": "Microsoft.AspNet.Hosting server=Microsoft.AspNet.Server.WebListener server.urls=http://localhost:5001", + "web": "Microsoft.AspNet.Server.Kestrel", "kestrel": "Microsoft.AspNet.Hosting --server Microsoft.AspNet.Server.Kestrel --server.urls http://localhost:5000" }, "compilationOptions": { diff --git a/src/Microsoft.AspNet.Mvc.Core/FileContentResult.cs b/src/Microsoft.AspNet.Mvc.Core/FileContentResult.cs index 292928217e..c9745663ba 100644 --- a/src/Microsoft.AspNet.Mvc.Core/FileContentResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/FileContentResult.cs @@ -25,7 +25,7 @@ namespace Microsoft.AspNet.Mvc /// The bytes that represent the file contents. /// The Content-Type header of the response. public FileContentResult(byte[] fileContents, string contentType) - : this(fileContents, new MediaTypeHeaderValue(contentType)) + : this(fileContents, MediaTypeHeaderValue.Parse(contentType)) { if (fileContents == null) { diff --git a/src/Microsoft.AspNet.Mvc.Core/FileResult.cs b/src/Microsoft.AspNet.Mvc.Core/FileResult.cs index c8bc0422d6..04bb55e48a 100644 --- a/src/Microsoft.AspNet.Mvc.Core/FileResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/FileResult.cs @@ -19,20 +19,6 @@ namespace Microsoft.AspNet.Mvc { private string _fileDownloadName; - /// - /// Creates a new instance with - /// the provided . - /// - /// The Content-Type header of the response. - protected FileResult(string contentType) - : this(new MediaTypeHeaderValue(contentType)) - { - if (contentType == null) - { - throw new ArgumentNullException(nameof(contentType)); - } - } - /// /// Creates a new instance with /// the provided . @@ -87,7 +73,7 @@ namespace Microsoft.AspNet.Mvc contentDisposition.SetHttpFileName(FileDownloadName); context.HttpContext.Response.Headers[HeaderNames.ContentDisposition] = contentDisposition.ToString(); } - + logger.FileResultExecuting(FileDownloadName); return WriteFileAsync(response); } diff --git a/src/Microsoft.AspNet.Mvc.Core/FileStreamResult.cs b/src/Microsoft.AspNet.Mvc.Core/FileStreamResult.cs index 91ca79fe6b..8f31fce05c 100644 --- a/src/Microsoft.AspNet.Mvc.Core/FileStreamResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/FileStreamResult.cs @@ -30,7 +30,7 @@ namespace Microsoft.AspNet.Mvc /// The stream with the file. /// The Content-Type header of the response. public FileStreamResult(Stream fileStream, string contentType) - : this(fileStream, new MediaTypeHeaderValue(contentType)) + : this(fileStream, MediaTypeHeaderValue.Parse(contentType)) { } diff --git a/src/Microsoft.AspNet.Mvc.Core/PhysicalFileResult.cs b/src/Microsoft.AspNet.Mvc.Core/PhysicalFileResult.cs index 67c8caca00..6c343d3fef 100644 --- a/src/Microsoft.AspNet.Mvc.Core/PhysicalFileResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/PhysicalFileResult.cs @@ -28,7 +28,7 @@ namespace Microsoft.AspNet.Mvc /// The path to the file. The path must be an absolute path. /// The Content-Type header of the response. public PhysicalFileResult(string fileName, string contentType) - : this(fileName, new MediaTypeHeaderValue(contentType)) + : this(fileName, MediaTypeHeaderValue.Parse(contentType)) { if (fileName == null) { diff --git a/src/Microsoft.AspNet.Mvc.Core/VirtualFileResult.cs b/src/Microsoft.AspNet.Mvc.Core/VirtualFileResult.cs index 3fb664130b..5dd5c302a5 100644 --- a/src/Microsoft.AspNet.Mvc.Core/VirtualFileResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/VirtualFileResult.cs @@ -31,7 +31,7 @@ namespace Microsoft.AspNet.Mvc /// The path to the file. The path must be relative/virtual. /// The Content-Type header of the response. public VirtualFileResult(string fileName, string contentType) - : this(fileName, new MediaTypeHeaderValue(contentType)) + : this(fileName, MediaTypeHeaderValue.Parse(contentType)) { if (fileName == null) { diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/Controller.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/Controller.cs index 79f00a413f..eb5f20ca6e 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/Controller.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/Controller.cs @@ -467,7 +467,7 @@ namespace Microsoft.AspNet.Mvc [NonAction] public virtual ContentResult Content(string content, string contentType) { - return Content(content, contentType, contentEncoding: null); + return Content(content, MediaTypeHeaderValue.Parse(contentType)); } /// @@ -478,10 +478,16 @@ namespace Microsoft.AspNet.Mvc /// The content type (MIME type). /// The content encoding. /// The created object for the response. + /// + /// If encoding is provided by both the 'charset' and the parameters, then + /// the parameter is chosen as the final encoding. + /// [NonAction] public virtual ContentResult Content(string content, string contentType, Encoding contentEncoding) { - return Content(content, new MediaTypeHeaderValue(contentType) { Encoding = contentEncoding }); + var mediaTypeHeaderValue = MediaTypeHeaderValue.Parse(contentType); + mediaTypeHeaderValue.Encoding = contentEncoding ?? mediaTypeHeaderValue.Encoding; + return Content(content, mediaTypeHeaderValue); } /// @@ -639,7 +645,7 @@ namespace Microsoft.AspNet.Mvc } /// - /// Creates a object with + /// Creates a object with /// set to true using the specified . /// /// The local URL to redirect to. @@ -1756,7 +1762,7 @@ namespace Microsoft.AspNet.Mvc { throw new ArgumentNullException(nameof(model)); } - + var modelName = prefix ?? string.Empty; // Clear ModelStateDictionary entries for the model so that it will be re-validated. diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/FileContentResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/FileContentResultTest.cs index cc6508f8dc..3895f81cd8 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/FileContentResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/FileContentResultTest.cs @@ -31,6 +31,22 @@ namespace Microsoft.AspNet.Mvc Assert.Same(fileContents, result.FileContents); } + [Fact] + public void Constructor_SetsContentTypeAndParameters() + { + // Arrange + var fileContents = new byte[0]; + var contentType = "text/plain; charset=us-ascii; p1=p1-value"; + var expectedMediaType = MediaTypeHeaderValue.Parse(contentType); + + // Act + var result = new FileContentResult(fileContents, contentType); + + // Assert + Assert.Same(fileContents, result.FileContents); + Assert.Equal(expectedMediaType, result.ContentType); + } + [Fact] public async Task WriteFileAsync_CopiesBuffer_ToOutputStream() { diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/FileResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/FileResultTest.cs index 99c246d627..2951a83a19 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/FileResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/FileResultTest.cs @@ -275,12 +275,12 @@ namespace Microsoft.AspNet.Mvc public bool WasWriteFileCalled; public EmptyFileResult() - : this(MediaTypeNames.Application.Octet) + : base(MediaTypeHeaderValue.Parse(MediaTypeNames.Application.Octet)) { } public EmptyFileResult(string contentType) - : base(contentType) + : base(MediaTypeHeaderValue.Parse(contentType)) { } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/FileStreamResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/FileStreamResultTest.cs index 4058176a15..3e765b8d75 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/FileStreamResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/FileStreamResultTest.cs @@ -35,6 +35,22 @@ namespace Microsoft.AspNet.Mvc Assert.Equal(stream, result.FileStream); } + [Fact] + public void Constructor_SetsContentTypeAndParameters() + { + // Arrange + var stream = Stream.Null; + var contentType = "text/plain; charset=us-ascii; p1=p1-value"; + var expectedMediaType = MediaTypeHeaderValue.Parse(contentType); + + // Act + var result = new FileStreamResult(stream, contentType); + + // Assert + Assert.Equal(stream, result.FileStream); + Assert.Equal(expectedMediaType, result.ContentType); + } + [Fact] public async Task WriteFileAsync_WritesResponse_InChunksOfFourKilobytes() { diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/PhysicalFileResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/PhysicalFileResultTest.cs index 7322c1e5f6..00884f2eb3 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/PhysicalFileResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/PhysicalFileResultTest.cs @@ -35,6 +35,22 @@ namespace Microsoft.AspNet.Mvc Assert.Equal(path, result.FileName); } + [Fact] + public void Constructor_SetsContentTypeAndParameters() + { + // Arrange + var path = Path.GetFullPath("helllo.txt"); + var contentType = "text/plain; charset=us-ascii; p1=p1-value"; + var expectedMediaType = MediaTypeHeaderValue.Parse(contentType); + + // Act + var result = new PhysicalFileResult(path, contentType); + + // Assert + Assert.Equal(path, result.FileName); + Assert.Equal(expectedMediaType, result.ContentType); + } + [Fact] public async Task ExecuteResultAsync_FallsbackToStreamCopy_IfNoIHttpSendFilePresent() { diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/VirtualFileResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/VirtualFileResultTest.cs index df0e20d1ab..557f1e95ef 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/VirtualFileResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/VirtualFileResultTest.cs @@ -36,6 +36,22 @@ namespace Microsoft.AspNet.Mvc Assert.Equal(path, result.FileName); } + [Fact] + public void Constructor_SetsContentTypeAndParameters() + { + // Arrange + var path = Path.GetFullPath("helllo.txt"); + var contentType = "text/plain; charset=us-ascii; p1=p1-value"; + var expectedMediaType = MediaTypeHeaderValue.Parse(contentType); + + // Act + var result = new VirtualFileResult(path, contentType); + + // Assert + Assert.Equal(path, result.FileName); + Assert.Equal(expectedMediaType, result.ContentType); + + } [Fact] public async Task ExecuteResultAsync_FallsBackToWebRootFileProvider_IfNoFileProviderIsPresent() { diff --git a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ControllerTest.cs b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ControllerTest.cs index 2a7d2d36fe..754958d94d 100644 --- a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ControllerTest.cs +++ b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ControllerTest.cs @@ -18,6 +18,7 @@ using Microsoft.AspNet.Mvc.ModelBinding.Validation; using Microsoft.AspNet.Mvc.ViewFeatures; using Microsoft.AspNet.Routing; using Microsoft.AspNet.Testing; +using Microsoft.Net.Http.Headers; using Moq; using Newtonsoft.Json; using Xunit; @@ -1107,6 +1108,66 @@ namespace Microsoft.AspNet.Mvc.Test Assert.Equal("text/plain; charset=utf-8", actualContentResult.ContentType.ToString()); } + [Fact] + public void Controller_Content_NoContentType_DefaultEncodingIsUsed() + { + // Arrange + var contentController = new ContentController(); + var expected = MediaTypeHeaderValue.Parse("text/plain; charset=utf-8"); + + // Act + var contentResult = (ContentResult)contentController.Content_WithNoEncoding(); + + // Assert + // The default content type of ContentResult is used when the result is executed. + Assert.Null(contentResult.ContentType); + } + + [Fact] + public void Controller_Content_InvalidCharset_DefaultEncodingIsUsed() + { + // Arrange + var contentController = new ContentController(); + var contentType = "text/xml; charset=invalid; p1=p1-value"; + + // Act + var contentResult = (ContentResult)contentController.Content_WithInvalidCharset(); + + // Assert + Assert.NotNull(contentResult.ContentType); + Assert.Equal(contentType, contentResult.ContentType.ToString()); + // The default encoding of ContentResult is used when this result is executed. + Assert.Null(contentResult.ContentType.Encoding); + } + + [Fact] + public void Controller_Content_CharsetAndEncodingProvided_EncodingIsUsed() + { + // Arrange + var contentController = new ContentController(); + var contentType = MediaTypeHeaderValue.Parse("text/xml; charset=us-ascii; p1=p1-value"); + + // Act + var contentResult = (ContentResult)contentController.Content_WithEncodingInCharset_AndEncodingParameter(); + + // Assert + Assert.Equal(contentType, contentResult.ContentType); + } + + [Fact] + public void Controller_Content_CharsetInContentType_IsUsedForEncoding() + { + // Arrange + var contentController = new ContentController(); + var contentType = MediaTypeHeaderValue.Parse("text/xml; charset=us-ascii; p1=p1-value"); + + // Act + var contentResult = (ContentResult)contentController.Content_WithEncodingInCharset(); + + // Assert + Assert.Equal(contentType, contentResult.ContentType); + } + [Fact] public void Controller_Json_WithParameterValue_SetsResultData() { @@ -1883,6 +1944,29 @@ namespace Microsoft.AspNet.Mvc.Test throw new NotImplementedException(); } } + + private class ContentController : Controller + { + public IActionResult Content_WithNoEncoding() + { + return Content("Hello!!"); + } + + public IActionResult Content_WithEncodingInCharset() + { + return Content("Hello!!", "text/xml; charset=us-ascii; p1=p1-value"); + } + + public IActionResult Content_WithInvalidCharset() + { + return Content("Hello!!", "text/xml; charset=invalid; p1=p1-value"); + } + + public IActionResult Content_WithEncodingInCharset_AndEncodingParameter() + { + return Content("Hello!!", "text/xml; charset=invalid; p1=p1-value", Encoding.ASCII); + } + } } }