From 03625c38afcccf83ef465475041c2d55a8cd3b2f Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Tue, 13 Oct 2015 15:18:11 -0700 Subject: [PATCH] Correct polarity of MediaTypeHeaderValue.IsSubsetOf()` checks and remove one conneg fallback - aspnet/Mvc#3138 part 2/2 - request's Content-Type header must be a subset of what an `IInputFormatter` can consume - `[Consumes]` is similar - what an `IOutputFormatter` produces must be a subset of the request's Accept header - `FormatFilter` and `ObjectResult` are similar - `ObjectResult` no longer falls back to `Content-Type` header if no `Accept` value is acceptable - left `WebApiCompatShim` code alone for consistency with down-level `System.Net.Http.Formatting` - correct tests to match new behaviour - do not test `Accept` values containing a `charset` parameter; that case is not valid WIP: - four test failures; something about comparing media types w/ charset included - why do some localization tests fail in VS? nits: - add `InputFormatterTests` - add / update comments and doc comments - correct xUnit attributes in `ActionResultTest`; odd it doesn't show up in command-line runs --- .../ConsumesAttribute.cs | 10 +- .../Controllers/FilterActionInvoker.cs | 4 +- .../Formatters/FormatFilter.cs | 26 +- .../Formatters/InputFormatter.cs | 9 +- .../Formatters/OutputFormatter.cs | 32 +- src/Microsoft.AspNet.Mvc.Core/ObjectResult.cs | 87 ++---- .../Formatters/InputFormatterTest.cs | 292 ++++++++++++++++++ .../ObjectResultTests.cs | 64 ++-- .../JsonInputFormatterTest.cs | 6 +- .../JsonPatchInputFormatterTest.cs | 4 +- ...ataContractSerializerInputFormatterTest.cs | 6 +- .../XmlSerializerInputFormatterTest.cs | 6 +- .../ActionResultTests.cs | 4 +- .../InputFormatterTests.cs | 3 - .../MvcSampleTests.cs | 2 +- .../RespectBrowserAcceptHeaderTests.cs | 40 ++- .../XmlOutputFormatterTests.cs | 12 +- 17 files changed, 460 insertions(+), 147 deletions(-) create mode 100644 test/Microsoft.AspNet.Mvc.Core.Test/Formatters/InputFormatterTest.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/ConsumesAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/ConsumesAttribute.cs index 177c090e7e..83ef895467 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ConsumesAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ConsumesAttribute.cs @@ -56,10 +56,10 @@ namespace Microsoft.AspNet.Mvc MediaTypeHeaderValue requestContentType = null; MediaTypeHeaderValue.TryParse(context.HttpContext.Request.ContentType, out requestContentType); - // Only execute if this is the last filter before calling the action. - // This ensures that we only run the filter which is closest to the action. + // Confirm the request's content type is more specific than a media type this action supports e.g. OK + // if client sent "text/plain" data and this action supports "text/*". if (requestContentType != null && - !ContentTypes.Any(contentType => contentType.IsSubsetOf(requestContentType))) + !ContentTypes.Any(contentType => requestContentType.IsSubsetOf(contentType))) { context.Result = new UnsupportedMediaTypeResult(); } @@ -102,7 +102,9 @@ namespace Microsoft.AspNet.Mvc return !isActionWithoutConsumeConstraintPresent; } - if (ContentTypes.Any(c => c.IsSubsetOf(requestContentType))) + // Confirm the request's content type is more specific than a media type this action supports e.g. OK + // if client sent "text/plain" data and this action supports "text/*". + if (ContentTypes.Any(contentType => requestContentType.IsSubsetOf(contentType))) { return true; } diff --git a/src/Microsoft.AspNet.Mvc.Core/Controllers/FilterActionInvoker.cs b/src/Microsoft.AspNet.Mvc.Core/Controllers/FilterActionInvoker.cs index 036b681162..34c5a346ac 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Controllers/FilterActionInvoker.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Controllers/FilterActionInvoker.cs @@ -183,7 +183,7 @@ namespace Microsoft.AspNet.Mvc.Controllers _cursor = new FilterCursor(_filters); ActionContext.ModelState.MaxAllowedErrors = _maxModelValidationErrors; - + await InvokeAllAuthorizationFiltersAsync(); // If Authorization Filters return a result, it's a short circuit because @@ -663,7 +663,7 @@ namespace Microsoft.AspNet.Mvc.Controllers "Microsoft.AspNet.Mvc.BeforeActionMethod", new { - actionContext = ActionContext, + actionContext = ActionContext, arguments = _actionExecutingContext.ActionArguments, controller = _actionExecutingContext.Controller }); diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/FormatFilter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/FormatFilter.cs index e1aa8ea74e..8dd2dd8f03 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/FormatFilter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Formatters/FormatFilter.cs @@ -13,7 +13,7 @@ using Microsoft.Net.Http.Headers; namespace Microsoft.AspNet.Mvc.Formatters { /// - /// A filter which will use the format value in the route data or query string to set the content type on an + /// A filter which will use the format value in the route data or query string to set the content type on an /// returned from an action. /// public class FormatFilter : IFormatFilter, IResourceFilter, IResultFilter @@ -48,13 +48,13 @@ namespace Microsoft.AspNet.Mvc.Formatters public MediaTypeHeaderValue ContentType { get; } /// - /// true if the current is active and will execute. + /// true if the current is active and will execute. /// public bool IsActive { get; } /// /// As a , this filter looks at the request and rejects it before going ahead if - /// 1. The format in the request doesnt match any format in the map. + /// 1. The format in the request does not match any format in the map. /// 2. If there is a conflicting producesFilter. /// /// The . @@ -67,7 +67,8 @@ namespace Microsoft.AspNet.Mvc.Formatters if (!IsActive) { - return; // no format specified by user, so the filter is muted + // no format specified by user, so the filter is muted + return; } if (ContentType == null) @@ -77,19 +78,22 @@ namespace Microsoft.AspNet.Mvc.Formatters return; } + // Determine media types this action supports. var responseTypeFilters = context.Filters.OfType(); - var contentTypes = new List(); - + var supportedMediaTypes = new List(); foreach (var filter in responseTypeFilters) { - filter.SetContentTypes(contentTypes); + filter.SetContentTypes(supportedMediaTypes); } - if (contentTypes.Count != 0) + // Check if support is adequate for requested media type. + if (supportedMediaTypes.Count != 0) { - // We need to check if the action can generate the content type the user asked for. If it cannot, exit - // here with not found result. - if (!contentTypes.Any(c => ContentType.IsSubsetOf(c))) + // We need to check if the action can generate the content type the user asked for. That is, treat the + // request's format and IApiResponseMetadataProvider-provided content types similarly to an Accept + // header and an output formatter's SupportedMediaTypes: Confirm action supports a more specific media + // type than requested e.g. OK if "text/*" requested and action supports "text/plain". + if (!supportedMediaTypes.Any(contentType => contentType.IsSubsetOf(ContentType))) { context.Result = new HttpNotFoundResult(); } diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/InputFormatter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/InputFormatter.cs index 55f9c87724..7af84585e0 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/InputFormatter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Formatters/InputFormatter.cs @@ -62,12 +62,17 @@ namespace Microsoft.AspNet.Mvc.Formatters var contentType = context.HttpContext.Request.ContentType; MediaTypeHeaderValue requestContentType; - if (!MediaTypeHeaderValue.TryParse(contentType, out requestContentType)) + if (!MediaTypeHeaderValue.TryParse(contentType, out requestContentType) || requestContentType == null) { return false; } - return SupportedMediaTypes.Any(supportedMediaType => supportedMediaType.IsSubsetOf(requestContentType)); + // Confirm the request's content type is more specific than a media type this formatter supports e.g. OK if + // client sent "text/plain" data and this formatter supports "text/*". + return SupportedMediaTypes.Any(supportedMediaType => + { + return requestContentType.IsSubsetOf(supportedMediaType); + }); } /// diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/OutputFormatter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/OutputFormatter.cs index 9de2716eb2..c1c5331ba5 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/OutputFormatter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Formatters/OutputFormatter.cs @@ -77,6 +77,8 @@ namespace Microsoft.AspNet.Mvc.Formatters { List mediaTypes = null; + // Confirm this formatter supports a more specific media type than requested e.g. OK if "text/*" + // requested and formatter supports "text/plain". Treat contentType like it came from an Accept header. foreach (var mediaType in _supportedMediaTypes) { if (mediaType.IsSubsetOf(contentType)) @@ -119,8 +121,7 @@ namespace Microsoft.AspNet.Mvc.Formatters { var requestCharset = requestContentType.Charset; encoding = SupportedEncodings.FirstOrDefault( - supportedEncoding => - requestCharset.Equals(supportedEncoding.WebName)); + supportedEncoding => requestCharset.Equals(supportedEncoding.WebName)); } } @@ -151,10 +152,11 @@ namespace Microsoft.AspNet.Mvc.Formatters } else { - // Since supportedMedia Type is going to be more specific check if supportedMediaType is a subset - // of the content type which is typically what we get on acceptHeader. - mediaType = SupportedMediaTypes - .FirstOrDefault(supportedMediaType => supportedMediaType.IsSubsetOf(contentType)); + // Confirm this formatter supports a more specific media type than requested e.g. OK if "text/*" + // requested and formatter supports "text/plain". contentType is typically what we got in an Accept + // header. + mediaType = SupportedMediaTypes.FirstOrDefault( + supportedMediaType => supportedMediaType.IsSubsetOf(contentType)); } if (mediaType != null) @@ -201,11 +203,11 @@ namespace Microsoft.AspNet.Mvc.Formatters // Copy the media type as we don't want it to affect the next request selectedMediaType = selectedMediaType.Copy(); - // Not text-based media types will use an encoding/charset - binary formats just ignore it. We want to + // Note text-based media types will use an encoding/charset - binary formats just ignore it. We want to // make this class work with media types that use encodings, and those that don't. // // The default implementation of SelectCharacterEncoding will read from the list of SupportedEncodings - // and will always choose a default encoding if any are supported. So, the only cases where the + // and will always choose a default encoding if any are supported. So, the only cases where the // selectedEncoding can be null are: // // 1). No supported encodings - we assume this is a non-text format @@ -237,21 +239,17 @@ namespace Microsoft.AspNet.Mvc.Formatters if (acceptCharsetHeaders != null && acceptCharsetHeaders.Count > 0) { var sortedAcceptCharsetHeaders = acceptCharsetHeaders - .Where(acceptCharset => - acceptCharset.Quality != HeaderQuality.NoMatch) - .OrderByDescending( - m => m, StringWithQualityHeaderValueComparer.QualityComparer); + .Where(acceptCharset => acceptCharset.Quality != HeaderQuality.NoMatch) + .OrderByDescending(m => m, StringWithQualityHeaderValueComparer.QualityComparer); foreach (var acceptCharset in sortedAcceptCharsetHeaders) { var charset = acceptCharset.Value; if (!string.IsNullOrWhiteSpace(charset)) { - var encoding = SupportedEncodings.FirstOrDefault( - supportedEncoding => - charset.Equals(supportedEncoding.WebName, - StringComparison.OrdinalIgnoreCase) || - charset.Equals("*", StringComparison.Ordinal)); + var encoding = SupportedEncodings.FirstOrDefault(supportedEncoding => + charset.Equals(supportedEncoding.WebName, StringComparison.OrdinalIgnoreCase) || + charset.Equals("*", StringComparison.Ordinal)); if (encoding != null) { return encoding; diff --git a/src/Microsoft.AspNet.Mvc.Core/ObjectResult.cs b/src/Microsoft.AspNet.Mvc.Core/ObjectResult.cs index 0f9f25f42f..6a47ce0618 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ObjectResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ObjectResult.cs @@ -86,17 +86,15 @@ namespace Microsoft.AspNet.Mvc { var logger = formatterContext.HttpContext.RequestServices.GetRequiredService>(); - // Check if any content-type was explicitly set (for example, via ProducesAttribute - // or Url path extension mapping). If yes, then ignore content-negotiation and use this content-type. + // Check if any content-type was explicitly set (for example, via ProducesAttribute + // or URL path extension mapping). If yes, then ignore content-negotiation and use this content-type. if (ContentTypes.Count == 1) { logger.LogVerbose( "Skipped content negotiation as content type '{ContentType}' is explicitly set for the response.", ContentTypes[0]); - return SelectFormatterUsingAnyAcceptableContentType(formatterContext, - formatters, - ContentTypes); + return SelectFormatterUsingAnyAcceptableContentType(formatterContext, formatters, ContentTypes); } var sortedAcceptHeaderMediaTypes = GetSortedAcceptHeaderMediaTypes(formatterContext); @@ -106,11 +104,7 @@ namespace Microsoft.AspNet.Mvc { // Check if we have enough information to do content-negotiation, otherwise get the first formatter // which can write the type. - MediaTypeHeaderValue requestContentType = null; - MediaTypeHeaderValue.TryParse( - formatterContext.HttpContext.Request.ContentType, - out requestContentType); - if (!sortedAcceptHeaderMediaTypes.Any() && requestContentType == null) + if (!sortedAcceptHeaderMediaTypes.Any()) { logger.LogVerbose("No information found on request to perform content negotiation."); @@ -122,25 +116,12 @@ namespace Microsoft.AspNet.Mvc // // 1. Select based on sorted accept headers. - if (sortedAcceptHeaderMediaTypes.Any()) - { - selectedFormatter = SelectFormatterUsingSortedAcceptHeaders( - formatterContext, - formatters, - sortedAcceptHeaderMediaTypes); - } + selectedFormatter = SelectFormatterUsingSortedAcceptHeaders( + formatterContext, + formatters, + sortedAcceptHeaderMediaTypes); - // 2. No formatter was found based on accept headers, fall back on request Content-Type header. - if (selectedFormatter == null && requestContentType != null) - { - selectedFormatter = SelectFormatterUsingAnyAcceptableContentType( - formatterContext, - formatters, - new[] { requestContentType }); - } - - // 3. No formatter was found based on Accept and request Content-Type headers, so - // fallback on type based match. + // 2. No formatter was found based on Accept header. Fallback to type-based match. if (selectedFormatter == null) { logger.LogVerbose("Could not find an output formatter based on content negotiation."); @@ -157,15 +138,15 @@ namespace Microsoft.AspNet.Mvc if (sortedAcceptHeaderMediaTypes.Any()) { // Filter and remove accept headers which cannot support any of the user specified content types. + // That is, confirm this result supports a more specific media type than requested e.g. OK if + // "text/*" requested and result supports "text/plain". var filteredAndSortedAcceptHeaders = sortedAcceptHeaderMediaTypes - .Where(acceptHeader => - ContentTypes.Any(contentType => - contentType.IsSubsetOf(acceptHeader))); + .Where(acceptHeader => ContentTypes.Any(contentType => contentType.IsSubsetOf(acceptHeader))); selectedFormatter = SelectFormatterUsingSortedAcceptHeaders( - formatterContext, - formatters, - filteredAndSortedAcceptHeaders); + formatterContext, + formatters, + filteredAndSortedAcceptHeaders); } if (selectedFormatter == null) @@ -177,9 +158,9 @@ namespace Microsoft.AspNet.Mvc // In any of these cases, if the user has specified content types, // do a last effort to find a formatter which can write any of the user specified content type. selectedFormatter = SelectFormatterUsingAnyAcceptableContentType( - formatterContext, - formatters, - ContentTypes); + formatterContext, + formatters, + ContentTypes); } } @@ -202,9 +183,9 @@ namespace Microsoft.AspNet.Mvc } public virtual IOutputFormatter SelectFormatterUsingSortedAcceptHeaders( - OutputFormatterContext formatterContext, - IEnumerable formatters, - IEnumerable sortedAcceptHeaders) + OutputFormatterContext formatterContext, + IEnumerable formatters, + IEnumerable sortedAcceptHeaders) { IOutputFormatter selectedFormatter = null; foreach (var contentType in sortedAcceptHeaders) @@ -212,8 +193,7 @@ namespace Microsoft.AspNet.Mvc // Loop through each of the formatters and see if any one will support this // mediaType Value. selectedFormatter = formatters.FirstOrDefault( - formatter => - formatter.CanWriteResult(formatterContext, contentType)); + formatter => formatter.CanWriteResult(formatterContext, contentType)); if (selectedFormatter != null) { // we found our match. @@ -225,15 +205,14 @@ namespace Microsoft.AspNet.Mvc } public virtual IOutputFormatter SelectFormatterUsingAnyAcceptableContentType( - OutputFormatterContext formatterContext, - IEnumerable formatters, - IEnumerable acceptableContentTypes) + OutputFormatterContext formatterContext, + IEnumerable formatters, + IEnumerable acceptableContentTypes) { var selectedFormatter = formatters.FirstOrDefault( - formatter => - acceptableContentTypes - .Any(contentType => - formatter.CanWriteResult(formatterContext, contentType))); + formatter => acceptableContentTypes.Any( + contentType => formatter.CanWriteResult(formatterContext, contentType))); + return selectedFormatter; } @@ -264,7 +243,7 @@ namespace Microsoft.AspNet.Mvc if (respectAcceptHeader) { sortedAcceptHeaderMediaTypes = SortMediaTypeHeaderValues(incomingAcceptHeaderMediaTypes) - .Where(header => header.Quality != HeaderQuality.NoMatch); + .Where(header => header.Quality != HeaderQuality.NoMatch); } return sortedAcceptHeaderMediaTypes; @@ -286,9 +265,9 @@ namespace Microsoft.AspNet.Mvc { // Use OrderBy() instead of Array.Sort() as it performs fewer comparisons. In this case the comparisons // are quite expensive so OrderBy() performs better. - return headerValues.OrderByDescending(headerValue => - headerValue, - MediaTypeHeaderValueComparer.QualityComparer); + return headerValues.OrderByDescending( + headerValue => headerValue, + MediaTypeHeaderValueComparer.QualityComparer); } private IEnumerable GetDefaultFormatters(ActionContext context) @@ -302,7 +281,7 @@ namespace Microsoft.AspNet.Mvc .GetRequiredService() .ActionBindingContext; - // In scenarios where there is a resource filter which directly shortcircuits using an ObjectResult. + // In scenarios where there is a resource filter which directly short-circuits using an ObjectResult. // actionBindingContext is not setup yet and is null. if (actionBindingContext == null) { diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/InputFormatterTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/InputFormatterTest.cs new file mode 100644 index 0000000000..dd7fca8b8e --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/InputFormatterTest.cs @@ -0,0 +1,292 @@ +// 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.Threading.Tasks; +using Microsoft.AspNet.Http.Internal; +using Microsoft.AspNet.Mvc.ModelBinding; +using Microsoft.Net.Http.Headers; +using Xunit; + +namespace Microsoft.AspNet.Mvc.Formatters +{ + public class InputFormatterTest + { + private class CatchAllFormatter : TestFormatter + { + public CatchAllFormatter() + { + SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("*/*")); + } + } + + [Theory] + [InlineData("application/mathml-content+xml")] + [InlineData("application/mathml-presentation+xml")] + [InlineData("application/mathml+xml; undefined=ignored")] + [InlineData("application/octet-stream; padding=3")] + [InlineData("application/xml")] + [InlineData("application/xml-dtd; undefined=ignored")] + [InlineData("multipart/mixed; boundary=gc0p4Jq0M2Yt08j34c0p")] + [InlineData("multipart/mixed; boundary=gc0p4Jq0M2Yt08j34c0p; undefined=ignored")] + [InlineData("text/html")] + public void CatchAll_CanRead_ReturnsTrueForSupportedMediaTypes(string requestContentType) + { + // Arrange + var formatter = new CatchAllFormatter(); + var httpContext = new DefaultHttpContext(); + httpContext.Request.ContentType = requestContentType; + var context = new InputFormatterContext( + httpContext, + modelName: string.Empty, + modelState: new ModelStateDictionary(), + modelType: typeof(void)); + + // Act + var result = formatter.CanRead(context); + + // Assert + Assert.True(result); + } + + private class MultipartFormatter : TestFormatter + { + public MultipartFormatter() + { + SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("multipart/*")); + } + } + + [Theory] + [InlineData("multipart/mixed; boundary=gc0p4Jq0M2Yt08j34c0p")] + [InlineData("multipart/mixed; boundary=gc0p4Jq0M2Yt08j34c0p; undefined=ignored")] + public void MultipartFormatter_CanRead_ReturnsTrueForSupportedMediaTypes(string requestContentType) + { + // Arrange + var formatter = new MultipartFormatter(); + var httpContext = new DefaultHttpContext(); + httpContext.Request.ContentType = requestContentType; + var context = new InputFormatterContext( + httpContext, + modelName: string.Empty, + modelState: new ModelStateDictionary(), + modelType: typeof(void)); + + // Act + var result = formatter.CanRead(context); + + // Assert + Assert.True(result); + } + + [Theory] + [InlineData("application/mathml-content+xml")] + [InlineData("application/mathml-presentation+xml")] + [InlineData("application/mathml+xml; undefined=ignored")] + [InlineData("application/octet-stream; padding=3")] + [InlineData("application/xml")] + [InlineData("application/xml-dtd; undefined=ignored")] + [InlineData("text/html")] + public void MultipartFormatter_CanRead_ReturnsFalseForUnsupportedMediaTypes(string requestContentType) + { + // Arrange + var formatter = new MultipartFormatter(); + var httpContext = new DefaultHttpContext(); + httpContext.Request.ContentType = requestContentType; + var context = new InputFormatterContext( + httpContext, + modelName: string.Empty, + modelState: new ModelStateDictionary(), + modelType: typeof(void)); + + // Act + var result = formatter.CanRead(context); + + // Assert + Assert.False(result); + } + + private class MultipartMixedFormatter : TestFormatter + { + public MultipartMixedFormatter() + { + SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("multipart/mixed")); + } + } + + [Theory] + [InlineData("multipart/mixed; boundary=gc0p4Jq0M2Yt08j34c0p")] + [InlineData("multipart/mixed; boundary=gc0p4Jq0M2Yt08j34c0p; undefined=ignored")] + public void MultipartMixedFormatter_CanRead_ReturnsTrueForSupportedMediaTypes(string requestContentType) + { + // Arrange + var formatter = new MultipartMixedFormatter(); + var httpContext = new DefaultHttpContext(); + httpContext.Request.ContentType = requestContentType; + var context = new InputFormatterContext( + httpContext, + modelName: string.Empty, + modelState: new ModelStateDictionary(), + modelType: typeof(void)); + + // Act + var result = formatter.CanRead(context); + + // Assert + Assert.True(result); + } + + [Theory] + [InlineData("application/mathml-content+xml")] + [InlineData("application/mathml-presentation+xml")] + [InlineData("application/mathml+xml; undefined=ignored")] + [InlineData("application/octet-stream; padding=3")] + [InlineData("application/xml")] + [InlineData("application/xml-dtd; undefined=ignored")] + [InlineData("text/html")] + public void MultipartMixedFormatter_CanRead_ReturnsFalseForUnsupportedMediaTypes(string requestContentType) + { + // Arrange + var formatter = new MultipartMixedFormatter(); + var httpContext = new DefaultHttpContext(); + httpContext.Request.ContentType = requestContentType; + var context = new InputFormatterContext( + httpContext, + modelName: string.Empty, + modelState: new ModelStateDictionary(), + modelType: typeof(void)); + + // Act + var result = formatter.CanRead(context); + + // Assert + Assert.False(result); + } + + private class MathMLFormatter : TestFormatter + { + public MathMLFormatter() + { + SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("application/mathml-content+xml")); + SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("application/mathml-presentation+xml")); + SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("application/mathml+xml")); + } + } + + [Theory] + [InlineData("application/mathml-content+xml")] + [InlineData("application/mathml-presentation+xml")] + [InlineData("application/mathml+xml; undefined=ignored")] + public void MathMLFormatter_CanRead_ReturnsTrueForSupportedMediaTypes(string requestContentType) + { + // Arrange + var formatter = new MathMLFormatter(); + var httpContext = new DefaultHttpContext(); + httpContext.Request.ContentType = requestContentType; + var context = new InputFormatterContext( + httpContext, + modelName: string.Empty, + modelState: new ModelStateDictionary(), + modelType: typeof(void)); + + // Act + var result = formatter.CanRead(context); + + // Assert + Assert.True(result); + } + + [Theory] + [InlineData("application/octet-stream; padding=3")] + [InlineData("application/xml")] + [InlineData("application/xml-dtd; undefined=ignored")] + [InlineData("multipart/mixed; boundary=gc0p4Jq0M2Yt08j34c0p")] + [InlineData("multipart/mixed; boundary=gc0p4Jq0M2Yt08j34c0p; undefined=ignored")] + [InlineData("text/html")] + public void MathMLFormatter_CanRead_ReturnsFalseForUnsupportedMediaTypes(string requestContentType) + { + // Arrange + var formatter = new MathMLFormatter(); + var httpContext = new DefaultHttpContext(); + httpContext.Request.ContentType = requestContentType; + var context = new InputFormatterContext( + httpContext, + modelName: string.Empty, + modelState: new ModelStateDictionary(), + modelType: typeof(void)); + + // Act + var result = formatter.CanRead(context); + + // Assert + Assert.False(result); + } + + // IsSubsetOf does not follow XML media type conventions. This formatter does not support "application/*+xml". + private class XmlFormatter : TestFormatter + { + public XmlFormatter() + { + SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("application/xml")); + SupportedMediaTypes.Add(MediaTypeHeaderValue.Parse("text/xml")); + } + } + + [Theory] + [InlineData("application/xml")] + public void XMLFormatter_CanRead_ReturnsTrueForSupportedMediaTypes(string requestContentType) + { + // Arrange + var formatter = new XmlFormatter(); + var httpContext = new DefaultHttpContext(); + httpContext.Request.ContentType = requestContentType; + var context = new InputFormatterContext( + httpContext, + modelName: string.Empty, + modelState: new ModelStateDictionary(), + modelType: typeof(void)); + + // Act + var result = formatter.CanRead(context); + + // Assert + Assert.True(result); + } + + [Theory] + [InlineData("application/mathml-content+xml")] + [InlineData("application/mathml-presentation+xml")] + [InlineData("application/mathml+xml; undefined=ignored")] + [InlineData("application/octet-stream; padding=3")] + [InlineData("application/xml-dtd; undefined=ignored")] + [InlineData("multipart/mixed; boundary=gc0p4Jq0M2Yt08j34c0p")] + [InlineData("multipart/mixed; boundary=gc0p4Jq0M2Yt08j34c0p; undefined=ignored")] + [InlineData("text/html")] + public void XMLFormatter_CanRead_ReturnsFalseForUnsupportedMediaTypes(string requestContentType) + { + // Arrange + var formatter = new XmlFormatter(); + var httpContext = new DefaultHttpContext(); + httpContext.Request.ContentType = requestContentType; + var context = new InputFormatterContext( + httpContext, + modelName: string.Empty, + modelState: new ModelStateDictionary(), + modelType: typeof(void)); + + // Act + var result = formatter.CanRead(context); + + // Assert + Assert.False(result); + } + + private class TestFormatter : InputFormatter + { + public override Task ReadRequestBodyAsync(InputFormatterContext context) + { + throw new NotImplementedException(); + } + } + } +} diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ObjectResultTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ObjectResultTests.cs index c8ca3b9712..bd15f3d5ec 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ObjectResultTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ObjectResultTests.cs @@ -12,7 +12,6 @@ using Microsoft.AspNet.Http.Features; using Microsoft.AspNet.Http.Internal; using Microsoft.AspNet.Mvc.Abstractions; using Microsoft.AspNet.Mvc.Formatters; -using Microsoft.AspNet.Mvc.Formatters.Xml; using Microsoft.AspNet.Mvc.Infrastructure; using Microsoft.AspNet.Routing; using Microsoft.AspNet.Testing.xunit; @@ -133,7 +132,7 @@ namespace Microsoft.AspNet.Mvc var result = new ObjectResult(input); result.ContentTypes = new List(); - result.ContentTypes.Add(MediaTypeHeaderValue.Parse(expectedContentType)); + result.ContentTypes.Add(MediaTypeHeaderValue.Parse("application/json")); // Act await result.ExecuteResultAsync(actionContext); @@ -157,7 +156,7 @@ namespace Microsoft.AspNet.Mvc // Set the content type property explicitly to a single value. var result = new ObjectResult(input); result.ContentTypes = new List(); - result.ContentTypes.Add(MediaTypeHeaderValue.Parse(expectedContentType)); + result.ContentTypes.Add(MediaTypeHeaderValue.Parse("application/json")); // Act await result.ExecuteResultAsync(actionContext); @@ -338,23 +337,23 @@ namespace Microsoft.AspNet.Mvc [InlineData("application/xml")] [InlineData("application/custom")] [InlineData("application/xml;q=1, application/custom;q=0.8")] - public void SelectFormatter_WithNoMatchingAcceptHeadersAndRequestContentType_PicksFormatterBasedOnObjectType - (string acceptHeader) + public void SelectFormatter_WithNoMatchingAcceptHeader_PicksFormatterBasedOnObjectType(string acceptHeader) { - // For no accept headers, - // can write is called twice once for the request Content-Type and once for the type match pass. - // For each additional accept header, it is called once. + // For no Accept headers, CanWriteResult is called once for the type match pass. For each additional Accept + // header, it is called once. // Arrange var acceptHeaderCollection = string.IsNullOrEmpty(acceptHeader) ? - null : MediaTypeHeaderValue.ParseList(new[] { acceptHeader }).ToArray(); + null : + MediaTypeHeaderValue.ParseList(new[] { acceptHeader }).ToArray(); var stream = new MemoryStream(); var httpResponse = new Mock(); httpResponse.SetupProperty(o => o.ContentType); httpResponse.SetupGet(r => r.Body).Returns(stream); - var actionContext = CreateMockActionContext(httpResponse.Object, - requestAcceptHeader: acceptHeader, - requestContentType: "application/xml"); + var actionContext = CreateMockActionContext( + httpResponse.Object, + requestAcceptHeader: acceptHeader, + requestContentType: "application/text"); var input = "testInput"; var result = new ObjectResult(input); var mockCountingFormatter = new Mock(); @@ -365,35 +364,40 @@ namespace Microsoft.AspNet.Mvc Object = input, DeclaredType = typeof(string) }; - var mockCountingSupportedContentType = MediaTypeHeaderValue.Parse("application/text"); - mockCountingFormatter.Setup(o => o.CanWriteResult(context, - It.Is(mth => mth == null))) - .Returns(true); - mockCountingFormatter.Setup(o => o.CanWriteResult(context, mockCountingSupportedContentType)) - .Returns(true); + var requestContentType = MediaTypeHeaderValue.Parse("application/text"); + mockCountingFormatter + .Setup(o => o.CanWriteResult(context, It.Is(mth => mth == null))) + .Returns(true); + mockCountingFormatter + .Setup(o => o.CanWriteResult(context, requestContentType)) + .Returns(true); // Set more than one formatters. The test output formatter throws on write. result.Formatters = new List - { - new CannotWriteFormatter(), - mockCountingFormatter.Object, - }; + { + new CannotWriteFormatter(), + mockCountingFormatter.Object, + }; // Act var formatter = result.SelectFormatter(context, result.Formatters); // Assert Assert.Equal(mockCountingFormatter.Object, formatter); + + // CanWriteResult is called once for the type-based match. mockCountingFormatter.Verify(v => v.CanWriteResult(context, null), Times.Once()); - // CanWriteResult is invoked for the following cases: - // 1. For each accept header present - // 2. Request Content-Type - // 3. Type based match - var callCount = (acceptHeaderCollection == null ? 0 : acceptHeaderCollection.Count()) + 2; - mockCountingFormatter.Verify(v => v.CanWriteResult(context, - It.IsNotIn(mockCountingSupportedContentType)), - Times.Exactly(callCount)); + // CanWriteResult is never called for the request's Content-Type. + mockCountingFormatter.Verify(v => v.CanWriteResult(context, requestContentType), Times.Never()); + + // In total, CanWriteResult is invoked for the following cases: + // 1. For each Accept header present + // 2. Type-based match + var callCount = (acceptHeaderCollection == null ? 0 : acceptHeaderCollection.Count()) + 1; + mockCountingFormatter.Verify( + v => v.CanWriteResult(It.IsAny< OutputFormatterContext>(), It.IsAny()), + Times.Exactly(callCount)); } [Fact] diff --git a/test/Microsoft.AspNet.Mvc.Formatters.Json.Test/JsonInputFormatterTest.cs b/test/Microsoft.AspNet.Mvc.Formatters.Json.Test/JsonInputFormatterTest.cs index e34557e898..5b502b2202 100644 --- a/test/Microsoft.AspNet.Mvc.Formatters.Json.Test/JsonInputFormatterTest.cs +++ b/test/Microsoft.AspNet.Mvc.Formatters.Json.Test/JsonInputFormatterTest.cs @@ -22,10 +22,10 @@ namespace Microsoft.AspNet.Mvc.Formatters [Theory] [InlineData("application/json", true)] - [InlineData("application/*", true)] - [InlineData("*/*", true)] + [InlineData("application/*", false)] + [InlineData("*/*", false)] [InlineData("text/json", true)] - [InlineData("text/*", true)] + [InlineData("text/*", false)] [InlineData("text/xml", false)] [InlineData("application/xml", false)] [InlineData("", false)] diff --git a/test/Microsoft.AspNet.Mvc.Formatters.Json.Test/JsonPatchInputFormatterTest.cs b/test/Microsoft.AspNet.Mvc.Formatters.Json.Test/JsonPatchInputFormatterTest.cs index 5bf672bffc..8b40c2059e 100644 --- a/test/Microsoft.AspNet.Mvc.Formatters.Json.Test/JsonPatchInputFormatterTest.cs +++ b/test/Microsoft.AspNet.Mvc.Formatters.Json.Test/JsonPatchInputFormatterTest.cs @@ -75,8 +75,8 @@ namespace Microsoft.AspNet.Mvc.Formatters [Theory] [InlineData("application/json-patch+json", true)] [InlineData("application/json", false)] - [InlineData("application/*", true)] - [InlineData("*/*", true)] + [InlineData("application/*", false)] + [InlineData("*/*", false)] public void CanRead_ReturnsTrueOnlyForJsonPatchContentType(string requestContentType, bool expectedCanRead) { // Arrange diff --git a/test/Microsoft.AspNet.Mvc.Formatters.Xml.Test/XmlDataContractSerializerInputFormatterTest.cs b/test/Microsoft.AspNet.Mvc.Formatters.Xml.Test/XmlDataContractSerializerInputFormatterTest.cs index c103b7be73..b16c0994eb 100644 --- a/test/Microsoft.AspNet.Mvc.Formatters.Xml.Test/XmlDataContractSerializerInputFormatterTest.cs +++ b/test/Microsoft.AspNet.Mvc.Formatters.Xml.Test/XmlDataContractSerializerInputFormatterTest.cs @@ -56,10 +56,10 @@ namespace Microsoft.AspNet.Mvc.Formatters.Xml // Mono issue - https://github.com/aspnet/External/issues/18 [FrameworkSkipCondition(RuntimeFrameworks.Mono)] [InlineData("application/xml", true)] - [InlineData("application/*", true)] - [InlineData("*/*", true)] + [InlineData("application/*", false)] + [InlineData("*/*", false)] [InlineData("text/xml", true)] - [InlineData("text/*", true)] + [InlineData("text/*", false)] [InlineData("text/json", false)] [InlineData("application/json", false)] [InlineData("", false)] diff --git a/test/Microsoft.AspNet.Mvc.Formatters.Xml.Test/XmlSerializerInputFormatterTest.cs b/test/Microsoft.AspNet.Mvc.Formatters.Xml.Test/XmlSerializerInputFormatterTest.cs index 36a6056873..5205fc9c73 100644 --- a/test/Microsoft.AspNet.Mvc.Formatters.Xml.Test/XmlSerializerInputFormatterTest.cs +++ b/test/Microsoft.AspNet.Mvc.Formatters.Xml.Test/XmlSerializerInputFormatterTest.cs @@ -41,10 +41,10 @@ namespace Microsoft.AspNet.Mvc.Formatters.Xml [Theory] [InlineData("application/xml", true)] - [InlineData("application/*", true)] - [InlineData("*/*", true)] + [InlineData("application/*", false)] + [InlineData("*/*", false)] [InlineData("text/xml", true)] - [InlineData("text/*", true)] + [InlineData("text/*", false)] [InlineData("text/json", false)] [InlineData("application/json", false)] [InlineData("", false)] diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ActionResultTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ActionResultTests.cs index 0865a5a9cd..79edd4808d 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ActionResultTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ActionResultTests.cs @@ -170,7 +170,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests "" + "2foo"; var request = new HttpRequestMessage(HttpMethod.Post, "Home/GetCustomErrorObject"); - request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/json;charset=utf-8")); + request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/json")); request.Content = new StringContent(input, Encoding.UTF8, "application/xml"); // Act @@ -218,7 +218,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests Assert.Equal("content", await response.Content.ReadAsStringAsync()); } - [Theory] + [Fact] public async Task ContentResult_WritesContent_SetsContentTypeAndEncoding() { // Arrange diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/InputFormatterTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/InputFormatterTests.cs index b30fe2ba8b..60f47fe84e 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/InputFormatterTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/InputFormatterTests.cs @@ -43,10 +43,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests [Theory] [InlineData("application/json")] - [InlineData("application/*")] - [InlineData("*/*")] [InlineData("text/json")] - [InlineData("text/*")] public async Task JsonInputFormatter_IsSelectedForJsonRequest(string requestContentType) { // Arrange diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/MvcSampleTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/MvcSampleTests.cs index 6027d34bf7..d2a4a893b4 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/MvcSampleTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/MvcSampleTests.cs @@ -100,7 +100,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests { // Arrange var request = new HttpRequestMessage(HttpMethod.Get, "http://localhost/Home/ReturnUser"); - request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/xml;charset=utf-8")); + request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/xml")); // Act var response = await Client.SendAsync(request); diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/RespectBrowserAcceptHeaderTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/RespectBrowserAcceptHeaderTests.cs index d03e70e3ff..3b2f121421 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/RespectBrowserAcceptHeaderTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/RespectBrowserAcceptHeaderTests.cs @@ -21,7 +21,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests public HttpClient Client { get; } [Theory] - [InlineData("application/xml,*/*;0.2")] + [InlineData("application/xml,*/*;q=0.2")] [InlineData("application/xml,*/*")] public async Task AllMediaRangeAcceptHeader_FirstFormatterInListWritesResponse(string acceptHeader) { @@ -43,7 +43,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests [ConditionalTheory] // Mono issue - https://github.com/aspnet/External/issues/18 [FrameworkSkipCondition(RuntimeFrameworks.Mono)] - [InlineData("application/xml,*/*;0.2")] + [InlineData("application/xml,*/*;q=0.2")] [InlineData("application/xml,*/*")] public async Task AllMediaRangeAcceptHeader_ProducesAttributeIsHonored(string acceptHeader) { @@ -71,9 +71,41 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests [ConditionalTheory] // Mono issue - https://github.com/aspnet/External/issues/18 [FrameworkSkipCondition(RuntimeFrameworks.Mono)] - [InlineData("application/xml,*/*;0.2")] + [InlineData("application/xml,*/*;q=0.2")] [InlineData("application/xml,*/*")] - public async Task AllMediaRangeAcceptHeader_WithContentTypeHeader_ContentTypeIsHonored(string acceptHeader) + public async Task AllMediaRangeAcceptHeader_WithContentTypeHeader_ContentTypeIsIgnored(string acceptHeader) + { + // Arrange + var requestData = + "35Jimmy" + + ""; + var expectedResponseData = @"{""Id"":35,""Name"":""Jimmy""}"; + var request = RequestWithAccept("http://localhost/RespectBrowserAcceptHeader/CreateEmployee", acceptHeader); + request.Content = new StringContent(requestData, Encoding.UTF8, "application/xml"); + request.Method = HttpMethod.Post; + + // Act + var response = await Client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(response.Content); + Assert.NotNull(response.Content.Headers.ContentType); + + // Site uses default output formatter (ignores Accept header) because that header contained a wildcard match. + Assert.Equal("application/json; charset=utf-8", response.Content.Headers.ContentType.ToString()); + + var responseData = await response.Content.ReadAsStringAsync(); + Assert.Equal(expectedResponseData, responseData); + } + + [ConditionalTheory] + // Mono issue - https://github.com/aspnet/External/issues/18 + [FrameworkSkipCondition(RuntimeFrameworks.Mono)] + [InlineData("application/xml,application/json;q=0.2")] + [InlineData("application/xml,application/json")] + public async Task AllMediaRangeAcceptHeader_WithExactMatch_ReturnsExpectedContent(string acceptHeader) { // Arrange var requestData = diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/XmlOutputFormatterTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/XmlOutputFormatterTests.cs index f94f44adc2..ec100e7761 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/XmlOutputFormatterTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/XmlOutputFormatterTests.cs @@ -29,7 +29,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var request = new HttpRequestMessage( HttpMethod.Post, "http://localhost/Home/GetDummyClass?sampleInput=10"); - request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/xml;charset=utf-8")); + request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/xml")); // Act var response = await Client.SendAsync(request); @@ -50,7 +50,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var request = new HttpRequestMessage( HttpMethod.Post, "http://localhost/XmlSerializer/GetDummyClass?sampleInput=10"); - request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/xml;charset=utf-8")); + request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/xml")); // Act var response = await Client.SendAsync(request); @@ -72,7 +72,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var request = new HttpRequestMessage( HttpMethod.Post, "http://localhost/DataContractSerializer/GetPerson?name=HelloWorld"); - request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/xml;charset=utf-8")); + request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/xml")); // Act var response = await Client.SendAsync(request); @@ -93,7 +93,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var request = new HttpRequestMessage( HttpMethod.Post, "http://localhost/XmlSerializer/GetDerivedDummyClass?sampleInput=10"); - request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/xml;charset=utf-8")); + request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/xml")); // Act var response = await Client.SendAsync(request); @@ -116,7 +116,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var request = new HttpRequestMessage( HttpMethod.Post, "http://localhost/Home/GetDerivedDummyClass?sampleInput=10"); - request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/xml;charset=utf-8")); + request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/xml")); // Act var response = await Client.SendAsync(request); @@ -137,7 +137,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var request = new HttpRequestMessage( HttpMethod.Post, "http://localhost/XmlSerializer/GetDictionary"); - request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/xml;charset=utf-8")); + request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/xml")); // Act var response = await Client.SendAsync(request);