Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Commit 03625c3

Browse files
committed
Correct polarity of MediaTypeHeaderValue.IsSubsetOf()` checks and remove one conneg fallback
- #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
1 parent 2e2043f commit 03625c3

File tree

17 files changed

+460
-147
lines changed

17 files changed

+460
-147
lines changed

src/Microsoft.AspNet.Mvc.Core/ConsumesAttribute.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,10 @@ public void OnResourceExecuting(ResourceExecutingContext context)
5656
MediaTypeHeaderValue requestContentType = null;
5757
MediaTypeHeaderValue.TryParse(context.HttpContext.Request.ContentType, out requestContentType);
5858

59-
// Only execute if this is the last filter before calling the action.
60-
// This ensures that we only run the filter which is closest to the action.
59+
// Confirm the request's content type is more specific than a media type this action supports e.g. OK
60+
// if client sent "text/plain" data and this action supports "text/*".
6161
if (requestContentType != null &&
62-
!ContentTypes.Any(contentType => contentType.IsSubsetOf(requestContentType)))
62+
!ContentTypes.Any(contentType => requestContentType.IsSubsetOf(contentType)))
6363
{
6464
context.Result = new UnsupportedMediaTypeResult();
6565
}
@@ -102,7 +102,9 @@ public bool Accept(ActionConstraintContext context)
102102
return !isActionWithoutConsumeConstraintPresent;
103103
}
104104

105-
if (ContentTypes.Any(c => c.IsSubsetOf(requestContentType)))
105+
// Confirm the request's content type is more specific than a media type this action supports e.g. OK
106+
// if client sent "text/plain" data and this action supports "text/*".
107+
if (ContentTypes.Any(contentType => requestContentType.IsSubsetOf(contentType)))
106108
{
107109
return true;
108110
}

src/Microsoft.AspNet.Mvc.Core/Controllers/FilterActionInvoker.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ public virtual async Task InvokeAsync()
183183
_cursor = new FilterCursor(_filters);
184184

185185
ActionContext.ModelState.MaxAllowedErrors = _maxModelValidationErrors;
186-
186+
187187
await InvokeAllAuthorizationFiltersAsync();
188188

189189
// If Authorization Filters return a result, it's a short circuit because
@@ -663,7 +663,7 @@ private async Task<ActionExecutedContext> InvokeActionFilterAsync()
663663
"Microsoft.AspNet.Mvc.BeforeActionMethod",
664664
new
665665
{
666-
actionContext = ActionContext,
666+
actionContext = ActionContext,
667667
arguments = _actionExecutingContext.ActionArguments,
668668
controller = _actionExecutingContext.Controller
669669
});

src/Microsoft.AspNet.Mvc.Core/Formatters/FormatFilter.cs

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
namespace Microsoft.AspNet.Mvc.Formatters
1414
{
1515
/// <summary>
16-
/// A filter which will use the format value in the route data or query string to set the content type on an
16+
/// A filter which will use the format value in the route data or query string to set the content type on an
1717
/// <see cref="ObjectResult" /> returned from an action.
1818
/// </summary>
1919
public class FormatFilter : IFormatFilter, IResourceFilter, IResultFilter
@@ -48,13 +48,13 @@ public FormatFilter(IOptions<MvcOptions> options, IActionContextAccessor actionC
4848
public MediaTypeHeaderValue ContentType { get; }
4949

5050
/// <summary>
51-
/// <c>true</c> if the current <see cref="FormatFilter"/> is active and will execute.
51+
/// <c>true</c> if the current <see cref="FormatFilter"/> is active and will execute.
5252
/// </summary>
5353
public bool IsActive { get; }
5454

5555
/// <summary>
5656
/// As a <see cref="IResourceFilter"/>, this filter looks at the request and rejects it before going ahead if
57-
/// 1. The format in the request doesnt match any format in the map.
57+
/// 1. The format in the request does not match any format in the map.
5858
/// 2. If there is a conflicting producesFilter.
5959
/// </summary>
6060
/// <param name="context">The <see cref="ResourceExecutingContext"/>.</param>
@@ -67,7 +67,8 @@ public void OnResourceExecuting(ResourceExecutingContext context)
6767

6868
if (!IsActive)
6969
{
70-
return; // no format specified by user, so the filter is muted
70+
// no format specified by user, so the filter is muted
71+
return;
7172
}
7273

7374
if (ContentType == null)
@@ -77,19 +78,22 @@ public void OnResourceExecuting(ResourceExecutingContext context)
7778
return;
7879
}
7980

81+
// Determine media types this action supports.
8082
var responseTypeFilters = context.Filters.OfType<IApiResponseMetadataProvider>();
81-
var contentTypes = new List<MediaTypeHeaderValue>();
82-
83+
var supportedMediaTypes = new List<MediaTypeHeaderValue>();
8384
foreach (var filter in responseTypeFilters)
8485
{
85-
filter.SetContentTypes(contentTypes);
86+
filter.SetContentTypes(supportedMediaTypes);
8687
}
8788

88-
if (contentTypes.Count != 0)
89+
// Check if support is adequate for requested media type.
90+
if (supportedMediaTypes.Count != 0)
8991
{
90-
// We need to check if the action can generate the content type the user asked for. If it cannot, exit
91-
// here with not found result.
92-
if (!contentTypes.Any(c => ContentType.IsSubsetOf(c)))
92+
// We need to check if the action can generate the content type the user asked for. That is, treat the
93+
// request's format and IApiResponseMetadataProvider-provided content types similarly to an Accept
94+
// header and an output formatter's SupportedMediaTypes: Confirm action supports a more specific media
95+
// type than requested e.g. OK if "text/*" requested and action supports "text/plain".
96+
if (!supportedMediaTypes.Any(contentType => contentType.IsSubsetOf(ContentType)))
9397
{
9498
context.Result = new HttpNotFoundResult();
9599
}

src/Microsoft.AspNet.Mvc.Core/Formatters/InputFormatter.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,17 @@ public virtual bool CanRead(InputFormatterContext context)
6262

6363
var contentType = context.HttpContext.Request.ContentType;
6464
MediaTypeHeaderValue requestContentType;
65-
if (!MediaTypeHeaderValue.TryParse(contentType, out requestContentType))
65+
if (!MediaTypeHeaderValue.TryParse(contentType, out requestContentType) || requestContentType == null)
6666
{
6767
return false;
6868
}
6969

70-
return SupportedMediaTypes.Any(supportedMediaType => supportedMediaType.IsSubsetOf(requestContentType));
70+
// Confirm the request's content type is more specific than a media type this formatter supports e.g. OK if
71+
// client sent "text/plain" data and this formatter supports "text/*".
72+
return SupportedMediaTypes.Any(supportedMediaType =>
73+
{
74+
return requestContentType.IsSubsetOf(supportedMediaType);
75+
});
7176
}
7277

7378
/// <summary>

src/Microsoft.AspNet.Mvc.Core/Formatters/OutputFormatter.cs

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ public virtual IReadOnlyList<MediaTypeHeaderValue> GetSupportedContentTypes(
7777
{
7878
List<MediaTypeHeaderValue> mediaTypes = null;
7979

80+
// Confirm this formatter supports a more specific media type than requested e.g. OK if "text/*"
81+
// requested and formatter supports "text/plain". Treat contentType like it came from an Accept header.
8082
foreach (var mediaType in _supportedMediaTypes)
8183
{
8284
if (mediaType.IsSubsetOf(contentType))
@@ -119,8 +121,7 @@ public virtual Encoding SelectCharacterEncoding(OutputFormatterContext context)
119121
{
120122
var requestCharset = requestContentType.Charset;
121123
encoding = SupportedEncodings.FirstOrDefault(
122-
supportedEncoding =>
123-
requestCharset.Equals(supportedEncoding.WebName));
124+
supportedEncoding => requestCharset.Equals(supportedEncoding.WebName));
124125
}
125126
}
126127

@@ -151,10 +152,11 @@ public virtual bool CanWriteResult(OutputFormatterContext context, MediaTypeHead
151152
}
152153
else
153154
{
154-
// Since supportedMedia Type is going to be more specific check if supportedMediaType is a subset
155-
// of the content type which is typically what we get on acceptHeader.
156-
mediaType = SupportedMediaTypes
157-
.FirstOrDefault(supportedMediaType => supportedMediaType.IsSubsetOf(contentType));
155+
// Confirm this formatter supports a more specific media type than requested e.g. OK if "text/*"
156+
// requested and formatter supports "text/plain". contentType is typically what we got in an Accept
157+
// header.
158+
mediaType = SupportedMediaTypes.FirstOrDefault(
159+
supportedMediaType => supportedMediaType.IsSubsetOf(contentType));
158160
}
159161

160162
if (mediaType != null)
@@ -201,11 +203,11 @@ public virtual void WriteResponseHeaders(OutputFormatterContext context)
201203
// Copy the media type as we don't want it to affect the next request
202204
selectedMediaType = selectedMediaType.Copy();
203205

204-
// Not text-based media types will use an encoding/charset - binary formats just ignore it. We want to
206+
// Note text-based media types will use an encoding/charset - binary formats just ignore it. We want to
205207
// make this class work with media types that use encodings, and those that don't.
206208
//
207209
// The default implementation of SelectCharacterEncoding will read from the list of SupportedEncodings
208-
// and will always choose a default encoding if any are supported. So, the only cases where the
210+
// and will always choose a default encoding if any are supported. So, the only cases where the
209211
// selectedEncoding can be null are:
210212
//
211213
// 1). No supported encodings - we assume this is a non-text format
@@ -237,21 +239,17 @@ private Encoding MatchAcceptCharacterEncoding(IList<StringWithQualityHeaderValue
237239
if (acceptCharsetHeaders != null && acceptCharsetHeaders.Count > 0)
238240
{
239241
var sortedAcceptCharsetHeaders = acceptCharsetHeaders
240-
.Where(acceptCharset =>
241-
acceptCharset.Quality != HeaderQuality.NoMatch)
242-
.OrderByDescending(
243-
m => m, StringWithQualityHeaderValueComparer.QualityComparer);
242+
.Where(acceptCharset => acceptCharset.Quality != HeaderQuality.NoMatch)
243+
.OrderByDescending(m => m, StringWithQualityHeaderValueComparer.QualityComparer);
244244

245245
foreach (var acceptCharset in sortedAcceptCharsetHeaders)
246246
{
247247
var charset = acceptCharset.Value;
248248
if (!string.IsNullOrWhiteSpace(charset))
249249
{
250-
var encoding = SupportedEncodings.FirstOrDefault(
251-
supportedEncoding =>
252-
charset.Equals(supportedEncoding.WebName,
253-
StringComparison.OrdinalIgnoreCase) ||
254-
charset.Equals("*", StringComparison.Ordinal));
250+
var encoding = SupportedEncodings.FirstOrDefault(supportedEncoding =>
251+
charset.Equals(supportedEncoding.WebName, StringComparison.OrdinalIgnoreCase) ||
252+
charset.Equals("*", StringComparison.Ordinal));
255253
if (encoding != null)
256254
{
257255
return encoding;

0 commit comments

Comments
 (0)