Fix action selection for failed response when lower precedence candidate might be erroroneously selected.

Author: Chris Martinez

src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ActionSelectionResult.cs

@@ -21,6 +21,13 @@ public class ActionSelectionResult
         /// <value>The total number of action selection iterations that have occurred. The default value is zero.</value>
         public int Iterations { get; private set; }
 
+        /// <summary>
+        /// Gets a value indicating whether are higher precendence matches in previous iterations.
+        /// </summary>
+        /// <value>True if there are higher precendence matches in one or more previous iterations;
+        /// otherwise, false. The default value is <c>false</c>.</value>
+        public bool HasMatchesInPreviousIterations { get; private set; }
+
         /// <summary>
         /// Gets the best action descriptor match.
         /// </summary>
@@ -142,6 +149,7 @@ public void EndIteration()
             }
 
             ++Iterations;
+            HasMatchesInPreviousIterations |= matchingActions[key].Count > 0;
         }
 
         /// <summary>
@@ -153,7 +161,7 @@ public void EndIteration()
         [CLSCompliant( false )]
         public bool TrySetBestMatch( ActionDescriptorMatch match )
         {
-            if ( match == null )
+            if ( match == null || HasMatchesInPreviousIterations )
             {
                 return false;
             }

src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs

@@ -120,8 +120,8 @@ public virtual ActionDescriptor SelectBestCandidate( RouteContext context, IRead
                 var selectedAction = finalMatches[0];
 
                 // note: short-circuit if the api version policy has already been applied to the match
-                // and no other better match has already been selected
-                if ( selectedAction.VersionPolicyIsApplied() && selectionResult.BestMatch == null )
+                // and there are no other matches in a previous iteration which would take precendence
+                if ( selectedAction.VersionPolicyIsApplied() && !selectionResult.HasMatchesInPreviousIterations )
                 {
                     httpContext.ApiVersionProperties().ApiVersion = selectionContext.RequestedVersion;
                     return selectedAction;

test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/Controllers/OverlappingRouteTemplateController.cs

@@ -12,5 +12,11 @@ public class OverlappingRouteTemplateController : Controller
 
         [HttpGet( "{id:int}/children" )]
         public IActionResult Get( int id ) => Ok( new { id } );
+
+        [HttpGet( "{id:int}/ambiguous" )]
+        public IActionResult Ambiguous( int id ) => Ok();
+
+        [HttpGet( "{id:int}/ambiguous" )]
+        public IActionResult Ambiguous2( int id ) => Ok();
     }
 }

test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/given a versioned Controller/when two route templates overlap.cs

@@ -1,9 +1,10 @@
 namespace given_a_versioned_Controller
 {
     using FluentAssertions;
-    using Microsoft.AspNetCore.Mvc;
     using Microsoft.AspNetCore.Mvc.Basic;
     using Microsoft.AspNetCore.Mvc.Basic.Controllers;
+    using Microsoft.AspNetCore.Mvc.Internal;
+    using System;
     using System.Reflection;
     using System.Threading.Tasks;
     using Xunit;
@@ -47,5 +48,20 @@ public async Task then_the_higher_precedence_route_should_be_selected_during_the
             result1.Should().Be( "{\"id\":42,\"childId\":\"abc\"}" );
             result2.Should().Be( "{\"id\":42}" );
         }
+
+        [Fact]
+        public async Task then_the_higher_precedence_route_should_result_in_ambiguous_action_exception_during_the_second_request()
+        {
+            // arrange
+            var response = await Client.GetAsync( "api/v1/values/42/abc" );
+            var result1 = await response.EnsureSuccessStatusCode().Content.ReadAsStringAsync();
+
+            // act
+            Func<Task> act = async () => await Client.GetAsync( "api/v1/values/42/ambiguous" );
+
+            // assert
+            result1.Should().Be( "{\"id\":42,\"childId\":\"abc\"}" );
+            act.ShouldThrow<AmbiguousActionException>();
+        }
     }
 }