Update how `FormTagHelper` handles `get` method attributes.

- Added functional test to verify `asp-*` attributes on form taghelpers work as expected.
- Added a unit test to validate `FormTagHelper` behaves as expected.
- Moved `Method == "get"` checks into appropriate code paths. These include the one where a user specifies an empty or non-existent `action` attribute and where a user doesn't utilize any `asp-*` attributes. In the later, we default `Method` to `"get"` if it's not provided.

#6006
This commit is contained in:
N. Taylor Mullen 2017-04-25 13:51:26 -07:00
parent ce4e94c1ac
commit f165914b40
4 changed files with 82 additions and 19 deletions

View File

@ -157,10 +157,6 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
{
output.CopyHtmlAttribute(nameof(Method), context);
}
else
{
Method = "get";
}
var antiforgeryDefault = true;
@ -205,9 +201,16 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
if (string.IsNullOrEmpty(attributeValue))
{
// User is using the FormTagHelper like a normal <form> tag that has an empty or complex IHtmlContent action attribute.
// e.g. <form action="" method="post"> or <form action="@CustomUrlIHtmlContent" method="post">
// e.g. <form action="" method="..."> or <form action="@CustomUrlIHtmlContent" method="...">
// Antiforgery default is already set to true
if (string.Equals(Method ?? "get", "get", StringComparison.OrdinalIgnoreCase))
{
antiforgeryDefault = false;
}
else
{
// Antiforgery default is already set to true
}
}
else
{
@ -251,17 +254,18 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
}
TagBuilder tagBuilder = null;
if (Action == null &&
Controller == null &&
Route == null &&
_routeValues == null &&
Fragment == null &&
Area == null &&
if (Action == null &&
Controller == null &&
Route == null &&
_routeValues == null &&
Fragment == null &&
Area == null &&
Page == null &&
// Antiforgery will sometime be set globally via TagHelper Initializers, verify it was provided in the cshtml.
!context.AllAttributes.ContainsName(AntiforgeryAttributeName))
{
// Empty form tag such as <form></form>. Let it flow to the output as-is and only handle anti-forgery.
// A <form> tag that doesn't utilize asp-* attributes. Let it flow to the output.
Method = Method ?? "get";
}
else if (pageLink)
{
@ -304,11 +308,11 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
output.PostContent.AppendHtml(tagBuilder.InnerHtml);
}
}
}
if (string.Equals(Method, "get", StringComparison.OrdinalIgnoreCase))
{
antiforgeryDefault = false;
if (string.Equals(Method, "get", StringComparison.OrdinalIgnoreCase))
{
antiforgeryDefault = false;
}
}
if (Antiforgery ?? antiforgeryDefault)

View File

@ -3,4 +3,5 @@
<form method="post"><input name="__RequestVerificationToken" type="hidden" value="{0}" /></form>
<form action="" method="post"><input name="__RequestVerificationToken" type="hidden" value="{0}" /></form>
<form action="/Foo/Bar/Baz.html" method="get"></form>
<form action="/Foo/Bar/Baz.html" method="post"></form>
<form action="/Foo/Bar/Baz.html" method="post"></form>
<form action="/RedirectToPage" method="post"><input name="__RequestVerificationToken" type="hidden" value="{0}" /></form>

View File

@ -25,6 +25,63 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
{
public class FormTagHelperTest
{
[Fact]
public async Task ProcessAsync_ActionAndControllerGenerateAntiforgery()
{
// Arrange
var expectedTagName = "form";
var metadataProvider = new TestModelMetadataProvider();
var tagHelperContext = new TagHelperContext(
tagName: "form",
allAttributes: new TagHelperAttributeList()
{
{ "asp-action", "index" },
{ "asp-controller", "home" }
},
items: new Dictionary<object, object>(),
uniqueId: "test");
var output = new TagHelperOutput(
expectedTagName,
attributes: new TagHelperAttributeList(),
getChildContentAsync: (useCachedResult, encoder) =>
{
var tagHelperContent = new DefaultTagHelperContent();
tagHelperContent.SetContent("Something");
return Task.FromResult<TagHelperContent>(tagHelperContent);
});
var urlHelper = new Mock<IUrlHelper>();
urlHelper
.Setup(mock => mock.Action(It.IsAny<UrlActionContext>())).Returns("home/index");
var htmlGenerator = new TestableHtmlGenerator(metadataProvider, urlHelper.Object);
var viewContext = TestableHtmlGenerator.GetViewContext(
model: null,
htmlGenerator: htmlGenerator,
metadataProvider: metadataProvider);
var expectedPostContent = HtmlContentUtilities.HtmlContentToString(
htmlGenerator.GenerateAntiforgery(viewContext),
HtmlEncoder.Default);
var formTagHelper = new FormTagHelper(htmlGenerator)
{
ViewContext = viewContext,
Action = "index",
Controller = "home",
};
// Act
await formTagHelper.ProcessAsync(tagHelperContext, output);
// Assert
Assert.Equal(2, output.Attributes.Count);
var attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("method"));
Assert.Equal("post", attribute.Value);
attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("action"));
Assert.Equal("home/index", attribute.Value);
Assert.Empty(output.PreContent.GetContent());
Assert.True(output.Content.GetContent().Length == 0);
Assert.Equal(expectedPostContent, output.PostContent.GetContent());
Assert.Equal(expectedTagName, output.TagName);
}
[Fact]
public async Task ProcessAsync_AspAntiforgeryAloneGeneratesProperFormTag()
{

View File

@ -6,4 +6,5 @@
<form method="post"></form>
<form action="" method="post"></form>
<form action="/Foo/Bar/Baz.html" method="get"></form>
<form action="/Foo/Bar/Baz.html" method="post"></form>
<form action="/Foo/Bar/Baz.html" method="post"></form>
<form asp-controller="Redirect" asp-action="RedirectToPageAction"></form>