Change FormTagHelper to apply to all form tags.

- Added functional test to validate that non-attributed form tags have an antiforgery input generated. Re-generated baseline to reflect changes. - Added a unit test to validate that parameterless `FormTagHelper`s behave as expected. #6006
aspnet · Apr 25, 2017 · 3d895a9 · 3d895a9
1 parent ccdaa5a
commit 3d895a9
Show file tree

Hide file tree

Showing 6 changed files with 218 additions and 32 deletions.
diff --git a/src/Microsoft.AspNetCore.Mvc.TagHelpers/FormTagHelper.cs b/src/Microsoft.AspNetCore.Mvc.TagHelpers/FormTagHelper.cs
@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.ComponentModel;
+using Microsoft.AspNetCore.Html;
 using Microsoft.AspNetCore.Mvc.Rendering;
 using Microsoft.AspNetCore.Mvc.ViewFeatures;
 using Microsoft.AspNetCore.Razor.TagHelpers;
@@ -14,15 +15,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
     /// <summary>
     /// <see cref="ITagHelper"/> implementation targeting &lt;form&gt; elements.
     /// </summary>
-    [HtmlTargetElement("form", Attributes = ActionAttributeName)]
-    [HtmlTargetElement("form", Attributes = AntiforgeryAttributeName)]
-    [HtmlTargetElement("form", Attributes = AreaAttributeName)]
-    [HtmlTargetElement("form", Attributes = PageAttributeName)]
-    [HtmlTargetElement("form", Attributes = FragmentAttributeName)]
-    [HtmlTargetElement("form", Attributes = ControllerAttributeName)]
-    [HtmlTargetElement("form", Attributes = RouteAttributeName)]
-    [HtmlTargetElement("form", Attributes = RouteValuesDictionaryName)]
-    [HtmlTargetElement("form", Attributes = RouteValuesPrefix + "*")]
+    [HtmlTargetElement("form")]
     public class FormTagHelper : TagHelper
     {
         private const string ActionAttributeName = "asp-action";
@@ -152,15 +145,20 @@ public override void Process(TagHelperContext context, TagHelperOutput output)
             {
                 throw new ArgumentNullException(nameof(output));
             }
+
             if (Method != null)
             {
                 output.CopyHtmlAttribute(nameof(Method), context);
             }
+            else
+            {
+                Method = "get";
+            }
 
             var antiforgeryDefault = true;
 
             // If "action" is already set, it means the user is attempting to use a normal <form>.
-            if (output.Attributes.ContainsName(HtmlActionAttributeName))
+            if (output.Attributes.TryGetAttribute(HtmlActionAttributeName, out var actionAttribute))
             {
                 if (Action != null ||
                     Controller != null ||
@@ -184,9 +182,29 @@ public override void Process(TagHelperContext context, TagHelperOutput output)
                             PageAttributeName));
                 }
 
-                // User is using the FormTagHelper like a normal <form> tag. Antiforgery default should be false to
-                // not force the antiforgery token on the user.
-                antiforgeryDefault = false;
+                string attributeValue = null;
+                switch (actionAttribute.Value)
+                {
+                    case HtmlString htmlString:
+                        attributeValue = htmlString.ToString();
+                        break;
+                    case string stringValue:
+                        attributeValue = stringValue;
+                        break;
+                }
+
+                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">
+
+                    // Antiforgery default is already set to true
+                }
+                else
+                {
+                    // User is likely using the <form> element to submit to another site. Do not send an antiforgery token to unknown sites.
+                    antiforgeryDefault = false;
+                }
             }
             else
             {
@@ -223,8 +241,12 @@ public override void Process(TagHelperContext context, TagHelperOutput output)
                     routeValues["area"] = Area;
                 }
 
-                TagBuilder tagBuilder;
-                if (pageLink)
+                TagBuilder tagBuilder = null;
+                if (Action == null && Controller == null && Route == null && _routeValues == null && Fragment == null && Area == null && Page == null)
+                {
+                    // Empty form tag such as <form></form>. Let it flow to the output as-is and only handle anti-forgery.
+                }
+                else if (pageLink)
                 {
                     tagBuilder = Generator.GeneratePageForm(
                         ViewContext,
@@ -256,13 +278,19 @@ public override void Process(TagHelperContext context, TagHelperOutput output)
                         htmlAttributes: null);
                 }
 
-                output.MergeAttributes(tagBuilder);
-                if (tagBuilder.HasInnerHtml)
+                if (tagBuilder != null)
                 {
-                    output.PostContent.AppendHtml(tagBuilder.InnerHtml);
+                    output.MergeAttributes(tagBuilder);
+                    if (tagBuilder.HasInnerHtml)
+                    {
+                        output.PostContent.AppendHtml(tagBuilder.InnerHtml);
+                    }
                 }
+            }
 
-                antiforgeryDefault = !string.Equals(Method, "get", StringComparison.OrdinalIgnoreCase);
+            if (string.Equals(Method, "get", StringComparison.OrdinalIgnoreCase))
+            {
+                antiforgeryDefault = false;
             }
 
             if (Antiforgery ?? antiforgeryDefault)

diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/AntiforgeryTestHelper.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/AntiforgeryTestHelper.cs
@@ -19,21 +19,15 @@ public static string RetrieveAntiforgeryToken(string htmlContent, string actionU
 
             foreach (var form in htmlDocument.Descendants("form"))
             {
-                foreach (var attribute in form.Attributes())
+                foreach (var input in form.Descendants("input"))
                 {
-                    if (string.Equals(attribute.Name.LocalName, "action", StringComparison.OrdinalIgnoreCase))
+                    if (input.Attribute("name") != null &&
+                        input.Attribute("type") != null &&
+                        input.Attribute("type").Value == "hidden" &&
+                        (input.Attribute("name").Value == "__RequestVerificationToken" ||
+                         input.Attribute("name").Value == "HtmlEncode[[__RequestVerificationToken]]"))
                     {
-                        foreach (var input in form.Descendants("input"))
-                        {
-                            if (input.Attribute("name") != null &&
-                                input.Attribute("type") != null &&
-                                input.Attribute("type").Value == "hidden" &&
-                                (input.Attribute("name").Value == "__RequestVerificationToken" ||
-                                 input.Attribute("name").Value == "HtmlEncode[[__RequestVerificationToken]]"))
-                            {
-                                return input.Attributes("value").First().Value;
-                            }
-                        }
+                        return input.Attributes("value").First().Value;
                     }
                 }
             }

diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs
@@ -6,20 +6,53 @@
 using System.Linq;
 using System.Net;
 using System.Net.Http;
+using System.Net.Http.Headers;
+using System.Reflection;
 using System.Threading.Tasks;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Mvc.FunctionalTests
 {
     public class RazorPagesTest : IClassFixture<MvcTestFixture<RazorPagesWebSite.Startup>>
     {
+        private static readonly Assembly _resourcesAssembly = typeof(RazorPagesTest).GetTypeInfo().Assembly;
+
         public RazorPagesTest(MvcTestFixture<RazorPagesWebSite.Startup> fixture)
         {
             Client = fixture.Client;
         }
 
         public HttpClient Client { get; }
 
+        [Fact]
+        public async Task Page_SimpleForms_RenderAntiforgery()
+        {
+            // Arrange
+            var expectedMediaType = MediaTypeHeaderValue.Parse("text/html; charset=utf-8");
+            var outputFile = "compiler/resources/RazorPagesWebSite.SimpleForms.html";
+            var expectedContent = await ResourceFile.ReadResourceAsync(_resourcesAssembly, outputFile, sourceFile: false);
+
+            // Act
+            var response = await Client.GetAsync("http://localhost/SimpleForms");
+            var responseContent = await response.Content.ReadAsStringAsync();
+
+            // Assert
+            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            Assert.Equal(expectedMediaType, response.Content.Headers.ContentType);
+
+            responseContent = responseContent.Trim();
+
+            var forgeryToken = AntiforgeryTestHelper.RetrieveAntiforgeryToken(responseContent, "SimpleForms");
+#if GENERATE_BASELINES
+            // Reverse usual substitution and insert a format item into the new file content.
+            responseContent = responseContent.Replace(forgeryToken, "{0}");
+            ResourceFile.UpdateFile(_resourcesAssembly, outputFile, expectedContent, responseContent);
+#else
+            expectedContent = string.Format(expectedContent, forgeryToken);
+            Assert.Equal(expectedContent.Trim(), responseContent, ignoreLineEndingDifferences: true);
+#endif
+        }
+
         [Fact]
         public async Task Page_Handler_HandlerFromQueryString()
         {

diff --git a/...soft.AspNetCore.Mvc.FunctionalTests/compiler/resources/RazorPagesWebSite.SimpleForms.html b/...soft.AspNetCore.Mvc.FunctionalTests/compiler/resources/RazorPagesWebSite.SimpleForms.html
@@ -0,0 +1,6 @@
+<form></form>
+<form method="get"></form>
+<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>
diff --git a/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/FormTagHelperTest.cs b/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/FormTagHelperTest.cs
@@ -25,6 +25,122 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
 {
     public class FormTagHelperTest
     {
+        [Fact]
+        public async Task ProcessAsync_EmptyHtmlStringActionGeneratesAntiforgery()
+        {
+            // Arrange
+            var expectedTagName = "form";
+            var metadataProvider = new TestModelMetadataProvider();
+            var tagHelperContext = new TagHelperContext(
+                tagName: "form",
+                allAttributes: new TagHelperAttributeList()
+                {
+                    { "method", new HtmlString("post") }
+                },
+                items: new Dictionary<object, object>(),
+                uniqueId: "test");
+            var output = new TagHelperOutput(
+                expectedTagName,
+                attributes: new TagHelperAttributeList()
+                {
+                    { "action", HtmlString.Empty },
+                },
+                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,
+                Method = "post",
+            };
+
+            // Act
+            await formTagHelper.ProcessAsync(tagHelperContext, output);
+
+            // Assert
+            var attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("action"));
+            Assert.Equal(HtmlString.Empty, attribute.Value);
+            Assert.Empty(output.PreElement.GetContent());
+            Assert.Empty(output.PreContent.GetContent());
+            Assert.Empty(output.Content.GetContent());
+            Assert.Equal(expectedPostContent, output.PostContent.GetContent());
+            Assert.Empty(output.PostElement.GetContent());
+            Assert.Equal(expectedTagName, output.TagName);
+        }
+
+        [Fact]
+        public async Task ProcessAsync_EmptyStringActionGeneratesAntiforgery()
+        {
+            // Arrange
+            var expectedTagName = "form";
+            var metadataProvider = new TestModelMetadataProvider();
+            var tagHelperContext = new TagHelperContext(
+                tagName: "form",
+                allAttributes: new TagHelperAttributeList()
+                {
+                    { "method", new HtmlString("post") }
+                },
+                items: new Dictionary<object, object>(),
+                uniqueId: "test");
+            var output = new TagHelperOutput(
+                expectedTagName,
+                attributes: new TagHelperAttributeList()
+                {
+                    { "action", string.Empty },
+                },
+                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,
+                Method = "post",
+            };
+
+            // Act
+            await formTagHelper.ProcessAsync(tagHelperContext, output);
+
+            // Assert
+            var attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("action"));
+            Assert.Equal(string.Empty, attribute.Value);
+            Assert.Empty(output.PreElement.GetContent());
+            Assert.Empty(output.PreContent.GetContent());
+            Assert.Empty(output.Content.GetContent());
+            Assert.Equal(expectedPostContent, output.PostContent.GetContent());
+            Assert.Empty(output.PostElement.GetContent());
+            Assert.Equal(expectedTagName, output.TagName);
+        }
+
         [Fact]
         public async Task ProcessAsync_GeneratesExpectedOutput()
         {

diff --git a/test/WebSites/RazorPagesWebSite/SimpleForms.cshtml b/test/WebSites/RazorPagesWebSite/SimpleForms.cshtml
@@ -0,0 +1,9 @@
+@page
+@addTagHelper "*, Microsoft.AspNetCore.Mvc.TagHelpers"
+
+<form></form>
+<form method="get"></form>
+<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>