From 7dc95439d47fbd037c218ba3ec7b73cc066413cc Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Tue, 25 Apr 2017 13:51:26 -0700 Subject: [PATCH] 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 --- .../FormTagHelper.cs | 38 +++++++------ .../RazorPagesWebSite.SimpleForms.html | 3 +- .../FormTagHelperTest.cs | 57 +++++++++++++++++++ .../RazorPagesWebSite/SimpleForms.cshtml | 3 +- 4 files changed, 82 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.TagHelpers/FormTagHelper.cs b/src/Microsoft.AspNetCore.Mvc.TagHelpers/FormTagHelper.cs index 44622b4aa2..33d398055b 100644 --- a/src/Microsoft.AspNetCore.Mvc.TagHelpers/FormTagHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.TagHelpers/FormTagHelper.cs @@ -157,10 +157,6 @@ public override void Process(TagHelperContext context, TagHelperOutput output) { output.CopyHtmlAttribute(nameof(Method), context); } - else - { - Method = "get"; - } var antiforgeryDefault = true; @@ -205,9 +201,16 @@ public override void Process(TagHelperContext context, TagHelperOutput output) if (string.IsNullOrEmpty(attributeValue)) { // User is using the FormTagHelper like a normal
tag that has an empty or complex IHtmlContent action attribute. - // e.g. or + // e.g. or - // 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 @@ public override void Process(TagHelperContext context, TagHelperOutput output) } 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
. Let it flow to the output as-is and only handle anti-forgery. + // A
tag that doesn't utilize asp-* attributes. Let it flow to the output. + Method = Method ?? "get"; } else if (pageLink) { @@ -304,11 +308,11 @@ public override void Process(TagHelperContext context, TagHelperOutput output) 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) diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/RazorPagesWebSite.SimpleForms.html b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/RazorPagesWebSite.SimpleForms.html index 27ca37082a..ef06135e61 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/RazorPagesWebSite.SimpleForms.html +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/RazorPagesWebSite.SimpleForms.html @@ -3,4 +3,5 @@
-
\ No newline at end of file +
+
\ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/FormTagHelperTest.cs b/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/FormTagHelperTest.cs index 266e9a45c8..9970667ae4 100644 --- a/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/FormTagHelperTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/FormTagHelperTest.cs @@ -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(), + uniqueId: "test"); + var output = new TagHelperOutput( + expectedTagName, + attributes: new TagHelperAttributeList(), + getChildContentAsync: (useCachedResult, encoder) => + { + var tagHelperContent = new DefaultTagHelperContent(); + tagHelperContent.SetContent("Something"); + return Task.FromResult(tagHelperContent); + }); + var urlHelper = new Mock(); + urlHelper + .Setup(mock => mock.Action(It.IsAny())).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() { diff --git a/test/WebSites/RazorPagesWebSite/SimpleForms.cshtml b/test/WebSites/RazorPagesWebSite/SimpleForms.cshtml index f4ed9e2b4f..535f672120 100644 --- a/test/WebSites/RazorPagesWebSite/SimpleForms.cshtml +++ b/test/WebSites/RazorPagesWebSite/SimpleForms.cshtml @@ -6,4 +6,5 @@
-
\ No newline at end of file +
+
\ No newline at end of file