Skip to content

Commit a494c2c

Browse files
Chris Martinezcommonsensesoftware
Chris Martinez
authored and
commonsensesoftware
committed
Fix endpoint policy for invalid candidate with matching API version. Fixes #423.
1 parent 78ff16a commit a494c2c

File tree

6 files changed

+71
-59
lines changed

6 files changed

+71
-59
lines changed

‎src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.AspNetCore.Mvc.Versioning.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22

33
<PropertyGroup>
4-
<VersionPrefix>3.1.0</VersionPrefix>
4+
<VersionPrefix>3.1.1</VersionPrefix>
55
<AssemblyVersion>3.1.0.0</AssemblyVersion>
66
<TargetFramework>netstandard2.0</TargetFramework>
77
<NETStandardImplicitPackageVersion>2.0.0-*</NETStandardImplicitPackageVersion>

‎src/Microsoft.AspNetCore.Mvc.Versioning/ReleaseNotes.txt

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1 @@
1-
Support Endpoint Routing (#413)
2-
ApiVersioningOptions.UseApiBehavior defaults to true
3-
Add IApiControllerSpecification for API controllers
4-
Add ApiBehaviorSpecification for [ApiController]
1+
Fix 404 and 405 endpoint matching (#423)

‎src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionMatcherPolicy.cs

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -101,53 +101,51 @@ public Task ApplyAsync( HttpContext httpContext, EndpointSelectorContext context
101101
{
102102
for ( var i = 0; i < finalMatches.Count; i++ )
103103
{
104-
candidates.SetValidity( finalMatches[i].index, true );
104+
var (index, _, valid) = finalMatches[i];
105+
candidates.SetValidity( index, valid );
105106
}
106107
}
107108

108109
return CompletedTask;
109110
}
110111

111-
static IReadOnlyList<(int index, ActionDescriptor action)> EvaluateApiVersion(
112+
static IReadOnlyList<(int index, ActionDescriptor action, bool valid)> EvaluateApiVersion(
112113
HttpContext httpContext,
113114
CandidateSet candidates,
114115
ApiVersion apiVersion )
115116
{
116117
Contract.Requires( httpContext != null );
117118
Contract.Requires( candidates != null );
118-
Contract.Ensures( Contract.Result<IReadOnlyList<(int index, ActionDescriptor action)>>() != null );
119+
Contract.Ensures( Contract.Result<IReadOnlyList<(int, ActionDescriptor, bool)>>() != null );
119120

120-
var bestMatches = new List<(int index, ActionDescriptor action)>();
121-
var implicitMatches = new List<(int, ActionDescriptor)>();
121+
var bestMatches = new List<(int index, ActionDescriptor action, bool)>();
122+
var implicitMatches = new List<(int, ActionDescriptor, bool)>();
122123

123124
for ( var i = 0; i < candidates.Count; i++ )
124125
{
125-
if ( !candidates.IsValidCandidate( i ) )
126-
{
127-
continue;
128-
}
129-
130126
ref var candidate = ref candidates[i];
131127
var action = candidate.Endpoint.Metadata?.GetMetadata<ActionDescriptor>();
132128

133129
if ( action == null )
134130
{
135-
candidates.SetValidity( i, false );
136131
continue;
137132
}
138133

134+
// remember whether the candidate is currently valid. a matching api version will not
135+
// make the candidate valid; however, we want to short-circuit with 400 if no candidates
136+
// match the api version at all.
139137
switch ( action.MappingTo( apiVersion ) )
140138
{
141139
case Explicit:
142-
bestMatches.Add( (i, action) );
140+
bestMatches.Add( (i, action, candidates.IsValidCandidate( i )) );
143141
break;
144142
case Implicit:
145-
implicitMatches.Add( (i, action) );
143+
implicitMatches.Add( (i, action, candidates.IsValidCandidate( i )) );
146144
break;
147145
}
148146

149147
// perf: always make the candidate invalid so we only need to loop through the
150-
// final, best matches for any remaining, valid candidates
148+
// final, best matches for any remaining candidates
151149
candidates.SetValidity( i, false );
152150
}
153151

‎test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Mvc/Basic/given a versioned Controller/when using a query string and split into two types.cs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,35 @@ public async Task then_get_with_integer_id_should_return_200()
6060
content.Should().BeEquivalentTo( new { controller = nameof( Values2Controller ), id = 42, version = "2.0" } );
6161
}
6262

63+
[Fact]
64+
public async Task then_get_returns_400_or_405_with_invalid_id()
65+
{
66+
// arrange
67+
var requestUrl = "api/values/abc?api-version=2.0";
68+
var statusCode = UsingEndpointRouting ? NotFound : BadRequest;
69+
70+
// act
71+
var response = await GetAsync( requestUrl );
72+
73+
// assert
74+
response.StatusCode.Should().Be( statusCode );
75+
}
76+
77+
[Theory]
78+
[InlineData( "1.0" )]
79+
[InlineData( "2.0" )]
80+
public async Task then_delete_should_return_405( string apiVersion )
81+
{
82+
// arrange
83+
var requestUrl = $"api/values/42?api-version={apiVersion}";
84+
85+
// act
86+
var response = await DeleteAsync( requestUrl );
87+
88+
// assert
89+
response.StatusCode.Should().Be( MethodNotAllowed );
90+
}
91+
6392
[Fact]
6493
public async Task then_get_should_return_400_for_an_unsupported_version()
6594
{

‎test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Mvc/Basic/given a versioned Controller/when using a url segment.cs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,34 @@ public async Task then_post_should_return_201( string version )
6464
response.Headers.Location.Should().Be( new Uri( $"http://localhost/api/{version}/HelloWorld/42" ) );
6565
}
6666

67+
[Fact]
68+
public async Task then_get_returns_400_or_405_with_invalid_id()
69+
{
70+
// arrange
71+
var requestUrl = "api/v2/helloworld/abc";
72+
var statusCode = UsingEndpointRouting ? NotFound : BadRequest;
73+
74+
// act
75+
var response = await GetAsync( requestUrl );
76+
77+
// assert
78+
response.StatusCode.Should().Be( statusCode );
79+
}
80+
81+
[Theory]
82+
[InlineData( "api/v1/helloworld/42" )]
83+
[InlineData( "api/v2/helloworld/42" )]
84+
public async Task then_delete_should_return_405( string requestUrl )
85+
{
86+
// arrange
87+
88+
// act
89+
var response = await DeleteAsync( requestUrl );
90+
91+
// assert
92+
response.StatusCode.Should().Be( MethodNotAllowed );
93+
}
94+
6795
[Fact]
6896
public async Task then_get_should_return_400_for_an_unsupported_version()
6997
{

‎test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Routing/ApiVersionMatcherPolicyTest.cs

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -156,46 +156,6 @@ public async Task apply_should_use_400_endpoint_for_unmatched_api_version()
156156
errorResponses.Verify( er => er.CreateResponse( It.Is<ErrorResponseContext>( c => c.StatusCode == 400 && c.ErrorCode == "UnsupportedApiVersion" ) ), Once() );
157157
}
158158

159-
[Fact]
160-
public async Task apply_should_use_405_endpoint_for_unmatched_api_version()
161-
{
162-
// arrange
163-
var feature = new Mock<IApiVersioningFeature>();
164-
var errorResponses = new Mock<IErrorResponseProvider>();
165-
var result = new Mock<IActionResult>();
166-
167-
feature.SetupProperty( f => f.RawRequestedApiVersion, "1.0" );
168-
feature.SetupProperty( f => f.RequestedApiVersion, new ApiVersion( 1, 0 ) );
169-
result.Setup( r => r.ExecuteResultAsync( It.IsAny<ActionContext>() ) ).Returns( CompletedTask );
170-
errorResponses.Setup( er => er.CreateResponse( It.IsAny<ErrorResponseContext>() ) ).Returns( result.Object );
171-
172-
var options = Options.Create( new ApiVersioningOptions() { ErrorResponses = errorResponses.Object } );
173-
var policy = new ApiVersionMatcherPolicy( options, NewReporter(), NewLoggerFactory() );
174-
var context = new EndpointSelectorContext();
175-
var items = new object[]
176-
{
177-
new ActionDescriptor()
178-
{
179-
DisplayName = "Test",
180-
ActionConstraints = new IActionConstraintMetadata[]{ new HttpMethodActionConstraint(new[] { "POST" }) },
181-
Properties = { [typeof( ApiVersionModel )] = new ApiVersionModel( new ApiVersion( 1, 0 ) ) },
182-
},
183-
};
184-
var endpoint = new Endpoint( c => CompletedTask, new EndpointMetadataCollection( items ), default );
185-
var candidates = new CandidateSet( new[] { endpoint }, new[] { new RouteValueDictionary() }, new[] { 0 } );
186-
var httpContext = NewHttpContext( feature );
187-
188-
candidates.SetValidity( 0, false );
189-
190-
// act
191-
await policy.ApplyAsync( httpContext, context, candidates );
192-
await context.Endpoint.RequestDelegate( httpContext );
193-
194-
// assert
195-
result.Verify( r => r.ExecuteResultAsync( It.IsAny<ActionContext>() ), Once() );
196-
errorResponses.Verify( er => er.CreateResponse( It.Is<ErrorResponseContext>( c => c.StatusCode == 405 && c.ErrorCode == "UnsupportedApiVersion" ) ), Once() );
197-
}
198-
199159
[Fact]
200160
public async Task apply_should_use_400_endpoint_for_invalid_api_version()
201161
{

0 commit comments

Comments
 (0)