Make handler selector more flexible

Some details of this pending discussion, but this is a new 2.1 change
and compatibility switch in the spirit of making pages handler selection
less error-prone.

In particular we don't want anyone to have to define HEAD to do the
trivial thing. This currently routes all 'safe' HTTP methods to the GET
handler and all other HTTP methods to the POST handler.

This is technically not the correct thing to do for OPTIONS and TRACE,
so we might still do something different.

The tests will change a little depending on exactly what we decide to
do, but this is the main idea of the change.
This commit is contained in:
Ryan Nowak 2018-03-13 14:09:47 -07:00
parent 1d6c09ab31
commit 14429721d9
10 changed files with 365 additions and 45 deletions

View File

@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.Extensions.Options;
using Microsoft.Extensions.Primitives;
namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
@ -12,6 +13,25 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
{
private const string Handler = "handler";
private readonly IOptions<RazorPagesOptions> _options;
[Obsolete("This constructor will be removed in a future release. Use the other constructor.")]
public DefaultPageHandlerMethodSelector()
{
}
public DefaultPageHandlerMethodSelector(IOptions<RazorPagesOptions> options)
{
if (options == null)
{
throw new ArgumentNullException(nameof(options));
}
_options = options;
}
private bool AllowFuzzyHttpMethodMatching => _options?.Value.AllowMappingHeadRequestsToGetHandler ?? false;
public HandlerMethodDescriptor Select(PageContext context)
{
var handlers = SelectHandlers(context);
@ -60,42 +80,71 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
return null;
}
private static List<HandlerMethodDescriptor> SelectHandlers(PageContext context)
private List<HandlerMethodDescriptor> SelectHandlers(PageContext context)
{
var handlers = context.ActionDescriptor.HandlerMethods;
List<HandlerMethodDescriptor> handlersToConsider = null;
var candidates = new List<HandlerMethodDescriptor>();
var handlerName = Convert.ToString(context.RouteData.Values[Handler]);
// Name is optional, may not be provided.
var handlerName = GetHandlerName(context);
if (string.IsNullOrEmpty(handlerName) &&
context.HttpContext.Request.Query.TryGetValue(Handler, out StringValues queryValues))
{
handlerName = queryValues[0];
}
// The handler selection process considers handlers according to a few criteria. Handlers
// have a defined HTTP method that they handle, and also optionally a 'name'.
//
// We don't really have a scenario for handler methods without a verb (we don't provide a way
// to create one). If we see one, it will just never match.
//
// The verb must match (with some fuzzy matching) and the handler name must match if
// there is one.
//
// The process is like this:
//
// 1. Match the possible candidates on HTTP method
// 1a. **Added in 2.1** if no candidates matched in 1, then do *fuzzy matching*
// 2. Match the candidates from 1 or 1a on handler name.
// Step 1: match on HTTP method.
var httpMethod = context.HttpContext.Request.Method;
for (var i = 0; i < handlers.Count; i++)
{
var handler = handlers[i];
if (handler.HttpMethod != null &&
!string.Equals(handler.HttpMethod, context.HttpContext.Request.Method, StringComparison.OrdinalIgnoreCase))
string.Equals(handler.HttpMethod, httpMethod, StringComparison.OrdinalIgnoreCase))
{
continue;
candidates.Add(handler);
}
else if (handler.Name != null &&
!handler.Name.Equals(handlerName, StringComparison.OrdinalIgnoreCase))
{
continue;
}
if (handlersToConsider == null)
{
handlersToConsider = new List<HandlerMethodDescriptor>();
}
handlersToConsider.Add(handler);
}
return handlersToConsider;
// Step 1a: do fuzzy HTTP method matching if needed.
if (candidates.Count == 0 && AllowFuzzyHttpMethodMatching)
{
var fuzzyHttpMethod = GetFuzzyMatchHttpMethod(context);
if (fuzzyHttpMethod != null)
{
for (var i = 0; i < handlers.Count; i++)
{
var handler = handlers[i];
if (handler.HttpMethod != null &&
string.Equals(handler.HttpMethod, fuzzyHttpMethod, StringComparison.OrdinalIgnoreCase))
{
candidates.Add(handler);
}
}
}
}
// Step 2: remove candiates with non-matching handlers.
for (var i = candidates.Count - 1; i >= 0; i--)
{
var handler = candidates[i];
if (handler.Name != null &&
!handler.Name.Equals(handlerName, StringComparison.OrdinalIgnoreCase))
{
candidates.RemoveAt(i);
}
}
return candidates;
}
private static int GetScore(HandlerMethodDescriptor descriptor)
@ -113,5 +162,34 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
return 0;
}
}
private static string GetHandlerName(PageContext context)
{
var handlerName = Convert.ToString(context.RouteData.Values[Handler]);
if (!string.IsNullOrEmpty(handlerName))
{
return handlerName;
}
if (context.HttpContext.Request.Query.TryGetValue(Handler, out StringValues queryValues))
{
return queryValues[0];
}
return null;
}
private static string GetFuzzyMatchHttpMethod(PageContext context)
{
var httpMethod = context.HttpContext.Request.Method;
// Map HEAD to get.
if (string.Equals("HEAD", httpMethod, StringComparison.OrdinalIgnoreCase))
{
return "GET";
}
return null;
}
}
}

View File

@ -7,7 +7,7 @@ using Microsoft.AspNetCore.Mvc.Abstractions;
namespace Microsoft.AspNetCore.Mvc.RazorPages
{
[DebuggerDisplay(nameof(DebuggerDisplayString))]
[DebuggerDisplay("{DebuggerDisplayString,nq}")]
public class PageActionDescriptor : ActionDescriptor
{
/// <summary>

View File

@ -15,6 +15,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages
public class RazorPagesOptions : IEnumerable<ICompatibilitySwitch>
{
private readonly CompatibilitySwitch<bool> _allowAreas;
private readonly CompatibilitySwitch<bool> _allowMappingHeadRequestsToGetHandler;
private readonly ICompatibilitySwitch[] _switches;
private string _root = "/Pages";
@ -23,10 +24,12 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages
public RazorPagesOptions()
{
_allowAreas = new CompatibilitySwitch<bool>(nameof(AllowAreas));
_allowMappingHeadRequestsToGetHandler = new CompatibilitySwitch<bool>(nameof(AllowMappingHeadRequestsToGetHandler));
_switches = new ICompatibilitySwitch[]
{
_allowAreas,
_allowMappingHeadRequestsToGetHandler,
};
}
@ -94,6 +97,46 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages
set => _allowAreas.Value = value;
}
/// <summary>
/// Gets or sets a value that determines if HTTP method matching for Razor Pages handler methods will use
/// fuzzy matching.
/// Defaults to <c>false</c>.
/// </summary>
/// <remarks>
/// <para>
/// When enabled, Razor Pages handler methods will be more flexible in which HTTP methods will be accepted
/// by GET and POST handler methods. This allows a GET handler methods to accept the HEAD HTTP methods in
/// addition to GET. A more specific handler method can still be defined to accept HEAD, and the most
/// specific handler will be invoked.
/// </para>
/// <para>
/// This setting reduces the number of handler methods that must be written to correctly respond to typical
/// web traffic including requests from internet infrastructure such as web crawlers.
/// </para>
/// <para>
/// This property is associated with a compatibility switch and can provide a different behavior depending on
/// the configured compatibility version for the application. See <see cref="CompatibilityVersion"/> for
/// guidance and examples of setting the application's compatibility version.
/// </para>
/// <para>
/// Configuring the desired of the value compatibility switch by calling this property's setter will take precedence
/// over the value implied by the application's <see cref="CompatibilityVersion"/>.
/// </para>
/// <para>
/// If the application's compatibility version is set to <see cref="CompatibilityVersion.Version_2_0"/> then
/// this setting will have value <c>false</c> unless explicitly configured.
/// </para>
/// <para>
/// If the application's compatibility version is set to <see cref="CompatibilityVersion.Version_2_1"/> or
/// higher then this setting will have value <c>true</c> unless explicitly configured.
/// </para>
/// </remarks>
public bool AllowMappingHeadRequestsToGetHandler
{
get => _allowMappingHeadRequestsToGetHandler.Value;
set => _allowMappingHeadRequestsToGetHandler.Value = value;
}
/// <summary>
/// Application relative path used as the root of discovery for Razor Page files associated with areas.
/// Defaults to the <c>/Areas</c> directory under application root.

View File

@ -26,6 +26,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages
if (Version >= CompatibilityVersion.Version_2_1)
{
values[nameof(RazorPagesOptions.AllowAreas)] = true;
values[nameof(RazorPagesOptions.AllowMappingHeadRequestsToGetHandler)] = true;
}
return values;

View File

@ -385,11 +385,14 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
Assert.StartsWith("Hello, You posted!", content.Trim());
}
[Fact]
public async Task HelloWorldWithPageModelHandler_CanGetContent()
[Theory]
[InlineData("GET")]
[InlineData("HEAD")]
public async Task HelloWorldWithPageModelHandler_CanGetContent(string httpMethod)
{
// Arrange
var request = new HttpRequestMessage(HttpMethod.Get, "http://localhost/HelloWorldWithPageModelHandler?message=pagemodel");
var url = "http://localhost/HelloWorldWithPageModelHandler?message=pagemodel";
var request = new HttpRequestMessage(new HttpMethod(httpMethod), url);
// Act
var response = await Client.SendAsync(request);

View File

@ -3,11 +3,10 @@
using System;
using System.Collections.Generic;
using System.Reflection;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.Primitives;
using Microsoft.Extensions.Options;
using Xunit;
namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
@ -15,7 +14,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
public class DefaultPageHandlerMethodSelectorTest
{
[Fact]
public void Select_ReturnsNull_WhenNoHandlerMatchesHttpMethod()
public void LegacyBehavior_Select_ReturnsNull_WhenNoHandlerMatchesHttpMethod()
{
// Arrange
var descriptor1 = new HandlerMethodDescriptor
@ -47,7 +46,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
},
},
};
var selector = new DefaultPageHandlerMethodSelector();
var selector = CreateSelector(legacyBehavior: true);
// Act
var actual = selector.Select(pageContext);
@ -56,6 +55,191 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
Assert.Null(actual);
}
[Fact]
public void NewBehavior_Select_ReturnsFuzzyMatchForHead_WhenNoHeadHandlerDefined()
{
// Arrange
var descriptor1 = new HandlerMethodDescriptor
{
HttpMethod = "GET"
};
var descriptor2 = new HandlerMethodDescriptor
{
HttpMethod = "POST"
};
var pageContext = new PageContext
{
ActionDescriptor = new CompiledPageActionDescriptor
{
HandlerMethods = new List<HandlerMethodDescriptor>()
{
descriptor1,
descriptor2,
},
},
RouteData = new RouteData(),
HttpContext = new DefaultHttpContext
{
Request =
{
Method = "HEAD"
},
},
};
var selector = CreateSelector();
// Act
var actual = selector.Select(pageContext);
// Assert
Assert.Same(descriptor1, actual);
}
[Fact]
public void NewBehavior_Select_PrefersExactMatch()
{
// Arrange
var descriptor1 = new HandlerMethodDescriptor
{
HttpMethod = "GET"
};
var descriptor2 = new HandlerMethodDescriptor
{
HttpMethod = "POST"
};
var descriptor3 = new HandlerMethodDescriptor
{
HttpMethod = "HEAD"
};
var pageContext = new PageContext
{
ActionDescriptor = new CompiledPageActionDescriptor
{
HandlerMethods = new List<HandlerMethodDescriptor>()
{
descriptor1,
descriptor2,
descriptor3,
},
},
RouteData = new RouteData(),
HttpContext = new DefaultHttpContext
{
Request =
{
Method = "HEAD",
},
},
};
var selector = CreateSelector();
// Act
var actual = selector.Select(pageContext);
// Assert
Assert.Same(descriptor3, actual);
}
[Fact]
public void NewBehavior_Select_PrefersExactMatch_ReturnsNullWhenHandlerNameDoesntMatch()
{
// Arrange
var descriptor1 = new HandlerMethodDescriptor
{
HttpMethod = "GET"
};
var descriptor2 = new HandlerMethodDescriptor
{
HttpMethod = "POST"
};
// This will match the HTTP method 'round' of selection, but won't match the
// handler name.
var descriptor3 = new HandlerMethodDescriptor
{
HttpMethod = "HEAD",
Name = "not-provided",
};
var pageContext = new PageContext
{
ActionDescriptor = new CompiledPageActionDescriptor
{
HandlerMethods = new List<HandlerMethodDescriptor>()
{
descriptor1,
descriptor2,
descriptor3,
},
},
RouteData = new RouteData(),
HttpContext = new DefaultHttpContext
{
Request =
{
Method = "HEAD",
},
},
};
var selector = CreateSelector();
// Act
var actual = selector.Select(pageContext);
// Assert
Assert.Null(actual);
}
[Theory]
[InlineData("HEAD")]
[InlineData("heAd")]
public void NewBehavior_Select_ReturnsFuzzyMatch_SafeVerbs(string httpMethod)
{
// Arrange
var descriptor1 = new HandlerMethodDescriptor
{
HttpMethod = "GET"
};
var descriptor2 = new HandlerMethodDescriptor
{
HttpMethod = "POST"
};
var pageContext = new PageContext
{
ActionDescriptor = new CompiledPageActionDescriptor
{
HandlerMethods = new List<HandlerMethodDescriptor>()
{
descriptor1,
descriptor2,
},
},
RouteData = new RouteData(),
HttpContext = new DefaultHttpContext
{
Request =
{
Method = httpMethod
},
},
};
var selector = CreateSelector();
// Act
var actual = selector.Select(pageContext);
// Assert
Assert.Same(descriptor1, actual);
}
[Fact]
public void Select_ReturnsOnlyHandler()
{
@ -83,7 +267,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
},
},
};
var selector = new DefaultPageHandlerMethodSelector();
var selector = CreateSelector();
// Act
var actual = selector.Select(pageContext);
@ -126,7 +310,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
},
},
};
var selector = new DefaultPageHandlerMethodSelector();
var selector = CreateSelector();
// Act
var actual = selector.Select(pageContext);
@ -176,7 +360,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
},
},
};
var selector = new DefaultPageHandlerMethodSelector();
var selector = CreateSelector();
// Act
var actual = selector.Select(pageContext);
@ -226,7 +410,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
},
},
};
var selector = new DefaultPageHandlerMethodSelector();
var selector = CreateSelector();
// Act
var actual = selector.Select(pageContext);
@ -271,7 +455,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
},
},
};
var selector = new DefaultPageHandlerMethodSelector();
var selector = CreateSelector();
// Act
var actual = selector.Select(pageContext);
@ -322,7 +506,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
},
},
};
var selector = new DefaultPageHandlerMethodSelector();
var selector = CreateSelector();
// Act
var actual = selector.Select(pageContext);
@ -367,7 +551,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
},
},
};
var selector = new DefaultPageHandlerMethodSelector();
var selector = CreateSelector();
// Act
var actual = selector.Select(pageContext);
@ -416,7 +600,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
},
},
};
var selector = new DefaultPageHandlerMethodSelector();
var selector = CreateSelector();
// Act
var actual = selector.Select(pageContext);
@ -466,7 +650,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
},
},
};
var selector = new DefaultPageHandlerMethodSelector();
var selector = CreateSelector();
// Act & Assert
var ex = Assert.Throws<InvalidOperationException>(() => selector.Select(pageContext));
@ -526,7 +710,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
},
},
};
var selector = new DefaultPageHandlerMethodSelector();
var selector = CreateSelector();
// Act & Assert
var ex = Assert.Throws<InvalidOperationException>(() => selector.Select(pageContext));
@ -544,5 +728,14 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal
protected void PostAsync()
{
}
private static DefaultPageHandlerMethodSelector CreateSelector(bool legacyBehavior = false)
{
var options = Options.Create(new RazorPagesOptions()
{
AllowMappingHeadRequestsToGetHandler = !legacyBehavior
});
return new DefaultPageHandlerMethodSelector(options);
}
}
}

View File

@ -25,7 +25,8 @@ namespace RazorPagesWebSite
options.Conventions.AddPageRoute("/Pages/NotTheRoot", string.Empty);
options.Conventions.Add(new CustomModelTypeConvention());
})
.WithRazorPagesAtContentRoot();
.WithRazorPagesAtContentRoot()
.SetCompatibilityVersion(Microsoft.AspNetCore.Mvc.CompatibilityVersion.Version_2_1);
}
public void Configure(IApplicationBuilder app)

View File

@ -24,7 +24,8 @@ namespace RazorPagesWebSite
options.Conventions.AuthorizeAreaFolder("Accounts", "/RequiresAuth");
options.Conventions.AllowAnonymousToAreaPage("Accounts", "/RequiresAuth/AllowAnonymous");
options.Conventions.Add(new CustomModelTypeConvention());
});
})
.SetCompatibilityVersion(Microsoft.AspNetCore.Mvc.CompatibilityVersion.Version_2_1);
}
public void Configure(IApplicationBuilder app)

View File

@ -4,7 +4,7 @@
{
public IActionResult OnGet()
{
TempData["TempDataProperty-Message"] = "Secret Message";
TempData["Message"] = "Secret Message";
return Redirect("~/TempData/TempDataPageModelProperty");
}
}

View File

@ -6,4 +6,4 @@ Message: @Model.Message
{
@Html.AntiForgeryToken()
}
TempData: @TempData["TempDataProperty-Message"]
TempData: @TempData["Message"]