Skip to content

Commit 1af6879

Browse files
gunndabadgunndabad
committed
Fix attribute encoding for new component generation
1 parent 7f1e129 commit 1af6879

File tree

10 files changed

+54
-45
lines changed

10 files changed

+54
-45
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ on:
55
branches:
66
- dev
77
- main
8+
- v*
89
tags:
910
- v*
1011
paths-ignore:

‎CHANGELOG.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
# Changelog
22

3+
## 2.8.1
4+
5+
### Fixes
6+
7+
#### Attribute encoding
8+
Newly-refactored tag helpers now correctly encode their attributes.
9+
310
## 2.8.0
411

512
Targets GOV.UK Frontend v5.8.0.

‎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 record 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 record 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/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(AspFor, 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 && AspFor is null)
345345
{
@@ -348,7 +348,7 @@ private IHtmlContent ResolveName()
348348
AspForAttributeName);
349349
}
350350

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

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

361-
return AspFor != null ? new HtmlString(_modelHelper.GetModelValue(ViewContext!, AspFor.ModelExplorer, AspFor.Name)) : null;
361+
return AspFor != null ? _modelHelper.GetModelValue(ViewContext!, AspFor.ModelExplorer, AspFor.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
@@ -98,7 +98,7 @@ await output.GetChildContentAsync() :
9898

9999
if (validationMessage != null)
100100
{
101-
resolvedContent = new HtmlString(HtmlEncoder.Default.Encode(validationMessage));
101+
resolvedContent = validationMessage.EncodeHtml();
102102
}
103103
}
104104

‎src/GovUk.Frontend.AspNetCore/TagHelpers/Extensions.cs‎

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.

‎src/GovUk.Frontend.AspNetCore/TagHelpers/TextInputTagHelper.cs‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ public override async Task ProcessAsync(TagHelperContext context, TagHelperOutpu
226226
await output.GetChildContentAsync();
227227
}
228228

229-
var name = ResolveName();
229+
var name = ResolveNameUnencoded();
230230
var id = ResolveId(name);
231231
var value = ResolveValue();
232232
var labelOptions = textInputContext.GetLabelOptions(AspFor, ViewContext!, _modelHelper, id, AspForAttributeName);
@@ -258,23 +258,23 @@ public override async Task ProcessAsync(TagHelperContext context, TagHelperOutpu
258258
var component = _componentGenerator.GenerateTextInput(new TextInputOptions()
259259
{
260260
Id = id,
261-
Name = name,
262-
Type = Type.ToHtmlContent(),
263-
Inputmode = InputMode.ToHtmlContent(),
261+
Name = name.EncodeHtml(),
262+
Type = Type.EncodeHtml(),
263+
Inputmode = InputMode.EncodeHtml(),
264264
Value = value,
265265
Disabled = Disabled,
266-
DescribedBy = DescribedBy.ToHtmlContent(),
266+
DescribedBy = DescribedBy.EncodeHtml(),
267267
Label = labelOptions,
268268
Hint = hintOptions,
269269
ErrorMessage = errorMessageOptions,
270270
Prefix = prefixOptions,
271271
Suffix = suffixOptions,
272272
FormGroup = formGroupOptions,
273273
Classes = classes,
274-
Autocomplete = Autocomplete.ToHtmlContent(),
275-
Pattern = Pattern.ToHtmlContent(),
274+
Autocomplete = Autocomplete.EncodeHtml(),
275+
Pattern = Pattern.EncodeHtml(),
276276
Spellcheck = Spellcheck,
277-
Autocapitalize = Autocapitalize.ToHtmlContent(),
277+
Autocapitalize = Autocapitalize.EncodeHtml(),
278278
InputWrapper = new TextInputOptionsInputWrapper()
279279
{
280280
Classes = inputWrapperClasses,
@@ -288,21 +288,21 @@ public override async Task ProcessAsync(TagHelperContext context, TagHelperOutpu
288288
if (errorMessageOptions is not null && context.TryGetContextItem<ContainerErrorContext>(out var containerErrorContext))
289289
{
290290
Debug.Assert(errorMessageOptions.Html is not null);
291-
containerErrorContext.AddError(errorMessageOptions.Html!, href: new HtmlString("#" + id));
291+
containerErrorContext.AddError(errorMessageOptions.Html!, href: new HtmlString("#" + id.ToHtmlString()));
292292
}
293293
}
294294

295-
private IHtmlContent ResolveId(IHtmlContent name)
295+
private IHtmlContent ResolveId(string nameUnencoded)
296296
{
297297
if (Id is not null)
298298
{
299299
return new HtmlString(Id);
300300
}
301301

302-
return new HtmlString(TagBuilder.CreateSanitizedId(name.ToHtmlString(), Constants.IdAttributeDotReplacement));
302+
return TagBuilder.CreateSanitizedId(nameUnencoded, Constants.IdAttributeDotReplacement).EncodeHtml();
303303
}
304304

305-
private IHtmlContent ResolveName()
305+
private string ResolveNameUnencoded()
306306
{
307307
if (Name is null && AspFor is null)
308308
{
@@ -311,16 +311,16 @@ private IHtmlContent ResolveName()
311311
AspForAttributeName);
312312
}
313313

314-
return new HtmlString(Name ?? _modelHelper.GetFullHtmlFieldName(ViewContext!, AspFor!.Name));
314+
return Name ?? _modelHelper.GetFullHtmlFieldName(ViewContext!, AspFor!.Name);
315315
}
316316

317317
private IHtmlContent? ResolveValue()
318318
{
319319
if (_valueSpecified)
320320
{
321-
return new HtmlString(_value);
321+
return _value.EncodeHtml();
322322
}
323323

324-
return AspFor != null ? new HtmlString(_modelHelper.GetModelValue(ViewContext!, AspFor.ModelExplorer, AspFor.Name)) : null;
324+
return AspFor != null ? _modelHelper.GetModelValue(ViewContext!, AspFor.ModelExplorer, AspFor.Name).EncodeHtml() : null;
325325
}
326326
}

0 commit comments

Comments
 (0)