fix: Escape text inside html tag and attributes

* fix: Added tests for escpable characters

* fix: Escape text inside html tag

* Added changelog entry

* chore: code cleanup

* fix: optimize char replacement

---------

Co-authored-by: Egil Hansen <[email protected]>

Author: linkdotnet

CHANGELOG.md

@@ -6,6 +6,10 @@ All notable changes to **bUnit** will be documented in this file. The project ad
 
 ## [Unreleased]
 
+### Changed
+
+-   Some characters where not properly escaped. Reported by [@pwhe23](https://github.com/pwhe23). Fixed by [@linkdotnet](https://github.com/linkdotnet).
+
 ## [1.17.2] - 2023-02-22
 
 -   Submit buttons and input fields now no longer cause a form submit when they have the `@onclick:preventDefault` attribute. By [@JelleHissink](https://github.com/JelleHissink).

src/bunit.web/Rendering/Internal/Htmlizer.cs

@@ -5,8 +5,6 @@
 using System.Diagnostics;
 using System.Globalization;
 using System.Text;
-using System.Text.Encodings.Web;
-using System.Text.Unicode;
 using Bunit.Rendering;
 
 namespace Bunit;
@@ -22,30 +20,34 @@ internal static class Htmlizer
 	internal const string BlazorAttrPrefix = "blazor:";
 	internal const string ElementReferenceAttrName = BlazorAttrPrefix + "elementReference";
 
-	private static readonly HashSet<string> SelfClosingElements = new(StringComparer.OrdinalIgnoreCase)
-	{
-		"area",
-		"base",
-		"br",
-		"col",
-		"embed",
-		"hr",
-		"img",
-		"input",
-		"link",
-		"meta",
-		"param",
-		"source",
-		"track",
-		"wbr",
-	};
+	private static readonly HashSet<string> SelfClosingElements =
+		new(StringComparer.OrdinalIgnoreCase)
+		{
+			"area",
+			"base",
+			"br",
+			"col",
+			"embed",
+			"hr",
+			"img",
+			"input",
+			"link",
+			"meta",
+			"param",
+			"source",
+			"track",
+			"wbr",
+		};
 
 	public static bool IsBlazorAttribute(string attributeName)
 	{
 		if (attributeName is null)
+		{
 			throw new ArgumentNullException(nameof(attributeName));
-		return attributeName.StartsWith(BlazorAttrPrefix, StringComparison.Ordinal) ||
-					  attributeName.StartsWith(BlazorCssScopeAttrPrefix, StringComparison.Ordinal);
+		}
+
+		return attributeName.StartsWith(BlazorAttrPrefix, StringComparison.Ordinal)
+			|| attributeName.StartsWith(BlazorCssScopeAttrPrefix, StringComparison.Ordinal);
 	}
 
 	public static string ToBlazorAttribute(string attributeName)
@@ -58,11 +60,19 @@ public static string GetHtml(int componentId, RenderTreeFrameDictionary framesCo
 		var context = new HtmlRenderingContext(framesCollection);
 		var frames = context.GetRenderTreeFrames(componentId);
 		var newPosition = RenderFrames(context, frames, 0, frames.Length);
-		Debug.Assert(newPosition == frames.Length, $"frames.Length = {frames.Length}. newPosition = {newPosition}");
+		Debug.Assert(
+			newPosition == frames.Length,
+			$"frames.Length = {frames.Length}. newPosition = {newPosition}"
+		);
 		return context.Result.ToString();
 	}
 
-	private static int RenderFrames(HtmlRenderingContext context, ReadOnlySpan<RenderTreeFrame> frames, int position, int maxElements)
+	private static int RenderFrames(
+		HtmlRenderingContext context,
+		ReadOnlySpan<RenderTreeFrame> frames,
+		int position,
+		int maxElements
+	)
 	{
 		var nextPosition = position;
 		var endPosition = position + maxElements;
@@ -84,17 +94,20 @@ private static int RenderFrames(HtmlRenderingContext context, ReadOnlySpan<Rende
 	private static int RenderCore(
 		HtmlRenderingContext context,
 		ReadOnlySpan<RenderTreeFrame> frames,
-		int position)
+		int position
+	)
 	{
 		var frame = frames[position];
 		switch (frame.FrameType)
 		{
 			case RenderTreeFrameType.Element:
 				return RenderElement(context, frames, position);
 			case RenderTreeFrameType.Attribute:
-				throw new InvalidOperationException($"Attributes should only be encountered within {nameof(RenderElement)}");
+				throw new InvalidOperationException(
+					$"Attributes should only be encountered within {nameof(RenderElement)}"
+				);
 			case RenderTreeFrameType.Text:
-				context.Result.Append(frame.TextContent);
+				AppendEscapeText(context, frame.TextContent);
 				return position + 1;
 			case RenderTreeFrameType.Markup:
 				context.Result.Append(frame.MarkupContent);
@@ -107,14 +120,17 @@ private static int RenderCore(
 			case RenderTreeFrameType.ComponentReferenceCapture:
 				return position + 1;
 			default:
-				throw new InvalidOperationException($"Invalid element frame type '{frame.FrameType}'.");
+				throw new InvalidOperationException(
+					$"Invalid element frame type '{frame.FrameType}'."
+				);
 		}
 	}
 
 	private static int RenderChildComponent(
 		HtmlRenderingContext context,
 		ReadOnlySpan<RenderTreeFrame> frames,
-		int position)
+		int position
+	)
 	{
 		var frame = frames[position];
 		var childFrames = context.GetRenderTreeFrames(frame.ComponentId);
@@ -125,20 +141,33 @@ private static int RenderChildComponent(
 	private static int RenderElement(
 		HtmlRenderingContext context,
 		ReadOnlySpan<RenderTreeFrame> frames,
-		int position)
+		int position
+	)
 	{
 		var frame = frames[position];
 		var result = context.Result;
 		result.Append('<');
 		result.Append(frame.ElementName);
-		var afterAttributes = RenderAttributes(context, frames, position + 1, frame.ElementSubtreeLength - 1, out var capturedValueAttribute);
+		var afterAttributes = RenderAttributes(
+			context,
+			frames,
+			position + 1,
+			frame.ElementSubtreeLength - 1,
+			out var capturedValueAttribute
+		);
 
 		// When we see an <option> as a descendant of a <select>, and the option's "value" attribute matches the
 		// "value" attribute on the <select>, then we auto-add the "selected" attribute to that option. This is
 		// a way of converting Blazor's select binding feature to regular static HTML.
-		if (context.ClosestSelectValueAsString != null
+		if (
+			context.ClosestSelectValueAsString != null
 			&& string.Equals(frame.ElementName, "option", StringComparison.OrdinalIgnoreCase)
-			&& string.Equals(capturedValueAttribute, context.ClosestSelectValueAsString, StringComparison.Ordinal))
+			&& string.Equals(
+				capturedValueAttribute,
+				context.ClosestSelectValueAsString,
+				StringComparison.Ordinal
+			)
+		)
 		{
 			result.Append(" selected");
 		}
@@ -148,7 +177,11 @@ private static int RenderElement(
 		{
 			result.Append('>');
 
-			var isSelect = string.Equals(frame.ElementName, "select", StringComparison.OrdinalIgnoreCase);
+			var isSelect = string.Equals(
+				frame.ElementName,
+				"select",
+				StringComparison.OrdinalIgnoreCase
+			);
 			if (isSelect)
 			{
 				context.ClosestSelectValueAsString = capturedValueAttribute;
@@ -181,11 +214,19 @@ private static int RenderElement(
 			result.Append('>');
 		}
 
-		Debug.Assert(afterAttributes == position + frame.ElementSubtreeLength, $"afterAttributes = {afterAttributes}. position = {position}. frame.ElementSubtreeLength = {frame.ElementSubtreeLength}");
+		Debug.Assert(
+			afterAttributes == position + frame.ElementSubtreeLength,
+			$"afterAttributes = {afterAttributes}. position = {position}. frame.ElementSubtreeLength = {frame.ElementSubtreeLength}"
+		);
 		return afterAttributes;
 	}
 
-	private static int RenderChildren(HtmlRenderingContext context, ReadOnlySpan<RenderTreeFrame> frames, int position, int maxElements)
+	private static int RenderChildren(
+		HtmlRenderingContext context,
+		ReadOnlySpan<RenderTreeFrame> frames,
+		int position,
+		int maxElements
+	)
 	{
 		if (maxElements == 0)
 		{
@@ -195,13 +236,13 @@ private static int RenderChildren(HtmlRenderingContext context, ReadOnlySpan<Ren
 		return RenderFrames(context, frames, position, maxElements);
 	}
 
-	[SuppressMessage("Design", "MA0051:Method is too long", Justification = "TODO: Refactor")]
 	private static int RenderAttributes(
 		HtmlRenderingContext context,
 		ReadOnlySpan<RenderTreeFrame> frames,
 		int position,
 		int maxElements,
-		out string? capturedValueAttribute)
+		out string? capturedValueAttribute
+	)
 	{
 		capturedValueAttribute = null;
 
@@ -251,11 +292,19 @@ private static int RenderAttributes(
 
 			switch (frame.AttributeValue)
 			{
-				case bool flag when flag && frame.AttributeName.StartsWith(BlazorInternalAttrPrefix, StringComparison.Ordinal):
+				case bool flag
+					when flag
+						&& frame.AttributeName.StartsWith(
+							BlazorInternalAttrPrefix,
+							StringComparison.Ordinal
+						):
 					// NOTE: This was added to make it more obvious
 					// that this is a generated/special blazor attribute
 					// for internal usage
-					var nameParts = frame.AttributeName.Split('_', StringSplitOptions.RemoveEmptyEntries);
+					var nameParts = frame.AttributeName.Split(
+						'_',
+						StringSplitOptions.RemoveEmptyEntries
+					);
 					result.Append(' ');
 					result.Append(BlazorAttrPrefix);
 					result.Append(nameParts[2]);
@@ -271,7 +320,7 @@ private static int RenderAttributes(
 					result.Append(frame.AttributeName);
 					result.Append('=');
 					result.Append('"');
-					result.Append(Escape(value));
+					AppendEscapeAttributeValue(context, value);
 					result.Append('"');
 					break;
 				default:
@@ -282,10 +331,66 @@ private static int RenderAttributes(
 		return position + maxElements;
 	}
 
-	private static string Escape(string value) =>
-		value
-			.Replace("&", "&amp;", StringComparison.OrdinalIgnoreCase)
-			.Replace("\"", "&quot;", StringComparison.OrdinalIgnoreCase);
+	private static void AppendEscapeText(HtmlRenderingContext context, string value)
+	{
+		var valueSpan = value.AsSpan();
+		var copyFromIndex = 0;
+		for (var index = 0; index < valueSpan.Length; index++)
+		{
+			var c = valueSpan[index];
+			if (c is '<' or '>' or '&')
+			{
+				context.Result.Append(valueSpan.Slice(copyFromIndex, index - copyFromIndex));
+				switch (c)
+				{
+					case '<':
+						context.Result.Append("&lt;");
+						break;
+					case '>':
+						context.Result.Append("&gt;");
+						break;
+					case '&':
+						context.Result.Append("&amp;");
+						break;
+				}
+				copyFromIndex = index + 1;
+			}
+		}
+
+		if (copyFromIndex < valueSpan.Length)
+		{
+			context.Result.Append(valueSpan.Slice(copyFromIndex));
+		}
+	}
+
+	private static void AppendEscapeAttributeValue(HtmlRenderingContext context, string value)
+	{
+		var valueSpan = value.AsSpan();
+		var copyFromIndex = 0;
+		for (var index = 0; index < valueSpan.Length; index++)
+		{
+			var c = valueSpan[index];
+			if (c is '"' or '&')
+			{
+				context.Result.Append(valueSpan.Slice(copyFromIndex, index - copyFromIndex));
+				switch (c)
+				{
+					case '"':
+						context.Result.Append("&quot;");
+						break;
+					case '&':
+						context.Result.Append("&amp;");
+						break;
+				}
+				copyFromIndex = index + 1;
+			}
+		}
+
+		if (copyFromIndex < valueSpan.Length)
+		{
+			context.Result.Append(valueSpan.Slice(copyFromIndex));
+		}
+	}
 
 	private sealed class HtmlRenderingContext
 	{
@@ -296,8 +401,8 @@ public HtmlRenderingContext(RenderTreeFrameDictionary frames)
 			this.frames = frames;
 		}
 
-		public ReadOnlySpan<RenderTreeFrame> GetRenderTreeFrames(int componentId)
-			=> new(frames[componentId].Array, 0, frames[componentId].Count);
+		public ReadOnlySpan<RenderTreeFrame> GetRenderTreeFrames(int componentId) =>
+			new(frames[componentId].Array, 0, frames[componentId].Count);
 
 		public StringBuilder Result { get; } = new();
 

tests/bunit.testassets/BlazorE2E/ComponentWithEscapableCharacters.razor

@@ -1,8 +1,6 @@
-<p style="@Escaped">@Escaped</p>
+<p some-attribute="@Escaped">@Escaped</p>
 
 @code {
-
 	[Parameter]
 	public string Escaped { get; set; }
-
 }

tests/bunit.web.tests/BlazorE2E/ComponentRenderingTest.cs

@@ -690,14 +690,16 @@ public async Task CanHandleRemovedParentObjectsAsync()
 		cut.FindAll("div").Count.ShouldBe(0);
 	}
 
-	[Fact]
-	public void SomeEscapableCharactersDontGetEncoded()
+	[Theory]
+	[InlineData("<b>Message</b>", "<p some-attribute=\"<b>Message</b>\">&lt;b&gt;Message&lt;/b&gt;</p>")]
+	[InlineData("url('&*')", "<p some-attribute=\"url('&amp;*')\">url('&amp;*')</p>")]
+	[InlineData("<\"*?_'", "<p some-attribute=\"<&quot;*?_'\">&lt;\"*?_'</p>")]
+	public void SomeEscapableCharactersDontGetEncoded(string input, string expected)
 	{
-		const string input = "url('\"&')";
 		var cut = RenderComponent<ComponentWithEscapableCharacters>(
 			p => p.Add(s => s.Escaped, input));
 
-		cut.Markup.ShouldBe("<p style=\"url('&quot;&amp;')\">url('\"&')</p>");
+		cut.Markup.ShouldBe(expected);
 	}
 
 	[Fact]