Return an `IHtmlContent` from `IAntiforgery.GetHtml()`

- part of aspnet/Mvc#3123
- no longer forces caller to wrap the return value in an `HtmlString`

nit: don't HTML encode the word "hidden"

src/Microsoft.AspNet.Antiforgery/DefaultAntiforgery.cs

@@ -3,8 +3,8 @@
 
 using System;
 using System.Diagnostics;
-using System.Text.Encodings.Web;
 using System.Threading.Tasks;
+using Microsoft.AspNet.Html.Abstractions;
 using Microsoft.AspNet.Http;
 using Microsoft.Extensions.OptionsModel;
 
@@ -16,7 +16,6 @@ namespace Microsoft.AspNet.Antiforgery
     /// </summary>
     public class DefaultAntiforgery : IAntiforgery
     {
-        private readonly HtmlEncoder _htmlEncoder;
         private readonly AntiforgeryOptions _options;
         private readonly IAntiforgeryTokenGenerator _tokenGenerator;
         private readonly IAntiforgeryTokenSerializer _tokenSerializer;
@@ -26,18 +25,16 @@ namespace Microsoft.AspNet.Antiforgery
             IOptions<AntiforgeryOptions> antiforgeryOptionsAccessor,
             IAntiforgeryTokenGenerator tokenGenerator,
             IAntiforgeryTokenSerializer tokenSerializer,
-            IAntiforgeryTokenStore tokenStore,
+            IAntiforgeryTokenStore tokenStore)
-            HtmlEncoder htmlEncoder)
         {
             _options = antiforgeryOptionsAccessor.Value;
             _tokenGenerator = tokenGenerator;
             _tokenSerializer = tokenSerializer;
             _tokenStore = tokenStore;
-            _htmlEncoder = htmlEncoder;
         }
 
         /// <inheritdoc />
-        public string GetHtml(HttpContext context)
+        public IHtmlContent GetHtml(HttpContext context)
         {
             if (context == null)
             {
@@ -48,12 +45,17 @@ namespace Microsoft.AspNet.Antiforgery
 
             var tokenSet = GetAndStoreTokens(context);
 
-            var inputTag = string.Format(
+            // Though FormToken normally contains only US-ASCII letters, numbers, '-', and '_', must assume the
-                "<input name=\"{0}\" type=\"{1}\" value=\"{2}\" />",
+            // IAntiforgeryTokenSerializer implementation has been overridden. Similarly, users may choose a
-                _htmlEncoder.Encode(_options.FormFieldName),
+            // FormFieldName containing almost any character.
-                _htmlEncoder.Encode("hidden"),
+            var content = new HtmlContentBuilder()
-                _htmlEncoder.Encode(tokenSet.FormToken));
+                .AppendHtml("<input name=\"")
-            return inputTag;
+                .Append(_options.FormFieldName)
+                .AppendHtml("\" type=\"hidden\" value=\"")
+                .Append(tokenSet.FormToken)
+                .AppendHtml("\" />");
+
+            return content;
         }
 
         /// <inheritdoc />
@@ -71,6 +73,7 @@ namespace Microsoft.AspNet.Antiforgery
             {
                 SaveCookieTokenAndHeader(context, tokenSet.CookieToken);
             }
+
             return Serialize(tokenSet);
         }
 

src/Microsoft.AspNet.Antiforgery/IAntiforgery.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System.Threading.Tasks;
+using Microsoft.AspNet.Html.Abstractions;
 using Microsoft.AspNet.Http;
 
 namespace Microsoft.AspNet.Antiforgery
@@ -13,18 +14,18 @@ namespace Microsoft.AspNet.Antiforgery
     public interface IAntiforgery
     {
         /// <summary>
-        /// Generates an input field for an antiforgery token. 
+        /// Generates an &lt;input type="hidden"&gt; element for an antiforgery token.
         /// </summary>
         /// <param name="context">The <see cref="HttpContext"/> associated with the current request.</param>
         /// <returns>
-        /// A string containing an &lt;input type="hidden"&gt; element. This element should be put inside
+        /// A <see cref="IHtmlContent"/> containing an &lt;input type="hidden"&gt; element. This element should be put
-        /// a &lt;form&gt;.
+        /// inside a &lt;form&gt;.
         /// </returns>
         /// <remarks>
         /// This method has a side effect:
         /// A response cookie is set if there is no valid cookie associated with the request.
         /// </remarks>
-        string GetHtml(HttpContext context);
+        IHtmlContent GetHtml(HttpContext context);
 
         /// <summary>
         /// Generates an <see cref="AntiforgeryTokenSet"/> for this request and stores the cookie token

src/Microsoft.AspNet.Antiforgery/ServiceCollectionExtensions.cs

@@ -18,7 +18,6 @@ namespace Microsoft.Extensions.DependencyInjection
             }
 
             services.AddDataProtection();
-            services.AddWebEncoders();
 
             // Don't overwrite any options setups that a user may have added.
             services.TryAddEnumerable(

src/Microsoft.AspNet.Antiforgery/project.json

@@ -11,12 +11,12 @@
   },
   "dependencies": {
     "Microsoft.AspNet.DataProtection": "1.0.0-*",
+    "Microsoft.AspNet.Html.Abstractions": "1.0.0-*",
     "Microsoft.AspNet.Http.Abstractions": "1.0.0-*",
-    "Microsoft.AspNet.WebUtilities": "1.0.0-*",
+    "Microsoft.AspNet.WebUtilities": "1.0.0-*"
-    "Microsoft.Extensions.WebEncoders": "1.0.0-*"
   },
   "frameworks": {
-    "net451": {},
+    "dotnet5.4": { },
-    "dotnet5.4": {}
+    "net451": { }
   }
 }

test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTest.cs

@@ -2,8 +2,10 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.IO;
 using System.Security.Claims;
 using System.Threading.Tasks;
+using Microsoft.AspNet.Html.Abstractions;
 using Microsoft.AspNet.Http;
 using Microsoft.AspNet.Http.Internal;
 using Microsoft.Extensions.OptionsModel;
@@ -73,8 +75,7 @@ namespace Microsoft.AspNet.Antiforgery
             var antiforgery = GetAntiforgery(options);
 
             // Act & Assert
-            var exception = Assert.Throws<InvalidOperationException>(
+            var exception = Assert.Throws<InvalidOperationException>(() => antiforgery.GetHtml(httpContext));
-                () => antiforgery.GetHtml(httpContext));
             Assert.Equal(
                  @"The antiforgery system has the configuration value AntiforgeryOptions.RequireSsl = true, " +
                  "but the current request is not an SSL request.",
@@ -159,15 +160,22 @@ namespace Microsoft.AspNet.Antiforgery
             // Make sure the existing cookie is invalid.
             var context = CreateMockContext(options, isOldCookieValid: false);
             var antiforgery = GetAntiforgery(context);
+            var encoder = new HtmlTestEncoder();
 
             // Act
             var inputElement = antiforgery.GetHtml(context.HttpContext);
 
             // Assert
-            Assert.Equal(
+            using (var writer = new StringWriter())
-                @"<input name=""HtmlEncode[[form-field-name]]"" type=""HtmlEncode[[hidden]]"" " +
+            {
-                @"value=""HtmlEncode[[serialized-form-token]]"" />",
+                inputElement.WriteTo(writer, encoder);
-                inputElement);
+
+                Assert.Equal(
+                    @"<input name=""HtmlEncode[[form-field-name]]"" type=""hidden"" " +
+                    @"value=""HtmlEncode[[serialized-form-token]]"" />",
+                    writer.ToString());
+            }
+
             context.TokenStore.Verify();
         }
 
@@ -185,21 +193,31 @@ namespace Microsoft.AspNet.Antiforgery
             var antiforgery = GetAntiforgery(context);
 
             // This will cause the cookieToken to be null.
-            context.TokenStore.Setup(o => o.GetCookieToken(context.HttpContext))
+            context.TokenStore
-                              .Throws(new Exception("should be swallowed"));
+                .Setup(o => o.GetCookieToken(context.HttpContext))
+                .Throws(new Exception("should be swallowed"));
 
             // Setup so that the null cookie token returned is treated as invalid.
-            context.TokenGenerator.Setup(o => o.IsCookieTokenValid(null))
+            context.TokenGenerator
-                                 .Returns(false);
+                .Setup(o => o.IsCookieTokenValid(null))
+                .Returns(false);
+
+            var encoder = new HtmlTestEncoder();
 
             // Act
             var inputElement = antiforgery.GetHtml(context.HttpContext);
 
             // Assert
-            Assert.Equal(
+            using (var writer = new StringWriter())
-                @"<input name=""HtmlEncode[[form-field-name]]"" type=""HtmlEncode[[hidden]]"" " +
+            {
-                @"value=""HtmlEncode[[serialized-form-token]]"" />",
+                inputElement.WriteTo(writer, encoder);
-                inputElement);
+
+                Assert.Equal(
+                    @"<input name=""HtmlEncode[[form-field-name]]"" type=""hidden"" " +
+                    @"value=""HtmlEncode[[serialized-form-token]]"" />",
+                    writer.ToString());
+            }
+
             context.TokenStore.Verify();
         }
 
@@ -215,15 +233,21 @@ namespace Microsoft.AspNet.Antiforgery
             // Make sure the existing cookie is valid and use the same cookie for the mock Token Provider.
             var context = CreateMockContext(options, useOldCookie: true, isOldCookieValid: true);
             var antiforgery = GetAntiforgery(context);
+            var encoder = new HtmlTestEncoder();
 
             // Act
             var inputElement = antiforgery.GetHtml(context.HttpContext);
 
             // Assert
-            Assert.Equal(
+            using (var writer = new StringWriter())
-                @"<input name=""HtmlEncode[[form-field-name]]"" type=""HtmlEncode[[hidden]]"" " +
+            {
-                @"value=""HtmlEncode[[serialized-form-token]]"" />",
+                inputElement.WriteTo(writer, encoder);
-                inputElement);
+
+                Assert.Equal(
+                    @"<input name=""HtmlEncode[[form-field-name]]"" type=""hidden"" " +
+                    @"value=""HtmlEncode[[serialized-form-token]]"" />",
+                    writer.ToString());
+            }
         }
 
         [Theory]
@@ -237,12 +261,12 @@ namespace Microsoft.AspNet.Antiforgery
                 SuppressXFrameOptionsHeader = suppressXFrameOptions
             };
 
-            // Genreate a new cookie.
+            // Generate a new cookie.
             var context = CreateMockContext(options, useOldCookie: false, isOldCookieValid: false);
             var antiforgery = GetAntiforgery(context);
 
             // Act
-            var inputElement = antiforgery.GetHtml(context.HttpContext);
+            antiforgery.GetHtml(context.HttpContext);
 
             // Assert
             string xFrameOptions = context.HttpContext.Response.Headers["X-Frame-Options"];
@@ -253,7 +277,7 @@ namespace Microsoft.AspNet.Antiforgery
         public void GetTokens_ExistingInvalidCookieToken_GeneratesANewCookieTokenAndANewFormToken()
         {
             // Arrange
-            // Genreate a new cookie.
+            // Generate a new cookie.
             var context = CreateMockContext(
                 new AntiforgeryOptions(),
                 useOldCookie: false,
@@ -328,8 +352,9 @@ namespace Microsoft.AspNet.Antiforgery
 
             // Assert
             // We shouldn't have saved the cookie because it already existed.
-            context.TokenStore.Verify(t => t.SaveCookieToken(It.IsAny<HttpContext>(), It.IsAny<AntiforgeryToken>()), Times.Never);
+            context.TokenStore.Verify(
-            
+                t => t.SaveCookieToken(It.IsAny<HttpContext>(), It.IsAny<AntiforgeryToken>()), Times.Never);
+
             Assert.Equal("serialized-old-cookie-token", tokenSet.CookieToken);
             Assert.Equal("serialized-form-token", tokenSet.FormToken);
         }
@@ -348,7 +373,8 @@ namespace Microsoft.AspNet.Antiforgery
             var tokenSet = antiforgery.GetAndStoreTokens(context.HttpContext);
 
             // Assert
-            context.TokenStore.Verify(t => t.SaveCookieToken(It.IsAny<HttpContext>(), It.IsAny<AntiforgeryToken>()), Times.Once);
+            context.TokenStore.Verify(
+                t => t.SaveCookieToken(It.IsAny<HttpContext>(), It.IsAny<AntiforgeryToken>()), Times.Once);
 
             Assert.Equal("serialized-new-cookie-token", tokenSet.CookieToken);
             Assert.Equal("serialized-form-token", tokenSet.FormToken);
@@ -360,22 +386,26 @@ namespace Microsoft.AspNet.Antiforgery
             // Arrange
             var context = CreateMockContext(new AntiforgeryOptions());
 
-            context.TokenSerializer.Setup(o => o.Deserialize("cookie-token"))
+            context.TokenSerializer
-                                   .Returns(context.TestTokenSet.OldCookieToken);
+                .Setup(o => o.Deserialize("cookie-token"))
-            context.TokenSerializer.Setup(o => o.Deserialize("form-token"))
+                .Returns(context.TestTokenSet.OldCookieToken);
-                                   .Returns(context.TestTokenSet.FormToken);
+            context.TokenSerializer
+                .Setup(o => o.Deserialize("form-token"))
+                .Returns(context.TestTokenSet.FormToken);
 
-            context.TokenGenerator.Setup(o => o.ValidateTokens(
+            context.TokenGenerator
-                                                context.HttpContext,
+                .Setup(o => o.ValidateTokens(
-                                                context.TestTokenSet.OldCookieToken, context.TestTokenSet.FormToken))
+                    context.HttpContext,
-                                 .Throws(new InvalidOperationException("my-message"));
+                    context.TestTokenSet.OldCookieToken,
+                    context.TestTokenSet.FormToken))
+                .Throws(new InvalidOperationException("my-message"));
             context.TokenStore = null;
             var antiforgery = GetAntiforgery(context);
 
             // Act & assert
             var exception = Assert.Throws<InvalidOperationException>(
                     () => antiforgery.ValidateTokens(
-                        context.HttpContext, 
+                        context.HttpContext,
                         new AntiforgeryTokenSet("form-token", "cookie-token")));
             Assert.Equal("my-message", exception.Message);
         }
@@ -386,15 +416,19 @@ namespace Microsoft.AspNet.Antiforgery
             // Arrange
             var context = CreateMockContext(new AntiforgeryOptions());
 
-            context.TokenSerializer.Setup(o => o.Deserialize("cookie-token"))
+            context.TokenSerializer
-                                   .Returns(context.TestTokenSet.OldCookieToken);
+                .Setup(o => o.Deserialize("cookie-token"))
-            context.TokenSerializer.Setup(o => o.Deserialize("form-token"))
+                .Returns(context.TestTokenSet.OldCookieToken);
-                                   .Returns(context.TestTokenSet.FormToken);
+            context.TokenSerializer
+                .Setup(o => o.Deserialize("form-token"))
+                .Returns(context.TestTokenSet.FormToken);
 
-            context.TokenGenerator.Setup(o => o.ValidateTokens(
+            context.TokenGenerator
-                                                context.HttpContext,
+                .Setup(o => o.ValidateTokens(
-                                                context.TestTokenSet.OldCookieToken, context.TestTokenSet.FormToken))
+                    context.HttpContext,
-                                 .Verifiable();
+                    context.TestTokenSet.OldCookieToken,
+                    context.TestTokenSet.FormToken))
+                .Verifiable();
             context.TokenStore = null;
             var antiforgery = GetAntiforgery(context);
 
@@ -480,7 +514,7 @@ namespace Microsoft.AspNet.Antiforgery
                 SuppressXFrameOptionsHeader = suppressXFrameOptions
             };
 
-            // Genreate a new cookie.
+            // Generate a new cookie.
             var context = CreateMockContext(options, useOldCookie: false, isOldCookieValid: false);
             var antiforgery = GetAntiforgery(context);
 
@@ -508,8 +542,7 @@ namespace Microsoft.AspNet.Antiforgery
                 antiforgeryOptionsAccessor: optionsManager,
                 tokenGenerator: tokenGenerator,
                 tokenSerializer: tokenSerializer,
-                tokenStore: tokenStore,
+                tokenStore: tokenStore);
-                htmlEncoder: new HtmlTestEncoder());
         }
 
         private HttpContext GetHttpContext()
@@ -522,9 +555,9 @@ namespace Microsoft.AspNet.Antiforgery
         private DefaultAntiforgery GetAntiforgery(AntiforgeryMockContext context)
         {
             return GetAntiforgery(
-                context.Options, 
+                context.Options,
-                context.TokenGenerator?.Object, 
+                context.TokenGenerator?.Object,
-                context.TokenSerializer?.Object, 
+                context.TokenSerializer?.Object,
                 context.TokenStore?.Object);
         }
 
@@ -538,7 +571,7 @@ namespace Microsoft.AspNet.Antiforgery
             var mockTokenStore = new Mock<IAntiforgeryTokenStore>(MockBehavior.Strict);
             mockTokenStore.Setup(o => o.GetCookieToken(context))
                           .Returns(oldCookieToken);
-            
+
             mockTokenStore.Setup(o => o.GetRequestTokensAsync(context))
                           .Returns(() => Task.FromResult(new AntiforgeryTokenSet(
                               testTokenSet.FormTokenString,

test/Microsoft.AspNet.Antiforgery.Test/project.json

@@ -7,6 +7,7 @@
     "Microsoft.AspNet.Antiforgery": "1.0.0-*",
     "Microsoft.AspNet.Http": "1.0.0-*",
     "Microsoft.Extensions.DependencyInjection": "1.0.0-*",
+    "Microsoft.Extensions.WebEncoders": "1.0.0-*",
     "xunit.runner.aspnet": "2.0.0-aspnet-*"
   },
   "commands": {