Skip to content

Commit 56fae38

Browse files
authored
Fix attribute encoding for new component generation (#311)
1 parent 98fdfcd commit 56fae38

File tree

14 files changed

+65
-60
lines changed

14 files changed

+65
-60
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ The `asp-for` attribute is now obsolete; the `for` attribute should be used in i
99

1010
### Fixes
1111

12+
#### Attribute encoding
13+
Newly-refactored tag helpers now correctly encode their attributes.
14+
1215
#### Source map errors
1316
The hosted CSS and JavaScript files no longer have source maps.
1417
Any console errors from browsers failing to download the referenced files should be eliminated.

src/GovUk.Frontend.AspNetCore/ComponentGeneration/ButtonOptions.cs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ namespace GovUk.Frontend.AspNetCore.ComponentGeneration;
66

77
public class ButtonOptions
88
{
9-
public IHtmlContent? Element { get; set; }
9+
public string? Element { get; set; }
1010
public IHtmlContent? Html { get; set; }
1111
public string? Text { get; set; }
1212
public IHtmlContent? Name { get; set; }
@@ -22,14 +22,12 @@ public class ButtonOptions
2222

2323
internal void Validate()
2424
{
25-
var elementStr = Element?.ToHtmlString();
26-
27-
if (elementStr is not null && elementStr != "a" && elementStr != "button" && elementStr != "input")
25+
if (Element is not null && Element != "a" && Element != "button" && Element != "input")
2826
{
2927
throw new InvalidOptionsException(GetType(), $"{nameof(Element)} must be 'a', 'button', or 'input'.");
3028
}
3129

32-
if (elementStr == "input" && IsStartButton == true)
30+
if (Element == "input" && IsStartButton == true)
3331
{
3432
throw new InvalidOptionsException(GetType(), $"{nameof(IsStartButton)} cannot be specified for 'input' elements.");
3533
}
@@ -39,12 +37,12 @@ internal void Validate()
3937
throw new InvalidOptionsException(GetType(), $"{nameof(PreventDoubleClick)} can only be specified for 'button' or 'input' elements.");
4038
}
4139

42-
if (elementStr == "a" && Disabled is not null)
40+
if (Element == "a" && Disabled is not null)
4341
{
4442
throw new InvalidOptionsException(GetType(), $"{nameof(Disabled)} cannot be specified for 'a' elements.");
4543
}
4644

47-
if (Html.NormalizeEmptyString() is not null && elementStr == "input")
45+
if (Html.NormalizeEmptyString() is not null && Element == "input")
4846
{
4947
throw new InvalidOptionsException(GetType(), $"{nameof(Html)} cannot be specified for 'input' elements.");
5048
}
@@ -56,5 +54,5 @@ internal void Validate()
5654
}
5755

5856
internal string GetElement() =>
59-
Element?.ToHtmlString().NormalizeEmptyString() ?? (Href.NormalizeEmptyString() is not null ? "a" : "button");
57+
Element?.NormalizeEmptyString() ?? (Href.NormalizeEmptyString() is not null ? "a" : "button");
6058
}

src/GovUk.Frontend.AspNetCore/TagHelpers/BreadcrumbsTagHelper.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public override async Task ProcessAsync(TagHelperContext context, TagHelperOutpu
6666
Classes = classes,
6767
Attributes = attributes,
6868
Items = breadcrumbsContext.Items,
69-
LabelText = LabelText.ToHtmlContent()
69+
LabelText = LabelText.EncodeHtml()
7070
});
7171

7272
component.WriteTo(output);

src/GovUk.Frontend.AspNetCore/TagHelpers/ButtonLinkTagHelper.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,13 @@ public override async Task ProcessAsync(TagHelperContext context, TagHelperOutpu
5555

5656
var component = _componentGenerator.GenerateButton(new ButtonOptions()
5757
{
58-
Element = Element.ToHtmlContent(),
58+
Element = Element,
5959
Html = content,
6060
Href = href,
6161
Classes = classes,
6262
Attributes = attributes,
6363
IsStartButton = IsStartButton,
64-
Id = Id.ToHtmlContent()
64+
Id = Id.EncodeHtml()
6565
});
6666

6767
component.WriteTo(output);

src/GovUk.Frontend.AspNetCore/TagHelpers/ButtonTagHelper.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,17 +94,17 @@ public override async Task ProcessAsync(TagHelperContext context, TagHelperOutpu
9494

9595
var component = _componentGenerator.GenerateButton(new ButtonOptions()
9696
{
97-
Element = Element.ToHtmlContent(),
97+
Element = Element,
9898
Html = content,
99-
Name = Name.ToHtmlContent(),
100-
Type = Type.ToHtmlContent(),
101-
Value = Value.ToHtmlContent(),
99+
Name = Name.EncodeHtml(),
100+
Type = Type.EncodeHtml(),
101+
Value = Value.EncodeHtml(),
102102
Disabled = Disabled,
103103
Classes = classes,
104104
Attributes = attributes,
105105
PreventDoubleClick = PreventDoubleClick,
106106
IsStartButton = IsStartButton,
107-
Id = Id.ToHtmlContent()
107+
Id = Id.EncodeHtml()
108108
});
109109

110110
component.WriteTo(output);

src/GovUk.Frontend.AspNetCore/TagHelpers/CharacterCountTagHelper.cs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ public override async Task ProcessAsync(TagHelperContext context, TagHelperOutpu
253253
await output.GetChildContentAsync();
254254
}
255255

256-
var name = ResolveName();
256+
var name = ResolveNameUnencoded();
257257
var id = ResolveId(name);
258258
var value = ResolveValue(characterCountContext);
259259
var labelOptions = characterCountContext.GetLabelOptions(For, ViewContext!, _modelHelper, id, AspForAttributeName);
@@ -278,7 +278,7 @@ public override async Task ProcessAsync(TagHelperContext context, TagHelperOutpu
278278

279279
if (Autocomplete is not null)
280280
{
281-
attributes.Add("autocomplete", Autocomplete!.ToHtmlContent()!);
281+
attributes.Add("autocomplete", Autocomplete!.EncodeHtml()!);
282282
}
283283

284284
if (Disabled)
@@ -293,7 +293,7 @@ public override async Task ProcessAsync(TagHelperContext context, TagHelperOutpu
293293
var component = _componentGenerator.GenerateCharacterCount(new CharacterCountOptions
294294
{
295295
Id = id,
296-
Name = name,
296+
Name = name.EncodeHtml(),
297297
Rows = Rows,
298298
Value = value,
299299
MaxLength = MaxLength,
@@ -325,21 +325,21 @@ public override async Task ProcessAsync(TagHelperContext context, TagHelperOutpu
325325
if (errorMessageOptions is not null && context.TryGetContextItem<ContainerErrorContext>(out var containerErrorContext))
326326
{
327327
Debug.Assert(errorMessageOptions.Html is not null);
328-
containerErrorContext.AddError(errorMessageOptions.Html!, href: new HtmlString("#" + id));
328+
containerErrorContext.AddError(errorMessageOptions.Html!, href: new HtmlString("#" + id.ToHtmlString()));
329329
}
330330
}
331331

332-
private IHtmlContent ResolveId(IHtmlContent name)
332+
private IHtmlContent ResolveId(string nameUnencoded)
333333
{
334334
if (Id is not null)
335335
{
336-
return new HtmlString(Id);
336+
return Id.EncodeHtml();
337337
}
338338

339-
return new HtmlString(TagBuilder.CreateSanitizedId(name.ToHtmlString(), Constants.IdAttributeDotReplacement));
339+
return TagBuilder.CreateSanitizedId(nameUnencoded, Constants.IdAttributeDotReplacement).EncodeHtml();
340340
}
341341

342-
private IHtmlContent ResolveName()
342+
private string ResolveNameUnencoded()
343343
{
344344
if (Name is null && For is null)
345345
{
@@ -348,7 +348,7 @@ private IHtmlContent ResolveName()
348348
AspForAttributeName);
349349
}
350350

351-
return new HtmlString(Name ?? _modelHelper.GetFullHtmlFieldName(ViewContext!, For!.Name));
351+
return Name ?? _modelHelper.GetFullHtmlFieldName(ViewContext!, For!.Name);
352352
}
353353

354354
private IHtmlContent? ResolveValue(CharacterCountContext characterCountContext)
@@ -358,6 +358,6 @@ private IHtmlContent ResolveName()
358358
return characterCountContext.Value;
359359
}
360360

361-
return For is not null ? new HtmlString(_modelHelper.GetModelValue(ViewContext!, For.ModelExplorer, For.Name)) : null;
361+
return For is not null ? _modelHelper.GetModelValue(ViewContext!, For.ModelExplorer, For.Name).EncodeHtml() : null;
362362
}
363363
}

src/GovUk.Frontend.AspNetCore/TagHelpers/ErrorMessageTagHelper.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ await output.GetChildContentAsync() :
106106

107107
if (validationMessage != null)
108108
{
109-
resolvedContent = new HtmlString(HtmlEncoder.Default.Encode(validationMessage));
109+
resolvedContent = validationMessage.EncodeHtml();
110110
}
111111
}
112112

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
using System.Diagnostics.CodeAnalysis;
2+
using System.Text.Encodings.Web;
3+
using Microsoft.AspNetCore.Html;
4+
5+
namespace GovUk.Frontend.AspNetCore.TagHelpers;
6+
7+
internal static class Extensions
8+
{
9+
[return: NotNullIfNotNull(nameof(value))]
10+
public static IHtmlContent? EncodeHtml(this string? value) =>
11+
value is not null ? new HtmlString(HtmlEncoder.Default.Encode(value)) : null;
12+
}

src/GovUk.Frontend.AspNetCore/TagHelpers/FormGroupErrorMessageTagHelper.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ private protected virtual void SetErrorMessage(TagHelperContent? childContent, T
6767
else if (context.TryGetContextItem<FormGroupContext2>(out var formGroupContext2))
6868
{
6969
formGroupContext2.SetErrorMessage(
70-
VisuallyHiddenText.ToHtmlContent(),
70+
VisuallyHiddenText.EncodeHtml(),
7171
new EncodedAttributesDictionary(output.Attributes),
7272
childContent?.Snapshot(),
7373
output.TagName);

src/GovUk.Frontend.AspNetCore/TagHelpers/TagHelperExtensions.cs

Lines changed: 0 additions & 9 deletions
This file was deleted.

0 commit comments

Comments
 (0)