From 8b773c90c20045bc2a336f482c1c1a70e6030b0f Mon Sep 17 00:00:00 2001 From: Mole Date: Wed, 19 Jan 2022 15:43:14 +0100 Subject: [PATCH 1/7] Add IHtmlSanitizer --- src/Umbraco.Core/Security/IHtmlSanitizer.cs | 7 +++++++ src/Umbraco.Core/Security/NoOpHtmlSanitizer.cs | 10 ++++++++++ 2 files changed, 17 insertions(+) create mode 100644 src/Umbraco.Core/Security/IHtmlSanitizer.cs create mode 100644 src/Umbraco.Core/Security/NoOpHtmlSanitizer.cs diff --git a/src/Umbraco.Core/Security/IHtmlSanitizer.cs b/src/Umbraco.Core/Security/IHtmlSanitizer.cs new file mode 100644 index 0000000000..7f3f033ba7 --- /dev/null +++ b/src/Umbraco.Core/Security/IHtmlSanitizer.cs @@ -0,0 +1,7 @@ +namespace Umbraco.Core.Security +{ + public interface IHtmlSanitizer + { + string Sanitize(string html); + } +} diff --git a/src/Umbraco.Core/Security/NoOpHtmlSanitizer.cs b/src/Umbraco.Core/Security/NoOpHtmlSanitizer.cs new file mode 100644 index 0000000000..f16ce81ce1 --- /dev/null +++ b/src/Umbraco.Core/Security/NoOpHtmlSanitizer.cs @@ -0,0 +1,10 @@ +namespace Umbraco.Core.Security +{ + public class NoOpHtmlSanitizer : IHtmlSanitizer + { + public string Sanitize(string html) + { + return html; + } + } +} From e2d0a0f699c755821df3a847aed5f0e1b170d64f Mon Sep 17 00:00:00 2001 From: Mole Date: Mon, 24 Jan 2022 08:38:31 +0100 Subject: [PATCH 2/7] Add docstrings to IHtmlSanitizer --- src/Umbraco.Core/Security/IHtmlSanitizer.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Umbraco.Core/Security/IHtmlSanitizer.cs b/src/Umbraco.Core/Security/IHtmlSanitizer.cs index 7f3f033ba7..fa1e0b3ee5 100644 --- a/src/Umbraco.Core/Security/IHtmlSanitizer.cs +++ b/src/Umbraco.Core/Security/IHtmlSanitizer.cs @@ -2,6 +2,11 @@ namespace Umbraco.Core.Security { public interface IHtmlSanitizer { + /// + /// Sanitizes HTML + /// + /// HTML to be sanitized + /// Sanitized HTML string Sanitize(string html); } } From 01c1e68cf023887ef25af6bda2c7b11efb816ad3 Mon Sep 17 00:00:00 2001 From: Mole Date: Mon, 24 Jan 2022 09:19:06 +0100 Subject: [PATCH 3/7] Fix up namespaces --- src/Umbraco.Core/Security/IHtmlSanitizer.cs | 2 +- src/Umbraco.Core/Security/NoOpHtmlSanitizer.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Security/IHtmlSanitizer.cs b/src/Umbraco.Core/Security/IHtmlSanitizer.cs index fa1e0b3ee5..9bcfe405dd 100644 --- a/src/Umbraco.Core/Security/IHtmlSanitizer.cs +++ b/src/Umbraco.Core/Security/IHtmlSanitizer.cs @@ -1,4 +1,4 @@ -namespace Umbraco.Core.Security +namespace Umbraco.Cms.Core.Security { public interface IHtmlSanitizer { diff --git a/src/Umbraco.Core/Security/NoOpHtmlSanitizer.cs b/src/Umbraco.Core/Security/NoOpHtmlSanitizer.cs index f16ce81ce1..f2e8a48ad0 100644 --- a/src/Umbraco.Core/Security/NoOpHtmlSanitizer.cs +++ b/src/Umbraco.Core/Security/NoOpHtmlSanitizer.cs @@ -1,4 +1,4 @@ -namespace Umbraco.Core.Security +namespace Umbraco.Cms.Core.Security { public class NoOpHtmlSanitizer : IHtmlSanitizer { From 249774c815345293ea98c56addb1dd6604ee51f8 Mon Sep 17 00:00:00 2001 From: Mole Date: Mon, 24 Jan 2022 09:23:07 +0100 Subject: [PATCH 4/7] Rename NoOp to Noop To match the rest of the classes --- src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs | 3 +++ .../Security/{NoOpHtmlSanitizer.cs => NoopHtmlSanitizer.cs} | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) rename src/Umbraco.Core/Security/{NoOpHtmlSanitizer.cs => NoopHtmlSanitizer.cs} (73%) diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs index eacd615830..c4a95d45e5 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs @@ -263,6 +263,9 @@ namespace Umbraco.Cms.Core.DependencyInjection // Register telemetry service used to gather data about installed packages Services.AddUnique(); + + // Register a noop IHtmlSanitizer to be replaced + Services.AddUnique(); } } } diff --git a/src/Umbraco.Core/Security/NoOpHtmlSanitizer.cs b/src/Umbraco.Core/Security/NoopHtmlSanitizer.cs similarity index 73% rename from src/Umbraco.Core/Security/NoOpHtmlSanitizer.cs rename to src/Umbraco.Core/Security/NoopHtmlSanitizer.cs index f2e8a48ad0..2ada23631a 100644 --- a/src/Umbraco.Core/Security/NoOpHtmlSanitizer.cs +++ b/src/Umbraco.Core/Security/NoopHtmlSanitizer.cs @@ -1,6 +1,6 @@ namespace Umbraco.Cms.Core.Security { - public class NoOpHtmlSanitizer : IHtmlSanitizer + public class NoopHtmlSanitizer : IHtmlSanitizer { public string Sanitize(string html) { From 39f7102312f55d08a18c86132a65479250966653 Mon Sep 17 00:00:00 2001 From: Mole Date: Mon, 24 Jan 2022 09:30:23 +0100 Subject: [PATCH 5/7] Use IHtmlSanitizer in RichTextValueEditor --- .../PropertyEditors/RichTextPropertyEditor.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Infrastructure/PropertyEditors/RichTextPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/RichTextPropertyEditor.cs index 1f05da3bde..8eeb935c12 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/RichTextPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/RichTextPropertyEditor.cs @@ -81,6 +81,7 @@ namespace Umbraco.Cms.Core.PropertyEditors private readonly HtmlLocalLinkParser _localLinkParser; private readonly RichTextEditorPastedImages _pastedImages; private readonly IImageUrlGenerator _imageUrlGenerator; + private readonly IHtmlSanitizer _htmlSanitizer; public RichTextPropertyValueEditor( DataEditorAttribute attribute, @@ -92,7 +93,8 @@ namespace Umbraco.Cms.Core.PropertyEditors RichTextEditorPastedImages pastedImages, IImageUrlGenerator imageUrlGenerator, IJsonSerializer jsonSerializer, - IIOHelper ioHelper) + IIOHelper ioHelper, + IHtmlSanitizer htmlSanitizer) : base(localizedTextService, shortStringHelper, jsonSerializer, ioHelper, attribute) { _backOfficeSecurityAccessor = backOfficeSecurityAccessor; @@ -100,6 +102,7 @@ namespace Umbraco.Cms.Core.PropertyEditors _localLinkParser = localLinkParser; _pastedImages = pastedImages; _imageUrlGenerator = imageUrlGenerator; + _htmlSanitizer = htmlSanitizer; } /// @@ -156,8 +159,9 @@ namespace Umbraco.Cms.Core.PropertyEditors var parseAndSavedTempImages = _pastedImages.FindAndPersistPastedTempImages(editorValue.Value.ToString(), mediaParentId, userId, _imageUrlGenerator); var editorValueWithMediaUrlsRemoved = _imageSourceParser.RemoveImageSources(parseAndSavedTempImages); var parsed = MacroTagParser.FormatRichTextContentForPersistence(editorValueWithMediaUrlsRemoved); + var sanitized = _htmlSanitizer.Sanitize(parsed); - return parsed.NullOrWhiteSpaceAsNull(); + return sanitized.NullOrWhiteSpaceAsNull(); } /// From ca56e0971f41314c416f1f55e89ae6dba553abfd Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Tue, 25 Jan 2022 06:38:20 +0100 Subject: [PATCH 6/7] Add IsRestarting property to Umbraco application notifications (#11883) * Add IsRestarting property to Umbraco application notifications * Add IUmbracoApplicationLifetimeNotification and update constructors * Only subscribe to events on initial startup * Cleanup CoreRuntime * Do not reset StaticApplicationLogging instance after stopping/during restart --- ...IUmbracoApplicationLifetimeNotification.cs | 17 +++ .../UmbracoApplicationStartedNotification.cs | 15 ++- .../UmbracoApplicationStartingNotification.cs | 27 ++++- .../UmbracoApplicationStoppedNotification.cs | 15 ++- .../UmbracoApplicationStoppingNotification.cs | 27 ++++- .../Runtime/CoreRuntime.cs | 106 +++++++++--------- 6 files changed, 139 insertions(+), 68 deletions(-) create mode 100644 src/Umbraco.Core/Notifications/IUmbracoApplicationLifetimeNotification.cs diff --git a/src/Umbraco.Core/Notifications/IUmbracoApplicationLifetimeNotification.cs b/src/Umbraco.Core/Notifications/IUmbracoApplicationLifetimeNotification.cs new file mode 100644 index 0000000000..4b0ea6826a --- /dev/null +++ b/src/Umbraco.Core/Notifications/IUmbracoApplicationLifetimeNotification.cs @@ -0,0 +1,17 @@ +namespace Umbraco.Cms.Core.Notifications +{ + /// + /// Represents an Umbraco application lifetime (starting, started, stopping, stopped) notification. + /// + /// + public interface IUmbracoApplicationLifetimeNotification : INotification + { + /// + /// Gets a value indicating whether Umbraco is restarting (e.g. after an install or upgrade). + /// + /// + /// true if Umbraco is restarting; otherwise, false. + /// + bool IsRestarting { get; } + } +} diff --git a/src/Umbraco.Core/Notifications/UmbracoApplicationStartedNotification.cs b/src/Umbraco.Core/Notifications/UmbracoApplicationStartedNotification.cs index a3d38720d7..196af7dfe1 100644 --- a/src/Umbraco.Core/Notifications/UmbracoApplicationStartedNotification.cs +++ b/src/Umbraco.Core/Notifications/UmbracoApplicationStartedNotification.cs @@ -3,7 +3,16 @@ namespace Umbraco.Cms.Core.Notifications /// /// Notification that occurs when Umbraco has completely booted up and the request processing pipeline is configured. /// - /// - public class UmbracoApplicationStartedNotification : INotification - { } + /// + public class UmbracoApplicationStartedNotification : IUmbracoApplicationLifetimeNotification + { + /// + /// Initializes a new instance of the class. + /// + /// Indicates whether Umbraco is restarting. + public UmbracoApplicationStartedNotification(bool isRestarting) => IsRestarting = isRestarting; + + /// + public bool IsRestarting { get; } + } } diff --git a/src/Umbraco.Core/Notifications/UmbracoApplicationStartingNotification.cs b/src/Umbraco.Core/Notifications/UmbracoApplicationStartingNotification.cs index dd60f9431c..82b87aa3bf 100644 --- a/src/Umbraco.Core/Notifications/UmbracoApplicationStartingNotification.cs +++ b/src/Umbraco.Core/Notifications/UmbracoApplicationStartingNotification.cs @@ -1,16 +1,34 @@ +using System; + namespace Umbraco.Cms.Core.Notifications { /// /// Notification that occurs at the very end of the Umbraco boot process (after all s are initialized). /// - /// - public class UmbracoApplicationStartingNotification : INotification + /// + public class UmbracoApplicationStartingNotification : IUmbracoApplicationLifetimeNotification { /// /// Initializes a new instance of the class. /// /// The runtime level - public UmbracoApplicationStartingNotification(RuntimeLevel runtimeLevel) => RuntimeLevel = runtimeLevel; + [Obsolete("Use ctor with all params")] + public UmbracoApplicationStartingNotification(RuntimeLevel runtimeLevel) + : this(runtimeLevel, false) + { + // TODO: Remove this constructor in V10 + } + + /// + /// Initializes a new instance of the class. + /// + /// The runtime level + /// Indicates whether Umbraco is restarting. + public UmbracoApplicationStartingNotification(RuntimeLevel runtimeLevel, bool isRestarting) + { + RuntimeLevel = runtimeLevel; + IsRestarting = isRestarting; + } /// /// Gets the runtime level. @@ -19,5 +37,8 @@ namespace Umbraco.Cms.Core.Notifications /// The runtime level. /// public RuntimeLevel RuntimeLevel { get; } + + /// + public bool IsRestarting { get; } } } diff --git a/src/Umbraco.Core/Notifications/UmbracoApplicationStoppedNotification.cs b/src/Umbraco.Core/Notifications/UmbracoApplicationStoppedNotification.cs index be4c6ccfd4..c6dac40a26 100644 --- a/src/Umbraco.Core/Notifications/UmbracoApplicationStoppedNotification.cs +++ b/src/Umbraco.Core/Notifications/UmbracoApplicationStoppedNotification.cs @@ -3,7 +3,16 @@ namespace Umbraco.Cms.Core.Notifications /// /// Notification that occurs when Umbraco has completely shutdown. /// - /// - public class UmbracoApplicationStoppedNotification : INotification - { } + /// + public class UmbracoApplicationStoppedNotification : IUmbracoApplicationLifetimeNotification + { + /// + /// Initializes a new instance of the class. + /// + /// Indicates whether Umbraco is restarting. + public UmbracoApplicationStoppedNotification(bool isRestarting) => IsRestarting = isRestarting; + + /// + public bool IsRestarting { get; } + } } diff --git a/src/Umbraco.Core/Notifications/UmbracoApplicationStoppingNotification.cs b/src/Umbraco.Core/Notifications/UmbracoApplicationStoppingNotification.cs index 6d5234bbcc..062ca954d9 100644 --- a/src/Umbraco.Core/Notifications/UmbracoApplicationStoppingNotification.cs +++ b/src/Umbraco.Core/Notifications/UmbracoApplicationStoppingNotification.cs @@ -1,9 +1,30 @@ +using System; + namespace Umbraco.Cms.Core.Notifications { /// /// Notification that occurs when Umbraco is shutting down (after all s are terminated). /// - /// - public class UmbracoApplicationStoppingNotification : INotification - { } + /// + public class UmbracoApplicationStoppingNotification : IUmbracoApplicationLifetimeNotification + { + /// + /// Initializes a new instance of the class. + /// + [Obsolete("Use ctor with all params")] + public UmbracoApplicationStoppingNotification() + : this(false) + { + // TODO: Remove this constructor in V10 + } + + /// + /// Initializes a new instance of the class. + /// + /// Indicates whether Umbraco is restarting. + public UmbracoApplicationStoppingNotification(bool isRestarting) => IsRestarting = isRestarting; + + /// + public bool IsRestarting { get; } + } } diff --git a/src/Umbraco.Infrastructure/Runtime/CoreRuntime.cs b/src/Umbraco.Infrastructure/Runtime/CoreRuntime.cs index 5dbe78c2f5..851d67e713 100644 --- a/src/Umbraco.Infrastructure/Runtime/CoreRuntime.cs +++ b/src/Umbraco.Infrastructure/Runtime/CoreRuntime.cs @@ -134,60 +134,48 @@ namespace Umbraco.Cms.Infrastructure.Runtime public IRuntimeState State { get; } /// - public async Task RestartAsync() - { - await StopAsync(_cancellationToken); - await _eventAggregator.PublishAsync(new UmbracoApplicationStoppedNotification(), _cancellationToken); - await StartAsync(_cancellationToken); - await _eventAggregator.PublishAsync(new UmbracoApplicationStartedNotification(), _cancellationToken); - } + public async Task StartAsync(CancellationToken cancellationToken) => await StartAsync(cancellationToken, false); /// - public async Task StartAsync(CancellationToken cancellationToken) + public async Task StopAsync(CancellationToken cancellationToken) => await StopAsync(cancellationToken, false); + + /// + public async Task RestartAsync() + { + await StopAsync(_cancellationToken, true); + await _eventAggregator.PublishAsync(new UmbracoApplicationStoppedNotification(true), _cancellationToken); + await StartAsync(_cancellationToken, true); + await _eventAggregator.PublishAsync(new UmbracoApplicationStartedNotification(true), _cancellationToken); + } + + private async Task StartAsync(CancellationToken cancellationToken, bool isRestarting) { // Store token, so we can re-use this during restart _cancellationToken = cancellationToken; - StaticApplicationLogging.Initialize(_loggerFactory); - StaticServiceProvider.Instance = _serviceProvider; - - AppDomain.CurrentDomain.UnhandledException += (_, args) => + if (isRestarting == false) { - var exception = (Exception)args.ExceptionObject; - var isTerminating = args.IsTerminating; // always true? + StaticApplicationLogging.Initialize(_loggerFactory); + StaticServiceProvider.Instance = _serviceProvider; - var msg = "Unhandled exception in AppDomain"; - - if (isTerminating) - { - msg += " (terminating)"; - } - - msg += "."; - - _logger.LogError(exception, msg); - }; - - // Add application started and stopped notifications (only on initial startup, not restarts) - if (_hostApplicationLifetime.ApplicationStarted.IsCancellationRequested == false) - { - _hostApplicationLifetime.ApplicationStarted.Register(() => _eventAggregator.Publish(new UmbracoApplicationStartedNotification())); - _hostApplicationLifetime.ApplicationStopped.Register(() => _eventAggregator.Publish(new UmbracoApplicationStoppedNotification())); + AppDomain.CurrentDomain.UnhandledException += (_, args) + => _logger.LogError(args.ExceptionObject as Exception, $"Unhandled exception in AppDomain{(args.IsTerminating ? " (terminating)" : null)}."); } - // acquire the main domain - if this fails then anything that should be registered with MainDom will not operate + // Acquire the main domain - if this fails then anything that should be registered with MainDom will not operate AcquireMainDom(); // TODO (V10): Remove this obsoleted notification publish. await _eventAggregator.PublishAsync(new UmbracoApplicationMainDomAcquiredNotification(), cancellationToken); - // notify for unattended install + // Notify for unattended install await _eventAggregator.PublishAsync(new RuntimeUnattendedInstallNotification(), cancellationToken); DetermineRuntimeLevel(); if (!State.UmbracoCanBoot()) { - return; // The exception will be rethrown by BootFailedMiddelware + // We cannot continue here, the exception will be rethrown by BootFailedMiddelware + return; } IApplicationShutdownRegistry hostingEnvironmentLifetime = _applicationShutdownRegistry; @@ -196,7 +184,7 @@ namespace Umbraco.Cms.Infrastructure.Runtime throw new InvalidOperationException($"An instance of {typeof(IApplicationShutdownRegistry)} could not be resolved from the container, ensure that one if registered in your runtime before calling {nameof(IRuntime)}.{nameof(StartAsync)}"); } - // if level is Run and reason is UpgradeMigrations, that means we need to perform an unattended upgrade + // If level is Run and reason is UpgradeMigrations, that means we need to perform an unattended upgrade var unattendedUpgradeNotification = new RuntimeUnattendedUpgradeNotification(); await _eventAggregator.PublishAsync(unattendedUpgradeNotification, cancellationToken); switch (unattendedUpgradeNotification.UnattendedUpgradeResult) @@ -207,54 +195,59 @@ namespace Umbraco.Cms.Infrastructure.Runtime throw new InvalidOperationException($"Unattended upgrade result was {RuntimeUnattendedUpgradeNotification.UpgradeResult.HasErrors} but no {nameof(BootFailedException)} was registered"); } - // we cannot continue here, the exception will be rethrown by BootFailedMiddelware + // We cannot continue here, the exception will be rethrown by BootFailedMiddelware return; case RuntimeUnattendedUpgradeNotification.UpgradeResult.CoreUpgradeComplete: case RuntimeUnattendedUpgradeNotification.UpgradeResult.PackageMigrationComplete: - // upgrade is done, set reason to Run + // Upgrade is done, set reason to Run DetermineRuntimeLevel(); break; case RuntimeUnattendedUpgradeNotification.UpgradeResult.NotRequired: break; } - // TODO (V10): Remove this obsoleted notification publish. + // TODO (V10): Remove this obsoleted notification publish await _eventAggregator.PublishAsync(new UmbracoApplicationComponentsInstallingNotification(State.Level), cancellationToken); - // create & initialize the components + // Initialize the components _components.Initialize(); - await _eventAggregator.PublishAsync(new UmbracoApplicationStartingNotification(State.Level), cancellationToken); + await _eventAggregator.PublishAsync(new UmbracoApplicationStartingNotification(State.Level, isRestarting), cancellationToken); + + if (isRestarting == false) + { + // Add application started and stopped notifications last (to ensure they're always published after starting) + _hostApplicationLifetime.ApplicationStarted.Register(() => _eventAggregator.Publish(new UmbracoApplicationStartedNotification(false))); + _hostApplicationLifetime.ApplicationStopped.Register(() => _eventAggregator.Publish(new UmbracoApplicationStoppedNotification(false))); + } } - public async Task StopAsync(CancellationToken cancellationToken) + private async Task StopAsync(CancellationToken cancellationToken, bool isRestarting) { _components.Terminate(); - await _eventAggregator.PublishAsync(new UmbracoApplicationStoppingNotification(), cancellationToken); - StaticApplicationLogging.Initialize(null); + await _eventAggregator.PublishAsync(new UmbracoApplicationStoppingNotification(isRestarting), cancellationToken); } private void AcquireMainDom() { - using (DisposableTimer timer = _profilingLogger.DebugDuration("Acquiring MainDom.", "Acquired.")) + using DisposableTimer timer = _profilingLogger.DebugDuration("Acquiring MainDom.", "Acquired."); + + try { - try - { - _mainDom.Acquire(_applicationShutdownRegistry); - } - catch - { - timer?.Fail(); - throw; - } + _mainDom.Acquire(_applicationShutdownRegistry); + } + catch + { + timer?.Fail(); + throw; } } private void DetermineRuntimeLevel() { - if (State.BootFailedException != null) + if (State.BootFailedException is not null) { - // there's already been an exception so cannot boot and no need to check + // There's already been an exception, so cannot boot and no need to check return; } @@ -277,7 +270,8 @@ namespace Umbraco.Cms.Infrastructure.Runtime State.Configure(RuntimeLevel.BootFailed, RuntimeLevelReason.BootFailedOnException); timer?.Fail(); _logger.LogError(ex, "Boot Failed"); - // We do not throw the exception. It will be rethrown by BootFailedMiddleware + + // We do not throw the exception, it will be rethrown by BootFailedMiddleware } } } From d70a207d60b6f4eff8cf1b446f0a0cf4f70d03b5 Mon Sep 17 00:00:00 2001 From: Mole Date: Wed, 26 Jan 2022 07:51:25 +0100 Subject: [PATCH 7/7] 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 --- .../CompositionExtensions/Services.cs | 2 + src/Umbraco.Core/Security/IHtmlSanitizer.cs | 12 +++++ .../Security/NoopHtmlSanitizer.cs | 10 ++++ src/Umbraco.Core/Umbraco.Core.csproj | 2 + .../PublishedContent/PublishedContentTests.cs | 3 +- .../PropertyEditors/GridPropertyEditor.cs | 37 ++++++++++++-- .../PropertyEditors/RichTextPropertyEditor.cs | 49 ++++++++++++++++--- 7 files changed, 103 insertions(+), 12 deletions(-) create mode 100644 src/Umbraco.Core/Security/IHtmlSanitizer.cs create mode 100644 src/Umbraco.Core/Security/NoopHtmlSanitizer.cs diff --git a/src/Umbraco.Core/Composing/CompositionExtensions/Services.cs b/src/Umbraco.Core/Composing/CompositionExtensions/Services.cs index e912f7281c..3c9dd9d701 100644 --- a/src/Umbraco.Core/Composing/CompositionExtensions/Services.cs +++ b/src/Umbraco.Core/Composing/CompositionExtensions/Services.cs @@ -6,6 +6,7 @@ using Umbraco.Core.Events; using Umbraco.Core.IO; using Umbraco.Core.Logging; using Umbraco.Core.Packaging; +using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Core.Services.Implement; using Umbraco.Core.Telemetry; @@ -81,6 +82,7 @@ namespace Umbraco.Core.Composing.CompositionExtensions new DirectoryInfo(IOHelper.GetRootDirectorySafe()))); composition.RegisterUnique(); + composition.RegisterUnique(); return composition; } diff --git a/src/Umbraco.Core/Security/IHtmlSanitizer.cs b/src/Umbraco.Core/Security/IHtmlSanitizer.cs new file mode 100644 index 0000000000..fa1e0b3ee5 --- /dev/null +++ b/src/Umbraco.Core/Security/IHtmlSanitizer.cs @@ -0,0 +1,12 @@ +namespace Umbraco.Core.Security +{ + public interface IHtmlSanitizer + { + /// + /// Sanitizes HTML + /// + /// HTML to be sanitized + /// Sanitized HTML + string Sanitize(string html); + } +} diff --git a/src/Umbraco.Core/Security/NoopHtmlSanitizer.cs b/src/Umbraco.Core/Security/NoopHtmlSanitizer.cs new file mode 100644 index 0000000000..2ea34d52ea --- /dev/null +++ b/src/Umbraco.Core/Security/NoopHtmlSanitizer.cs @@ -0,0 +1,10 @@ +namespace Umbraco.Core.Security +{ + public class NoopHtmlSanitizer : IHtmlSanitizer + { + public string Sanitize(string html) + { + return html; + } + } +} diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 6729930174..e27c6eeceb 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -194,6 +194,8 @@ + + diff --git a/src/Umbraco.Tests/PublishedContent/PublishedContentTests.cs b/src/Umbraco.Tests/PublishedContent/PublishedContentTests.cs index cafda161f4..9b8f5a05fd 100644 --- a/src/Umbraco.Tests/PublishedContent/PublishedContentTests.cs +++ b/src/Umbraco.Tests/PublishedContent/PublishedContentTests.cs @@ -16,6 +16,7 @@ using Umbraco.Core.Cache; using Umbraco.Core.Configuration; using Umbraco.Core.Logging; using Umbraco.Core.Models; +using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Tests.TestHelpers; using Umbraco.Tests.Testing; @@ -54,7 +55,7 @@ namespace Umbraco.Tests.PublishedContent var dataTypeService = new TestObjects.TestDataTypeService( new DataType(new VoidEditor(logger)) { Id = 1 }, new DataType(new TrueFalsePropertyEditor(logger)) { Id = 1001 }, - new DataType(new RichTextPropertyEditor(logger, umbracoContextAccessor, imageSourceParser, linkParser, pastedImages, Mock.Of())) { Id = 1002 }, + new DataType(new RichTextPropertyEditor(logger, umbracoContextAccessor, imageSourceParser, linkParser, pastedImages, Mock.Of(), Mock.Of())) { Id = 1002 }, new DataType(new IntegerPropertyEditor(logger)) { Id = 1003 }, new DataType(new TextboxPropertyEditor(logger)) { Id = 1004 }, new DataType(new MediaPickerPropertyEditor(logger)) { Id = 1005 }); diff --git a/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs index 6f919868f7..5ed3051e07 100644 --- a/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs @@ -8,6 +8,7 @@ using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.Editors; using Umbraco.Core.PropertyEditors; +using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Web.Templates; @@ -32,8 +33,9 @@ namespace Umbraco.Web.PropertyEditors private readonly RichTextEditorPastedImages _pastedImages; private readonly HtmlLocalLinkParser _localLinkParser; private readonly IImageUrlGenerator _imageUrlGenerator; + private readonly IHtmlSanitizer _htmlSanitizer; - [Obsolete("Use the constructor which takes an IImageUrlGenerator")] + [Obsolete("Use the constructor which takes an IHtmlSanitizer")] public GridPropertyEditor(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, @@ -43,12 +45,24 @@ namespace Umbraco.Web.PropertyEditors { } + [Obsolete("Use the constructor which takes an IHtmlSanitizer")] public GridPropertyEditor(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, RichTextEditorPastedImages pastedImages, HtmlLocalLinkParser localLinkParser, IImageUrlGenerator imageUrlGenerator) + : this(logger, umbracoContextAccessor, imageSourceParser, pastedImages, localLinkParser, imageUrlGenerator, Current.Factory.GetInstance()) + { + } + + public GridPropertyEditor(ILogger logger, + IUmbracoContextAccessor umbracoContextAccessor, + HtmlImageSourceParser imageSourceParser, + RichTextEditorPastedImages pastedImages, + HtmlLocalLinkParser localLinkParser, + IImageUrlGenerator imageUrlGenerator, + IHtmlSanitizer htmlSanitizer) : base(logger) { _umbracoContextAccessor = umbracoContextAccessor; @@ -56,6 +70,7 @@ namespace Umbraco.Web.PropertyEditors _pastedImages = pastedImages; _localLinkParser = localLinkParser; _imageUrlGenerator = imageUrlGenerator; + _htmlSanitizer = htmlSanitizer; } public override IPropertyIndexValueFactory PropertyIndexValueFactory => new GridPropertyIndexValueFactory(); @@ -64,7 +79,7 @@ namespace Umbraco.Web.PropertyEditors /// Overridden to ensure that the value is validated /// /// - protected override IDataValueEditor CreateValueEditor() => new GridPropertyValueEditor(Attribute, _umbracoContextAccessor, _imageSourceParser, _pastedImages, _localLinkParser, _imageUrlGenerator); + protected override IDataValueEditor CreateValueEditor() => new GridPropertyValueEditor(Attribute, _umbracoContextAccessor, _imageSourceParser, _pastedImages, _localLinkParser, _imageUrlGenerator, _htmlSanitizer); protected override IConfigurationEditor CreateConfigurationEditor() => new GridConfigurationEditor(); @@ -77,7 +92,7 @@ namespace Umbraco.Web.PropertyEditors private readonly MediaPickerPropertyEditor.MediaPickerPropertyValueEditor _mediaPickerPropertyValueEditor; private readonly IImageUrlGenerator _imageUrlGenerator; - [Obsolete("Use the constructor which takes an IImageUrlGenerator")] + [Obsolete("Use the constructor which takes an IHtmlSanitizer")] public GridPropertyValueEditor(DataEditorAttribute attribute, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, @@ -87,20 +102,32 @@ namespace Umbraco.Web.PropertyEditors { } + [Obsolete("Use the constructor which takes an IHtmlSanitizer")] public GridPropertyValueEditor(DataEditorAttribute attribute, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, RichTextEditorPastedImages pastedImages, HtmlLocalLinkParser localLinkParser, IImageUrlGenerator imageUrlGenerator) + : this(attribute, umbracoContextAccessor, imageSourceParser, pastedImages, localLinkParser, imageUrlGenerator, Current.Factory.GetInstance()) + { + } + + public GridPropertyValueEditor(DataEditorAttribute attribute, + IUmbracoContextAccessor umbracoContextAccessor, + HtmlImageSourceParser imageSourceParser, + RichTextEditorPastedImages pastedImages, + HtmlLocalLinkParser localLinkParser, + IImageUrlGenerator imageUrlGenerator, + IHtmlSanitizer htmlSanitizer) : base(attribute) { _umbracoContextAccessor = umbracoContextAccessor; _imageSourceParser = imageSourceParser; _pastedImages = pastedImages; - _richTextPropertyValueEditor = new RichTextPropertyEditor.RichTextPropertyValueEditor(attribute, umbracoContextAccessor, imageSourceParser, localLinkParser, pastedImages, _imageUrlGenerator); - _mediaPickerPropertyValueEditor = new MediaPickerPropertyEditor.MediaPickerPropertyValueEditor(attribute); _imageUrlGenerator = imageUrlGenerator; + _richTextPropertyValueEditor = new RichTextPropertyEditor.RichTextPropertyValueEditor(attribute, umbracoContextAccessor, imageSourceParser, localLinkParser, pastedImages, _imageUrlGenerator, htmlSanitizer); + _mediaPickerPropertyValueEditor = new MediaPickerPropertyEditor.MediaPickerPropertyValueEditor(attribute); } /// diff --git a/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs index 2d698835b0..4a6c0c09b6 100644 --- a/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/RichTextPropertyEditor.cs @@ -7,6 +7,7 @@ using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.Editors; using Umbraco.Core.PropertyEditors; +using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Examine; using Umbraco.Web.Macros; @@ -32,21 +33,46 @@ namespace Umbraco.Web.PropertyEditors private readonly HtmlLocalLinkParser _localLinkParser; private readonly RichTextEditorPastedImages _pastedImages; private readonly IImageUrlGenerator _imageUrlGenerator; + private readonly IHtmlSanitizer _htmlSanitizer; /// /// The constructor will setup the property editor based on the attribute if one is found /// - [Obsolete("Use the constructor which takes an IImageUrlGenerator")] - public RichTextPropertyEditor(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, HtmlLocalLinkParser localLinkParser, RichTextEditorPastedImages pastedImages) + [Obsolete("Use the constructor which takes an IHtmlSanitizer")] + public RichTextPropertyEditor( + ILogger logger, + IUmbracoContextAccessor umbracoContextAccessor, + HtmlImageSourceParser imageSourceParser, + HtmlLocalLinkParser localLinkParser, + RichTextEditorPastedImages pastedImages) : this(logger, umbracoContextAccessor, imageSourceParser, localLinkParser, pastedImages, Current.ImageUrlGenerator) { } + [Obsolete("Use the constructor which takes an IHtmlSanitizer")] + public RichTextPropertyEditor( + ILogger logger, + IUmbracoContextAccessor umbracoContextAccessor, + HtmlImageSourceParser imageSourceParser, + HtmlLocalLinkParser localLinkParser, + RichTextEditorPastedImages pastedImages, + IImageUrlGenerator imageUrlGenerator) + : this(logger, umbracoContextAccessor, imageSourceParser, localLinkParser, pastedImages, imageUrlGenerator, Current.Factory.GetInstance()) + { + } + /// /// The constructor will setup the property editor based on the attribute if one is found /// - public RichTextPropertyEditor(ILogger logger, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, HtmlLocalLinkParser localLinkParser, RichTextEditorPastedImages pastedImages, IImageUrlGenerator imageUrlGenerator) + public RichTextPropertyEditor( + ILogger logger, + IUmbracoContextAccessor umbracoContextAccessor, + HtmlImageSourceParser imageSourceParser, + HtmlLocalLinkParser localLinkParser, + RichTextEditorPastedImages pastedImages, + IImageUrlGenerator imageUrlGenerator, + IHtmlSanitizer htmlSanitizer) : base(logger) { _umbracoContextAccessor = umbracoContextAccessor; @@ -54,13 +80,14 @@ namespace Umbraco.Web.PropertyEditors _localLinkParser = localLinkParser; _pastedImages = pastedImages; _imageUrlGenerator = imageUrlGenerator; + _htmlSanitizer = htmlSanitizer; } /// /// Create a custom value editor /// /// - protected override IDataValueEditor CreateValueEditor() => new RichTextPropertyValueEditor(Attribute, _umbracoContextAccessor, _imageSourceParser, _localLinkParser, _pastedImages, _imageUrlGenerator); + protected override IDataValueEditor CreateValueEditor() => new RichTextPropertyValueEditor(Attribute, _umbracoContextAccessor, _imageSourceParser, _localLinkParser, _pastedImages, _imageUrlGenerator, _htmlSanitizer); protected override IConfigurationEditor CreateConfigurationEditor() => new RichTextConfigurationEditor(); @@ -76,8 +103,16 @@ namespace Umbraco.Web.PropertyEditors private readonly HtmlLocalLinkParser _localLinkParser; private readonly RichTextEditorPastedImages _pastedImages; private readonly IImageUrlGenerator _imageUrlGenerator; + private readonly IHtmlSanitizer _htmlSanitizer; - public RichTextPropertyValueEditor(DataEditorAttribute attribute, IUmbracoContextAccessor umbracoContextAccessor, HtmlImageSourceParser imageSourceParser, HtmlLocalLinkParser localLinkParser, RichTextEditorPastedImages pastedImages, IImageUrlGenerator imageUrlGenerator) + public RichTextPropertyValueEditor( + DataEditorAttribute attribute, + IUmbracoContextAccessor umbracoContextAccessor, + HtmlImageSourceParser imageSourceParser, + HtmlLocalLinkParser localLinkParser, + RichTextEditorPastedImages pastedImages, + IImageUrlGenerator imageUrlGenerator, + IHtmlSanitizer htmlSanitizer) : base(attribute) { _umbracoContextAccessor = umbracoContextAccessor; @@ -85,6 +120,7 @@ namespace Umbraco.Web.PropertyEditors _localLinkParser = localLinkParser; _pastedImages = pastedImages; _imageUrlGenerator = imageUrlGenerator; + _htmlSanitizer = htmlSanitizer; } /// @@ -141,8 +177,9 @@ namespace Umbraco.Web.PropertyEditors var parseAndSavedTempImages = _pastedImages.FindAndPersistPastedTempImages(editorValue.Value.ToString(), mediaParentId, userId, _imageUrlGenerator); var editorValueWithMediaUrlsRemoved = _imageSourceParser.RemoveImageSources(parseAndSavedTempImages); var parsed = MacroTagParser.FormatRichTextContentForPersistence(editorValueWithMediaUrlsRemoved); + var sanitized = _htmlSanitizer.Sanitize(parsed); - return parsed.NullOrWhiteSpaceAsNull(); + return sanitized.NullOrWhiteSpaceAsNull(); } ///