Skip to content

Commit d70a207

Browse files
nikolajlauridsennikolajlauridsen
authored
V8: Add ability to implement your own HtmlSanitizer (#11897)
* Add IHtmlSanitizer * Use IHtmlSanitizer in RTE value editor * Add docstrings to IHtmlSanitizer * Rename NoOp to Noop To match the rest of the classes * Fix tests
1 parent c60d8c8 commit d70a207

File tree

7 files changed

+104
-13
lines changed

7 files changed

+104
-13
lines changed

‎src/Umbraco.Core/Composing/CompositionExtensions/Services.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using Umbraco.Core.IO;
77
using Umbraco.Core.Logging;
88
using Umbraco.Core.Packaging;
9+
using Umbraco.Core.Security;
910
using Umbraco.Core.Services;
1011
using Umbraco.Core.Services.Implement;
1112
using Umbraco.Core.Telemetry;
@@ -81,6 +82,7 @@ public static Composition ComposeServices(this Composition composition)
8182
new DirectoryInfo(IOHelper.GetRootDirectorySafe())));
8283

8384
composition.RegisterUnique<ITelemetryService, TelemetryService>();
85+
composition.RegisterUnique<IHtmlSanitizer, NoopHtmlSanitizer>();
8486

8587
return composition;
8688
}

‎src/Umbraco.Core/Security/IHtmlSanitizer.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
namespace Umbraco.Core.Security
2+
{
3+
public interface IHtmlSanitizer
4+
{
5+
/// <summary>
6+
/// Sanitizes HTML
7+
/// </summary>
8+
/// <param name="html">HTML to be sanitized</param>
9+
/// <returns>Sanitized HTML</returns>
10+
string Sanitize(string html);
11+
}
12+
}

‎src/Umbraco.Core/Security/NoopHtmlSanitizer.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
namespace Umbraco.Core.Security
2+
{
3+
public class NoopHtmlSanitizer : IHtmlSanitizer
4+
{
5+
public string Sanitize(string html)
6+
{
7+
return html;
8+
}
9+
}
10+
}

‎src/Umbraco.Core/Umbraco.Core.csproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,8 @@
194194
<Compile Include="PropertyEditors\PropertyCacheCompression.cs" />
195195
<Compile Include="PropertyEditors\IPropertyCacheCompression.cs" />
196196
<Compile Include="PropertyEditors\UnPublishedContentPropertyCacheCompressionOptions.cs" />
197+
<Compile Include="Security\IHtmlSanitizer.cs" />
198+
<Compile Include="Security\NoopHtmlSanitizer.cs" />
197199
<Compile Include="Serialization\AutoInterningStringConverter.cs" />
198200
<Compile Include="Serialization\AutoInterningStringKeyCaseInsensitiveDictionaryConverter.cs" />
199201
<Compile Include="PropertyEditors\EyeDropperColorPickerConfiguration.cs" />

‎src/Umbraco.Tests/PublishedContent/PublishedContentTests.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
using Umbraco.Core.Configuration;
1717
using Umbraco.Core.Logging;
1818
using Umbraco.Core.Models;
19+
using Umbraco.Core.Security;
1920
using Umbraco.Core.Services;
2021
using Umbraco.Tests.TestHelpers;
2122
using Umbraco.Tests.Testing;
@@ -54,7 +55,7 @@ protected override void Compose()
5455
var dataTypeService = new TestObjects.TestDataTypeService(
5556
new DataType(new VoidEditor(logger)) { Id = 1 },
5657
new DataType(new TrueFalsePropertyEditor(logger)) { Id = 1001 },
57-
new DataType(new RichTextPropertyEditor(logger, umbracoContextAccessor, imageSourceParser, linkParser, pastedImages, Mock.Of<IImageUrlGenerator>())) { Id = 1002 },
58+
new DataType(new RichTextPropertyEditor(logger, umbracoContextAccessor, imageSourceParser, linkParser, pastedImages, Mock.Of<IImageUrlGenerator>(), Mock.Of<IHtmlSanitizer>())) { Id = 1002 },
5859
new DataType(new IntegerPropertyEditor(logger)) { Id = 1003 },
5960
new DataType(new TextboxPropertyEditor(logger)) { Id = 1004 },
6061
new DataType(new MediaPickerPropertyEditor(logger)) { Id = 1005 });

‎src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using Umbraco.Core.Models;
99
using Umbraco.Core.Models.Editors;
1010
using Umbraco.Core.PropertyEditors;
11+
using Umbraco.Core.Security;
1112
using Umbraco.Core.Services;
1213
using Umbraco.Web.Templates;
1314

@@ -32,8 +33,9 @@ public class GridPropertyEditor : DataEditor
3233
private readonly RichTextEditorPastedImages _pastedImages;
3334
private readonly HtmlLocalLinkParser _localLinkParser;
3435
private readonly IImageUrlGenerator _imageUrlGenerator;
36+
private readonly IHtmlSanitizer _htmlSanitizer;
3537

36-
[Obsolete("Use the constructor which takes an IImageUrlGenerator")]
38+
[Obsolete("Use the constructor which takes an IHtmlSanitizer")]
3739
public GridPropertyEditor(ILogger logger,
3840
IUmbracoContextAccessor umbracoContextAccessor,
3941
HtmlImageSourceParser imageSourceParser,
@@ -43,19 +45,32 @@ public GridPropertyEditor(ILogger logger,
4345
{
4446
}
4547

48+
[Obsolete("Use the constructor which takes an IHtmlSanitizer")]
4649
public GridPropertyEditor(ILogger logger,
4750
IUmbracoContextAccessor umbracoContextAccessor,
4851
HtmlImageSourceParser imageSourceParser,
4952
RichTextEditorPastedImages pastedImages,
5053
HtmlLocalLinkParser localLinkParser,
5154
IImageUrlGenerator imageUrlGenerator)
55+
: this(logger, umbracoContextAccessor, imageSourceParser, pastedImages, localLinkParser, imageUrlGenerator, Current.Factory.GetInstance<IHtmlSanitizer>())
56+
{
57+
}
58+
59+
public GridPropertyEditor(ILogger logger,
60+
IUmbracoContextAccessor umbracoContextAccessor,
61+
HtmlImageSourceParser imageSourceParser,
62+
RichTextEditorPastedImages pastedImages,
63+
HtmlLocalLinkParser localLinkParser,
64+
IImageUrlGenerator imageUrlGenerator,
65+
IHtmlSanitizer htmlSanitizer)
5266
: base(logger)
5367
{
5468
_umbracoContextAccessor = umbracoContextAccessor;
5569
_imageSourceParser = imageSourceParser;
5670
_pastedImages = pastedImages;
5771
_localLinkParser = localLinkParser;
5872
_imageUrlGenerator = imageUrlGenerator;
73+
_htmlSanitizer = htmlSanitizer;
5974
}
6075

6176
public override IPropertyIndexValueFactory PropertyIndexValueFactory => new GridPropertyIndexValueFactory();
@@ -64,7 +79,7 @@ public GridPropertyEditor(ILogger logger,
6479
/// Overridden to ensure that the value is validated
6580
/// </summary>
6681
/// <returns></returns>
67-
protected override IDataValueEditor CreateValueEditor() => new GridPropertyValueEditor(Attribute, _umbracoContextAccessor, _imageSourceParser, _pastedImages, _localLinkParser, _imageUrlGenerator);
82+
protected override IDataValueEditor CreateValueEditor() => new GridPropertyValueEditor(Attribute, _umbracoContextAccessor, _imageSourceParser, _pastedImages, _localLinkParser, _imageUrlGenerator, _htmlSanitizer);
6883

6984
protected override IConfigurationEditor CreateConfigurationEditor() => new GridConfigurationEditor();
7085

@@ -77,7 +92,7 @@ internal class GridPropertyValueEditor : DataValueEditor, IDataValueReference
7792
private readonly MediaPickerPropertyEditor.MediaPickerPropertyValueEditor _mediaPickerPropertyValueEditor;
7893
private readonly IImageUrlGenerator _imageUrlGenerator;
7994

80-
[Obsolete("Use the constructor which takes an IImageUrlGenerator")]
95+
[Obsolete("Use the constructor which takes an IHtmlSanitizer")]
8196
public GridPropertyValueEditor(DataEditorAttribute attribute,
8297
IUmbracoContextAccessor umbracoContextAccessor,
8398
HtmlImageSourceParser imageSourceParser,
@@ -87,20 +102,32 @@ public GridPropertyValueEditor(DataEditorAttribute attribute,
87102
{
88103
}
89104

105+
[Obsolete("Use the constructor which takes an IHtmlSanitizer")]
90106
public GridPropertyValueEditor(DataEditorAttribute attribute,
91107
IUmbracoContextAccessor umbracoContextAccessor,
92108
HtmlImageSourceParser imageSourceParser,
93109
RichTextEditorPastedImages pastedImages,
94110
HtmlLocalLinkParser localLinkParser,
95111
IImageUrlGenerator imageUrlGenerator)
112+
: this(attribute, umbracoContextAccessor, imageSourceParser, pastedImages, localLinkParser, imageUrlGenerator, Current.Factory.GetInstance<IHtmlSanitizer>())
113+
{
114+
}
115+
116+
public GridPropertyValueEditor(DataEditorAttribute attribute,
117+
IUmbracoContextAccessor umbracoContextAccessor,
118+
HtmlImageSourceParser imageSourceParser,
119+
RichTextEditorPastedImages pastedImages,
120+
HtmlLocalLinkParser localLinkParser,
121+
IImageUrlGenerator imageUrlGenerator,
122+
IHtmlSanitizer htmlSanitizer)
96123
: base(attribute)
97124
{
98125
_umbracoContextAccessor = umbracoContextAccessor;
99126
_imageSourceParser = imageSourceParser;
100127
_pastedImages = pastedImages;
101-
_richTextPropertyValueEditor = new RichTextPropertyEditor.RichTextPropertyValueEditor(attribute, umbracoContextAccessor, imageSourceParser, localLinkParser, pastedImages, _imageUrlGenerator);
102-
_mediaPickerPropertyValueEditor = new MediaPickerPropertyEditor.MediaPickerPropertyValueEditor(attribute);
103128
_imageUrlGenerator = imageUrlGenerator;
129+
_richTextPropertyValueEditor = new RichTextPropertyEditor.RichTextPropertyValueEditor(attribute, umbracoContextAccessor, imageSourceParser, localLinkParser, pastedImages, _imageUrlGenerator, htmlSanitizer);
130+
_mediaPickerPropertyValueEditor = new MediaPickerPropertyEditor.MediaPickerPropertyValueEditor(attribute);
104131
}
105132

106133
/// <summary>

‎src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using Umbraco.Core.Models;
88
using Umbraco.Core.Models.Editors;
99
using Umbraco.Core.PropertyEditors;
10+
using Umbraco.Core.Security;
1011
using Umbraco.Core.Services;
1112
using Umbraco.Examine;
1213
using Umbraco.Web.Macros;
@@ -32,35 +33,61 @@ public class RichTextPropertyEditor : DataEditor
3233
private readonly HtmlLocalLinkParser _localLinkParser;
3334
private readonly RichTextEditorPastedImages _pastedImages;
3435
private readonly IImageUrlGenerator _imageUrlGenerator;
36+
private readonly IHtmlSanitizer _htmlSanitizer;
3537

3638

3739
/// <summary>
3840
/// The constructor will setup the property editor based on the attribute if one is found
3941
/// </summary>
40-
[Obsolete("Use the constructor which takes an IImageUrlGenerator")]
41-
public RichTextPropertyEditor(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, HtmlLocalLinkParser localLinkParser, RichTextEditorPastedImages pastedImages)
42+
[Obsolete("Use the constructor which takes an IHtmlSanitizer")]
43+
public RichTextPropertyEditor(
44+
ILogger logger,
45+
IUmbracoContextAccessor umbracoContextAccessor,
46+
HtmlImageSourceParser imageSourceParser,
47+
HtmlLocalLinkParser localLinkParser,
48+
RichTextEditorPastedImages pastedImages)
4249
: this(logger, umbracoContextAccessor, imageSourceParser, localLinkParser, pastedImages, Current.ImageUrlGenerator)
4350
{
4451
}
4552

53+
[Obsolete("Use the constructor which takes an IHtmlSanitizer")]
54+
public RichTextPropertyEditor(
55+
ILogger logger,
56+
IUmbracoContextAccessor umbracoContextAccessor,
57+
HtmlImageSourceParser imageSourceParser,
58+
HtmlLocalLinkParser localLinkParser,
59+
RichTextEditorPastedImages pastedImages,
60+
IImageUrlGenerator imageUrlGenerator)
61+
: this(logger, umbracoContextAccessor, imageSourceParser, localLinkParser, pastedImages, imageUrlGenerator, Current.Factory.GetInstance<IHtmlSanitizer>())
62+
{
63+
}
64+
4665
/// <summary>
4766
/// The constructor will setup the property editor based on the attribute if one is found
4867
/// </summary>
49-
public RichTextPropertyEditor(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, HtmlLocalLinkParser localLinkParser, RichTextEditorPastedImages pastedImages, IImageUrlGenerator imageUrlGenerator)
68+
public RichTextPropertyEditor(
69+
ILogger logger,
70+
IUmbracoContextAccessor umbracoContextAccessor,
71+
HtmlImageSourceParser imageSourceParser,
72+
HtmlLocalLinkParser localLinkParser,
73+
RichTextEditorPastedImages pastedImages,
74+
IImageUrlGenerator imageUrlGenerator,
75+
IHtmlSanitizer htmlSanitizer)
5076
: base(logger)
5177
{
5278
_umbracoContextAccessor = umbracoContextAccessor;
5379
_imageSourceParser = imageSourceParser;
5480
_localLinkParser = localLinkParser;
5581
_pastedImages = pastedImages;
5682
_imageUrlGenerator = imageUrlGenerator;
83+
_htmlSanitizer = htmlSanitizer;
5784
}
5885

5986
/// <summary>
6087
/// Create a custom value editor
6188
/// </summary>
6289
/// <returns></returns>
63-
protected override IDataValueEditor CreateValueEditor() => new RichTextPropertyValueEditor(Attribute, _umbracoContextAccessor, _imageSourceParser, _localLinkParser, _pastedImages, _imageUrlGenerator);
90+
protected override IDataValueEditor CreateValueEditor() => new RichTextPropertyValueEditor(Attribute, _umbracoContextAccessor, _imageSourceParser, _localLinkParser, _pastedImages, _imageUrlGenerator, _htmlSanitizer);
6491

6592
protected override IConfigurationEditor CreateConfigurationEditor() => new RichTextConfigurationEditor();
6693

@@ -76,15 +103,24 @@ internal class RichTextPropertyValueEditor : DataValueEditor, IDataValueReferenc
76103
private readonly HtmlLocalLinkParser _localLinkParser;
77104
private readonly RichTextEditorPastedImages _pastedImages;
78105
private readonly IImageUrlGenerator _imageUrlGenerator;
79-
80-
public RichTextPropertyValueEditor(DataEditorAttribute attribute, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, HtmlLocalLinkParser localLinkParser, RichTextEditorPastedImages pastedImages, IImageUrlGenerator imageUrlGenerator)
106+
private readonly IHtmlSanitizer _htmlSanitizer;
107+
108+
public RichTextPropertyValueEditor(
109+
DataEditorAttribute attribute,
110+
IUmbracoContextAccessor umbracoContextAccessor,
111+
HtmlImageSourceParser imageSourceParser,
112+
HtmlLocalLinkParser localLinkParser,
113+
RichTextEditorPastedImages pastedImages,
114+
IImageUrlGenerator imageUrlGenerator,
115+
IHtmlSanitizer htmlSanitizer)
81116
: base(attribute)
82117
{
83118
_umbracoContextAccessor = umbracoContextAccessor;
84119
_imageSourceParser = imageSourceParser;
85120
_localLinkParser = localLinkParser;
86121
_pastedImages = pastedImages;
87122
_imageUrlGenerator = imageUrlGenerator;
123+
_htmlSanitizer = htmlSanitizer;
88124
}
89125

90126
/// <inheritdoc />
@@ -141,8 +177,9 @@ public override object FromEditor(Core.Models.Editors.ContentPropertyData editor
141177
var parseAndSavedTempImages = _pastedImages.FindAndPersistPastedTempImages(editorValue.Value.ToString(), mediaParentId, userId, _imageUrlGenerator);
142178
var editorValueWithMediaUrlsRemoved = _imageSourceParser.RemoveImageSources(parseAndSavedTempImages);
143179
var parsed = MacroTagParser.FormatRichTextContentForPersistence(editorValueWithMediaUrlsRemoved);
180+
var sanitized = _htmlSanitizer.Sanitize(parsed);
144181

145-
return parsed.NullOrWhiteSpaceAsNull();
182+
return sanitized.NullOrWhiteSpaceAsNull();
146183
}
147184

148185
/// <summary>

0 commit comments

Comments
 (0)