Skip to content

Commit

Permalink
Change consumes behavior to ignore requests with no content type
Browse files Browse the repository at this point in the history
  • Loading branch information
JamesNK committed Dec 6, 2018
1 parent 929d7f3 commit a902943
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,10 @@ public void OnResourceExecuting(ResourceExecutingContext context)

// 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 || !IsSubsetOfAnyContentType(requestContentType))
//
// Requests without a content type do not return a 415. It is a common pattern to place [Consumes] on
// a controller and have GET actions
if (requestContentType != null && !IsSubsetOfAnyContentType(requestContentType))
{
context.Result = new UnsupportedMediaTypeResult();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Microsoft.AspNetCore.Routing;
using Microsoft.AspNetCore.Routing.Matching;
using Microsoft.AspNetCore.Routing.Patterns;
using Microsoft.Extensions.Primitives;

namespace Microsoft.AspNetCore.Mvc.Routing
{
Expand Down Expand Up @@ -120,13 +121,23 @@ public IReadOnlyList<PolicyNodeEdge> GetEdges(IReadOnlyList<Endpoint> endpoints)

// If after we're done there isn't any endpoint that accepts */*, then we'll synthesize an
// endpoint that always returns a 415.
if (!edges.ContainsKey(AnyContentType))
if (!edges.TryGetValue(AnyContentType, out var anyEndpoints))
{
edges.Add(AnyContentType, new List<Endpoint>()
{
CreateRejectionEndpoint(),
});

// Add a node to use when there is no request content type.
// When there is no content type we want the policy to no-op
edges.Add(string.Empty, endpoints.ToList());
}
else
{
// If there is an endpoint that accepts */* then it is also used when there is no content type
edges.Add(string.Empty, anyEndpoints.ToList());
}


return edges
.Select(kvp => new PolicyNodeEdge(kvp.Key, kvp.Value))
Expand Down Expand Up @@ -155,7 +166,7 @@ public PolicyJumpTable BuildJumpTable(int exitDestination, IReadOnlyList<PolicyJ
// Since our 'edges' can have wildcards, we do a sort based on how wildcard-ey they
// are then then execute them in linear order.
var ordered = edges
.Select(e => (mediaType: new MediaType((string)e.State), destination: e.Destination))
.Select(e => (mediaType: CreateEdgeMediaType(ref e), destination: e.Destination))
.OrderBy(e => GetScore(e.mediaType))
.ToArray();

Expand All @@ -170,7 +181,28 @@ public PolicyJumpTable BuildJumpTable(int exitDestination, IReadOnlyList<PolicyJ
}
}

return new ConsumesPolicyJumpTable(exitDestination, ordered);
var noContentTypeDestination = GetNoContentTypeDestination(ordered);

return new ConsumesPolicyJumpTable(exitDestination, noContentTypeDestination, ordered);
}

private static int GetNoContentTypeDestination((MediaType mediaType, int destination)[] destinations)
{
for (var i = 0; i < destinations.Length; i++)
{
if (!destinations[i].mediaType.Type.HasValue)
{
return destinations[i].destination;
}
}

throw new InvalidOperationException("Could not find destination for no content type.");
}

private static MediaType CreateEdgeMediaType(ref PolicyJumpTableEdge e)
{
var mediaType = (string)e.State;
return !string.IsNullOrEmpty(mediaType) ? new MediaType(mediaType) : default;
}

private int GetScore(in MediaType mediaType)
Expand Down Expand Up @@ -207,21 +239,24 @@ protected override int CompareMetadata(IConsumesMetadata x, IConsumesMetadata y)

private class ConsumesPolicyJumpTable : PolicyJumpTable
{
private (MediaType mediaType, int destination)[] _destinations;
private int _exitDestination;
private readonly (MediaType mediaType, int destination)[] _destinations;
private readonly int _exitDestination;
private readonly int _noContentTypeDestination;

public ConsumesPolicyJumpTable(int exitDestination, (MediaType mediaType, int destination)[] destinations)
public ConsumesPolicyJumpTable(int exitDestination, int noContentTypeDestination, (MediaType mediaType, int destination)[] destinations)
{
_exitDestination = exitDestination;
_noContentTypeDestination = noContentTypeDestination;
_destinations = destinations;
}

public override int GetDestination(HttpContext httpContext)
{
var contentType = httpContext.Request.ContentType;

if (string.IsNullOrEmpty(contentType))
{
return _exitDestination;
return _noContentTypeDestination;
}

var requestMediaType = new MediaType(contentType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ public void OnResourceExecuting_ForNoContentTypeMatch_SetsUnsupportedMediaTypeRe
[Theory]
[InlineData("")]
[InlineData(null)]
public void OnResourceExecuting_NullOrEmptyRequestContentType_SetsUnsupportedMediaTypeResult(string contentType)
public void OnResourceExecuting_NullOrEmptyRequestContentType_IsNoOp(string contentType)
{
// Arrange
var httpContext = new DefaultHttpContext();
Expand All @@ -348,8 +348,7 @@ public void OnResourceExecuting_NullOrEmptyRequestContentType_SetsUnsupportedMed
consumesFilter.OnResourceExecuting(resourceExecutingContext);

// Assert
Assert.NotNull(resourceExecutingContext.Result);
Assert.IsType<UnsupportedMediaTypeResult>(resourceExecutingContext.Result);
Assert.Null(resourceExecutingContext.Result);
}

[Theory]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ public void GetEdges_GroupsByContentType()
Assert.Collection(
edges.OrderBy(e => e.State),
e =>
{
Assert.Equal(string.Empty, e.State);
Assert.Equal(new[] { endpoints[1], endpoints[4], }, e.Endpoints.ToArray());
},
e =>
{
Assert.Equal("*/*", e.State);
Assert.Equal(new[] { endpoints[1], endpoints[4], }, e.Endpoints.ToArray());
Expand Down Expand Up @@ -123,7 +128,7 @@ public void GetEdges_GroupsByContentType()
}

[Fact] // See explanation in GetEdges for how this case is different
public void GetEdges_GroupsByContentType_CreatesHttp405Endpoint()
public void GetEdges_GroupsByContentType_CreatesHttp415Endpoint()
{
// Arrange
var endpoints = new[]
Expand All @@ -144,6 +149,11 @@ public void GetEdges_GroupsByContentType_CreatesHttp405Endpoint()
Assert.Collection(
edges.OrderBy(e => e.State),
e =>
{
Assert.Equal(string.Empty, e.State);
Assert.Equal(new[] { endpoints[0], endpoints[1], endpoints[2], }, e.Endpoints.ToArray());
},
e =>
{
Assert.Equal("*/*", e.State);
Assert.Equal(ConsumesMatcherPolicy.Http415EndpointDisplayName, Assert.Single(e.Endpoints).DisplayName);
Expand Down Expand Up @@ -190,6 +200,7 @@ public void BuildJumpTable_SortsEdgesByPriority(string contentType, int expected
var edges = new PolicyJumpTableEdge[]
{
// In reverse order of how they should be processed
new PolicyJumpTableEdge(string.Empty, 0),
new PolicyJumpTableEdge("*/*", 1),
new PolicyJumpTableEdge("application/*", 2),
new PolicyJumpTableEdge("text/*", 3),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public async Task NoRequestContentType_SelectsActionWithoutConstraint()
// Arrange
var request = new HttpRequestMessage(
HttpMethod.Post,
"http://localhost/ConsumesAttribute_Company/CreateProduct");
"http://localhost/ConsumesAttribute_WithFallbackActionController/CreateProduct");

// Act
var response = await Client.SendAsync(request);
Expand All @@ -49,7 +49,7 @@ public async Task NoRequestContentType_SelectsActionWithoutConstraint()
}

[Fact]
public async Task NoRequestContentType_Selects_IfASingleActionWithConstraintIsPresent_ReturnsUnsupported()
public async Task NoRequestContentType_Selects_IfASingleActionWithConstraintIsPresent()
{
// Arrange
var request = new HttpRequestMessage(
Expand All @@ -58,7 +58,32 @@ public async Task NoRequestContentType_Selects_IfASingleActionWithConstraintIsPr

// Act
var response = await Client.SendAsync(request);
await response.AssertStatusCodeAsync(HttpStatusCode.UnsupportedMediaType);
var body = await response.Content.ReadAsStringAsync();

// Assert
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.Equal("ConsumesAttribute_PassThrough_Product_Json", body);
}

[Fact]
public async Task NoRequestContentType_MultipleMatches_IfAMultipleActionWithConstraintIsPresent()
{
// Arrange
var request = new HttpRequestMessage(
HttpMethod.Post,
"http://localhost/ConsumesAttribute_PassThrough/CreateProductMultiple");

// Act
try
{
var response = await Client.SendAsync(request);

// Assert
Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode);
}
catch
{
}
}

[Theory]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,17 @@ public IActionResult CreateProduct(Product_Json jsonInput)
{
return Content("ConsumesAttribute_PassThrough_Product_Json");
}

[Consumes("application/json")]
public IActionResult CreateProductMultiple(Product_Json jsonInput)
{
return Content("ConsumesAttribute_PassThrough_Product_Json");
}

[Consumes("application/xml")]
public IActionResult CreateProductMultiple(Product_Xml jsonInput)
{
return Content("ConsumesAttribute_PassThrough_Product_Xml");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace BasicWebSite.Controllers.ActionConstraints
{
[Route("ConsumesAttribute_Company/[action]")]
[Route("ConsumesAttribute_WithFallbackActionController/[action]")]
public class ConsumesAttribute_WithFallbackActionController : Controller
{
[Consumes("application/json")]
Expand Down

0 comments on commit a902943

Please sign in to comment.