From 27ae8bdba92199af04e372250807386547dd46a5 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Thu, 11 May 2023 11:01:03 +0200 Subject: [PATCH] v12: Add HMAC image processing protection (#14181) * Update to ImageSharp 2.1.0 and ImageSharp.Web 2.0.0-alpha.0.23 * Rename CachedNameLength to CacheHashLength and add CacheFolderDepth setting * Replace PhysicalFileSystemProvider with WebRootImageProvider * Support EXIF-orientation in image dimention extractor * Remove virtual methods on FileProviderImageProvider * Simplify FileInfoImageResolver * Update to SixLabors.ImageSharp.Web 2.0.0-alpha.0.25 and remove custom providers * Make CropWebProcessor EXIF orientation-aware * Improve width/height sanitization * Also use 'v' as cache buster value * Add WebP to supported image file types * Update to SixLabors.ImageSharp.Web 2.0.0-alpha.0.27 and fix test * Fix rounding error and add test cases * Update to newest and stable releases * Move ImageSharpImageUrlGenerator to Umbraco.Web.Common * Use IConfigureOptions to configure ImageSharp options * Implement IEquatable on ImageUrlGenerationOptions classes * Fix empty/null values in image URL generation and corresponding tests * Use IsSupportedImageFormat extension method * Remove unneeded reflection * Add HMACSecretKey setting and add token when generating image URLs * Ensure backoffice image URLs are generated by the server (and include a correct HMAC token) * Abstract HMAC generation to IImageUrlTokenGenerator * Change cache buster value to 'v' and use hexadecimal timestamp * Update comments * Fix backoffice thumbnail URL generation * Update grid media thumbnail URL generation * Remove breaking changes * Strip unknown commands from image URL token * Remove HMAC whitelisting possibility (not supported by ImageSharp) * Update to SixLabors.ImageSharp 2.1.3 * Add comment to internal constructor * Fix to support absolute image URLs * Update to SixLabors.ImageSharp.Web 2.0.3-alpha.0.3 * Remove IImageUrlTokenGenerator and use ImageSharpRequestAuthorizationUtilities * Move NuGet feed to config file * Update to ImageSharp v3 --- .../ConfigureImageSharpMiddlewareOptions.cs | 27 +- .../Media/ImageSharpImageUrlGenerator.cs | 82 +++-- .../Models/ImagingResizeSettings.cs | 6 +- .../Configuration/Models/ImagingSettings.cs | 15 +- .../Controllers/ImagesController.cs | 56 ++-- .../ImageCropperTemplateCoreExtensions.cs | 2 +- .../common/services/mediahelper.service.js | 2 +- .../blockcard/umbBlockCard.component.js | 6 +- .../fileupload/fileupload.controller.js | 12 +- .../grid/editors/media.controller.js | 52 ++-- .../imagecropper/imagecropper.controller.js | 5 +- .../Media/ImageSharpImageUrlGeneratorTests.cs | 280 ++++++++++++------ 12 files changed, 329 insertions(+), 216 deletions(-) diff --git a/src/Umbraco.Cms.Imaging.ImageSharp/ConfigureImageSharpMiddlewareOptions.cs b/src/Umbraco.Cms.Imaging.ImageSharp/ConfigureImageSharpMiddlewareOptions.cs index aaaade3544..9a1ecead89 100644 --- a/src/Umbraco.Cms.Imaging.ImageSharp/ConfigureImageSharpMiddlewareOptions.cs +++ b/src/Umbraco.Cms.Imaging.ImageSharp/ConfigureImageSharpMiddlewareOptions.cs @@ -10,7 +10,7 @@ using Umbraco.Cms.Core.Configuration.Models; namespace Umbraco.Cms.Imaging.ImageSharp; /// -/// Configures the ImageSharp middleware options. +/// Configures the ImageSharp middleware options. /// /// public sealed class ConfigureImageSharpMiddlewareOptions : IConfigureOptions @@ -19,7 +19,7 @@ public sealed class ConfigureImageSharpMiddlewareOptions : IConfigureOptions - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// /// The ImageSharp configuration. /// The Umbraco imaging settings. @@ -34,6 +34,7 @@ public sealed class ConfigureImageSharpMiddlewareOptions : IConfigureOptions { - if (context.Commands.Count == 0) + if (context.Commands.Count == 0 || _imagingSettings.HMACSecretKey.Length > 0) { + // Nothing to parse or using HMAC authentication return Task.CompletedTask; } - var width = context.Parser.ParseValue( - context.Commands.GetValueOrDefault(ResizeWebProcessor.Width), - context.Culture); + int width = context.Parser.ParseValue(context.Commands.GetValueOrDefault(ResizeWebProcessor.Width), context.Culture); if (width <= 0 || width > _imagingSettings.Resize.MaxWidth) { context.Commands.Remove(ResizeWebProcessor.Width); } - var height = context.Parser.ParseValue( - context.Commands.GetValueOrDefault(ResizeWebProcessor.Height), - context.Culture); + int height = context.Parser.ParseValue(context.Commands.GetValueOrDefault(ResizeWebProcessor.Height), context.Culture); if (height <= 0 || height > _imagingSettings.Resize.MaxHeight) { context.Commands.Remove(ResizeWebProcessor.Height); @@ -72,11 +70,16 @@ public sealed class ConfigureImageSharpMiddlewareOptions : IConfigureOptions -/// Exposes a method that generates an image URL based on the specified options that can be processed by ImageSharp. +/// Exposes a method that generates an image URL based on the specified options that can be processed by ImageSharp. /// /// public sealed class ImageSharpImageUrlGenerator : IImageUrlGenerator { - /// - public IEnumerable SupportedImageFileTypes { get; } + private readonly RequestAuthorizationUtilities? _requestAuthorizationUtilities; /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// /// The ImageSharp configuration. - public ImageSharpImageUrlGenerator(Configuration configuration) - : this(configuration.ImageFormats.SelectMany(f => f.FileExtensions).ToArray()) + /// Contains helpers that allow authorization of image requests. + public ImageSharpImageUrlGenerator(Configuration configuration, RequestAuthorizationUtilities? requestAuthorizationUtilities) + : this(configuration.ImageFormats.SelectMany(f => f.FileExtensions).ToArray(), requestAuthorizationUtilities) { } /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. + /// + /// The ImageSharp configuration. + [Obsolete("Use ctor with all params - This will be removed in Umbraco 13.")] + public ImageSharpImageUrlGenerator(Configuration configuration) + : this(configuration, StaticServiceProvider.Instance.GetService()) + { } + + /// + /// Initializes a new instance of the class. /// /// The supported image file types/extensions. + /// Contains helpers that allow authorization of image requests. /// - /// This constructor is only used for testing. + /// This constructor is only used for testing. /// - internal ImageSharpImageUrlGenerator(IEnumerable supportedImageFileTypes) => + internal ImageSharpImageUrlGenerator(IEnumerable supportedImageFileTypes, RequestAuthorizationUtilities? requestAuthorizationUtilities = null) + { SupportedImageFileTypes = supportedImageFileTypes; + _requestAuthorizationUtilities = requestAuthorizationUtilities; + } + + /// + public IEnumerable SupportedImageFileTypes { get; } /// public string? GetImageUrl(ImageUrlGenerationOptions? options) @@ -47,47 +65,44 @@ public sealed class ImageSharpImageUrlGenerator : IImageUrlGenerator var queryString = new Dictionary(); Dictionary furtherOptions = QueryHelpers.ParseQuery(options.FurtherOptions); - if (options.Crop is not null) + if (options.Crop is CropCoordinates crop) { - CropCoordinates? crop = options.Crop; - queryString.Add( - CropWebProcessor.Coordinates, - FormattableString.Invariant($"{crop.Left},{crop.Top},{crop.Right},{crop.Bottom}")); + queryString.Add(CropWebProcessor.Coordinates, FormattableString.Invariant($"{crop.Left},{crop.Top},{crop.Right},{crop.Bottom}")); } - if (options.FocalPoint is not null) + if (options.FocalPoint is FocalPointPosition focalPoint) { - queryString.Add(ResizeWebProcessor.Xy, FormattableString.Invariant($"{options.FocalPoint.Left},{options.FocalPoint.Top}")); + queryString.Add(ResizeWebProcessor.Xy, FormattableString.Invariant($"{focalPoint.Left},{focalPoint.Top}")); } - if (options.ImageCropMode is not null) + if (options.ImageCropMode is ImageCropMode imageCropMode) { - queryString.Add(ResizeWebProcessor.Mode, options.ImageCropMode.ToString()?.ToLowerInvariant()); + queryString.Add(ResizeWebProcessor.Mode, imageCropMode.ToString().ToLowerInvariant()); } - if (options.ImageCropAnchor is not null) + if (options.ImageCropAnchor is ImageCropAnchor imageCropAnchor) { - queryString.Add(ResizeWebProcessor.Anchor, options.ImageCropAnchor.ToString()?.ToLowerInvariant()); + queryString.Add(ResizeWebProcessor.Anchor, imageCropAnchor.ToString().ToLowerInvariant()); } - if (options.Width is not null) + if (options.Width is int width) { - queryString.Add(ResizeWebProcessor.Width, options.Width?.ToString(CultureInfo.InvariantCulture)); + queryString.Add(ResizeWebProcessor.Width, width.ToString(CultureInfo.InvariantCulture)); } - if (options.Height is not null) + if (options.Height is int height) { - queryString.Add(ResizeWebProcessor.Height, options.Height?.ToString(CultureInfo.InvariantCulture)); + queryString.Add(ResizeWebProcessor.Height, height.ToString(CultureInfo.InvariantCulture)); } if (furtherOptions.Remove(FormatWebProcessor.Format, out StringValues format)) { - queryString.Add(FormatWebProcessor.Format, format[0]); + queryString.Add(FormatWebProcessor.Format, format.ToString()); } - if (options.Quality is not null) + if (options.Quality is int quality) { - queryString.Add(QualityWebProcessor.Quality, options.Quality?.ToString(CultureInfo.InvariantCulture)); + queryString.Add(QualityWebProcessor.Quality, quality.ToString(CultureInfo.InvariantCulture)); } foreach (KeyValuePair kvp in furtherOptions) @@ -95,9 +110,18 @@ public sealed class ImageSharpImageUrlGenerator : IImageUrlGenerator queryString.Add(kvp.Key, kvp.Value); } - if (options.CacheBusterValue is not null && !string.IsNullOrWhiteSpace(options.CacheBusterValue)) + if (options.CacheBusterValue is string cacheBusterValue && !string.IsNullOrEmpty(cacheBusterValue)) { - queryString.Add("rnd", options.CacheBusterValue); + queryString.Add("v", cacheBusterValue); + } + + if (_requestAuthorizationUtilities is not null) + { + var uri = QueryHelpers.AddQueryString(options.ImageUrl, queryString); + if (_requestAuthorizationUtilities.ComputeHMAC(uri, CommandHandling.Sanitize) is string token && !string.IsNullOrEmpty(token)) + { + queryString.Add(RequestAuthorizationUtilities.TokenCommand, token); + } } return QueryHelpers.AddQueryString(options.ImageUrl, queryString); diff --git a/src/Umbraco.Core/Configuration/Models/ImagingResizeSettings.cs b/src/Umbraco.Core/Configuration/Models/ImagingResizeSettings.cs index dc4585bf9c..2ae7d855be 100644 --- a/src/Umbraco.Core/Configuration/Models/ImagingResizeSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/ImagingResizeSettings.cs @@ -6,7 +6,7 @@ using System.ComponentModel; namespace Umbraco.Cms.Core.Configuration.Models; /// -/// Typed configuration options for image resize settings. +/// Typed configuration options for image resize settings. /// public class ImagingResizeSettings { @@ -14,13 +14,13 @@ public class ImagingResizeSettings internal const int StaticMaxHeight = 5000; /// - /// Gets or sets a value for the maximim resize width. + /// Gets or sets a value for the maximum resize width. /// [DefaultValue(StaticMaxWidth)] public int MaxWidth { get; set; } = StaticMaxWidth; /// - /// Gets or sets a value for the maximim resize height. + /// Gets or sets a value for the maximum resize height. /// [DefaultValue(StaticMaxHeight)] public int MaxHeight { get; set; } = StaticMaxHeight; diff --git a/src/Umbraco.Core/Configuration/Models/ImagingSettings.cs b/src/Umbraco.Core/Configuration/Models/ImagingSettings.cs index 8232746ead..32bfeedb51 100644 --- a/src/Umbraco.Core/Configuration/Models/ImagingSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/ImagingSettings.cs @@ -4,18 +4,27 @@ namespace Umbraco.Cms.Core.Configuration.Models; /// -/// Typed configuration options for imaging settings. +/// Typed configuration options for imaging settings. /// [UmbracoOptions(Constants.Configuration.ConfigImaging)] public class ImagingSettings { /// - /// Gets or sets a value for imaging cache settings. + /// Gets or sets a value for the Hash-based Message Authentication Code (HMAC) secret key for request authentication. + /// + /// + /// Setting or updating this value will cause all existing generated URLs to become invalid and return a 400 Bad Request response code. + /// When set, the maximum resize settings are not used/validated anymore, because you can only request URLs with a valid HMAC token anyway. + /// + public byte[] HMACSecretKey { get; set; } = Array.Empty(); + + /// + /// Gets or sets a value for imaging cache settings. /// public ImagingCacheSettings Cache { get; set; } = new(); /// - /// Gets or sets a value for imaging resize settings. + /// Gets or sets a value for imaging resize settings. /// public ImagingResizeSettings Resize { get; set; } = new(); } diff --git a/src/Umbraco.Web.BackOffice/Controllers/ImagesController.cs b/src/Umbraco.Web.BackOffice/Controllers/ImagesController.cs index e718696ae3..787aa0070c 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/ImagesController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/ImagesController.cs @@ -1,3 +1,4 @@ +using System.Globalization; using System.Web; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.DependencyInjection; @@ -14,24 +15,24 @@ using Umbraco.Extensions; namespace Umbraco.Cms.Web.BackOffice.Controllers; /// -/// A controller used to return images for media +/// A controller used to return images for media. /// [PluginController(Constants.Web.Mvc.BackOfficeApiArea)] public class ImagesController : UmbracoAuthorizedApiController { - private readonly IImageUrlGenerator _imageUrlGenerator; private readonly MediaFileManager _mediaFileManager; + private readonly IImageUrlGenerator _imageUrlGenerator; private ContentSettings _contentSettings; [Obsolete("Use non obsolete-constructor. Scheduled for removal in Umbraco 13.")] public ImagesController( MediaFileManager mediaFileManager, IImageUrlGenerator imageUrlGenerator) - : this(mediaFileManager, - imageUrlGenerator, - StaticServiceProvider.Instance.GetRequiredService>()) + : this( + mediaFileManager, + imageUrlGenerator, + StaticServiceProvider.Instance.GetRequiredService>()) { - } [ActivatorUtilitiesConstructor] @@ -45,30 +46,29 @@ public class ImagesController : UmbracoAuthorizedApiController _contentSettings = contentSettingsMonitor.CurrentValue; contentSettingsMonitor.OnChange(x => _contentSettings = x); - } /// - /// Gets the big thumbnail image for the original image path + /// Gets the big thumbnail image for the original image path. /// /// /// /// - /// If there is no original image is found then this will return not found. + /// If there is no original image is found then this will return not found. /// - public IActionResult GetBigThumbnail(string originalImagePath) => - string.IsNullOrWhiteSpace(originalImagePath) - ? Ok() - : GetResized(originalImagePath, 500); + public IActionResult GetBigThumbnail(string originalImagePath) + => string.IsNullOrWhiteSpace(originalImagePath) + ? Ok() + : GetResized(originalImagePath, 500); /// - /// Gets a resized image for the image at the given path + /// Gets a resized image for the image at the given path. /// /// /// /// /// - /// If there is no media, image property or image file is found then this will return not found. + /// If there is no media, image property or image file is found then this will return not found. /// public IActionResult GetResized(string imagePath, int width) { @@ -76,7 +76,6 @@ public class ImagesController : UmbracoAuthorizedApiController // We cannot use the WebUtility, as we only want to encode the path, and not the entire string var encodedImagePath = HttpUtility.UrlPathEncode(imagePath); - var ext = Path.GetExtension(encodedImagePath); // check if imagePath is local to prevent open redirect @@ -91,13 +90,13 @@ public class ImagesController : UmbracoAuthorizedApiController return NotFound(); } - // redirect to ImageProcessor thumbnail with rnd generated from last modified time of original media file + // Redirect to thumbnail with cache buster value generated from last modified time of original media file DateTimeOffset? imageLastModified = null; try { imageLastModified = _mediaFileManager.FileSystem.GetLastModified(imagePath); } - catch (Exception) + catch { // if we get an exception here it's probably because the image path being requested is an image that doesn't exist // in the local media file system. This can happen if someone is storing an absolute path to an image online, which @@ -105,12 +104,12 @@ public class ImagesController : UmbracoAuthorizedApiController // so ignore and we won't set a last modified date. } - var rnd = imageLastModified.HasValue ? $"&rnd={imageLastModified:yyyyMMddHHmmss}" : null; + var cacheBusterValue = imageLastModified.HasValue ? imageLastModified.Value.ToFileTime().ToString("x", CultureInfo.InvariantCulture) : null; var imageUrl = _imageUrlGenerator.GetImageUrl(new ImageUrlGenerationOptions(encodedImagePath) { Width = width, ImageCropMode = ImageCropMode.Max, - CacheBusterValue = rnd + CacheBusterValue = cacheBusterValue }); if (imageUrl is not null) @@ -142,7 +141,7 @@ public class ImagesController : UmbracoAuthorizedApiController } /// - /// Gets a processed image for the image at the given path + /// Gets a processed image for the image at the given path /// /// /// @@ -150,14 +149,9 @@ public class ImagesController : UmbracoAuthorizedApiController /// /// /// - /// - /// - /// - /// - /// /// /// - /// If there is no media, image property or image file is found then this will return not found. + /// If there is no media, image property or image file is found then this will return not found. /// public string? GetProcessedImageUrl( string imagePath, @@ -166,7 +160,7 @@ public class ImagesController : UmbracoAuthorizedApiController decimal? focalPointLeft = null, decimal? focalPointTop = null, ImageCropMode mode = ImageCropMode.Max, - string cacheBusterValue = "", + string? cacheBusterValue = null, decimal? cropX1 = null, decimal? cropX2 = null, decimal? cropY1 = null, @@ -182,13 +176,11 @@ public class ImagesController : UmbracoAuthorizedApiController if (focalPointLeft.HasValue && focalPointTop.HasValue) { - options.FocalPoint = - new ImageUrlGenerationOptions.FocalPointPosition(focalPointLeft.Value, focalPointTop.Value); + options.FocalPoint = new ImageUrlGenerationOptions.FocalPointPosition(focalPointLeft.Value, focalPointTop.Value); } else if (cropX1.HasValue && cropX2.HasValue && cropY1.HasValue && cropY2.HasValue) { - options.Crop = - new ImageUrlGenerationOptions.CropCoordinates(cropX1.Value, cropY1.Value, cropX2.Value, cropY2.Value); + options.Crop = new ImageUrlGenerationOptions.CropCoordinates(cropX1.Value, cropY1.Value, cropX2.Value, cropY2.Value); } return _imageUrlGenerator.GetImageUrl(options); diff --git a/src/Umbraco.Web.Common/Extensions/ImageCropperTemplateCoreExtensions.cs b/src/Umbraco.Web.Common/Extensions/ImageCropperTemplateCoreExtensions.cs index 78a01dca2d..676b05317e 100644 --- a/src/Umbraco.Web.Common/Extensions/ImageCropperTemplateCoreExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/ImageCropperTemplateCoreExtensions.cs @@ -558,7 +558,7 @@ public static class ImageCropperTemplateCoreExtensions } var cacheBusterValue = - cacheBuster ? mediaItem.UpdateDate.ToFileTimeUtc().ToString(CultureInfo.InvariantCulture) : null; + cacheBuster ? mediaItem.UpdateDate.ToFileTimeUtc().ToString("x", CultureInfo.InvariantCulture) : null; return GetCropUrl( mediaItemUrl, diff --git a/src/Umbraco.Web.UI.Client/src/common/services/mediahelper.service.js b/src/Umbraco.Web.UI.Client/src/common/services/mediahelper.service.js index e98a597e76..14de3bb1c4 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/mediahelper.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/mediahelper.service.js @@ -313,7 +313,7 @@ function mediaHelper(umbRequestHelper, $http, $log) { var thumbnailUrl = umbRequestHelper.getApiUrl( "imagesApiBaseUrl", "GetBigThumbnail", - [{ originalImagePath: imagePath }]) + '&rnd=' + Math.random(); + [{ originalImagePath: imagePath }]); return thumbnailUrl; }, diff --git a/src/Umbraco.Web.UI.Client/src/views/components/blockcard/umbBlockCard.component.js b/src/Umbraco.Web.UI.Client/src/views/components/blockcard/umbBlockCard.component.js index 6d6872c1e7..edcec632db 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/blockcard/umbBlockCard.component.js +++ b/src/Umbraco.Web.UI.Client/src/views/components/blockcard/umbBlockCard.component.js @@ -14,7 +14,7 @@ } }); - function BlockCardController($scope, umbRequestHelper) { + function BlockCardController($scope, umbRequestHelper, mediaHelper) { const vm = this; vm.styleBackgroundImage = "none"; @@ -49,8 +49,10 @@ var path = umbRequestHelper.convertVirtualToAbsolutePath(vm.blockConfigModel.thumbnail); if (path.toLowerCase().endsWith(".svg") === false) { - path += "?width=400"; + + path = mediaHelper.getThumbnailFromPath(path); } + vm.styleBackgroundImage = `url('${path}')`; }; diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/fileupload/fileupload.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/fileupload/fileupload.controller.js index b4d59c683c..5d4776b8f4 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/fileupload/fileupload.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/fileupload/fileupload.controller.js @@ -53,16 +53,8 @@ // they contain different data structures so if we need to query against it we need to be aware of this. mediaHelper.registerFileResolver("Umbraco.UploadField", function (property, entity, thumbnail) { if (thumbnail) { - if (mediaHelper.detectIfImageByExtension(property.value)) { - //get default big thumbnail from image processor - var thumbnailUrl = property.value + "?width=500&rnd=" + moment(entity.updateDate).format("YYYYMMDDHHmmss"); - return thumbnailUrl; - } - else { - return null; - } - } - else { + return mediaHelper.getThumbnailFromPath(property.value); + } else { return property.value; } }); diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/editors/media.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/editors/media.controller.js index 81a548a116..71519c5245 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/editors/media.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/editors/media.controller.js @@ -1,10 +1,10 @@ angular.module("umbraco") .controller("Umbraco.PropertyEditors.Grid.MediaController", - function ($scope, userService, editorService, localizationService) { + function ($scope, userService, editorService, localizationService, mediaHelper) { $scope.control.icon = $scope.control.icon || 'icon-picture'; - $scope.thumbnailUrl = getThumbnailUrl(); + updateThumbnailUrl(); if (!$scope.model.config.startNodeId) { if ($scope.model.config.ignoreUserStartNodes === true) { @@ -61,40 +61,31 @@ angular.module("umbraco") /** * */ - function getThumbnailUrl() { - + function updateThumbnailUrl() { if ($scope.control.value && $scope.control.value.image) { - var url = $scope.control.value.image; + var options = { + width: 800 + }; - if ($scope.control.editor.config && $scope.control.editor.config.size){ - if ($scope.control.value.coordinates) { - // New way, crop by percent must come before width/height. - var coords = $scope.control.value.coordinates; - url += `?cc=${coords.x1},${coords.y1},${coords.x2},${coords.y2}`; - } else { - // Here in order not to break existing content where focalPoint were used. - if ($scope.control.value.focalPoint) { - url += `?rxy=${$scope.control.value.focalPoint.left},${$scope.control.value.focalPoint.top}`; - } else { - // Prevent black padding and no crop when focal point not set / changed from default - url += '?rxy=0.5,0.5'; - } - } - - url += '&width=' + $scope.control.editor.config.size.width; - url += '&height=' + $scope.control.editor.config.size.height; + if ($scope.control.value.coordinates) { + // Use crop + options.crop = $scope.control.value.coordinates; + } else if ($scope.control.value.focalPoint) { + // Otherwise use focal point + options.focalPoint = $scope.control.value.focalPoint; } - // set default size if no crop present (moved from the view) - if (url.includes('?') === false) - { - url += '?width=800' + if ($scope.control.editor.config && $scope.control.editor.config.size) { + options.width = $scope.control.editor.config.size.width; + options.height = $scope.control.editor.config.size.height; } - return url; + mediaHelper.getProcessedImageUrl($scope.control.value.image, options).then(imageUrl => { + $scope.thumbnailUrl = imageUrl; + }); + } else { + $scope.thumbnailUrl = null; } - - return null; } /** @@ -113,6 +104,7 @@ angular.module("umbraco") caption: selectedImage.caption, altText: selectedImage.altText }; - $scope.thumbnailUrl = getThumbnailUrl(); + + updateThumbnailUrl(); } }); diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/imagecropper/imagecropper.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/imagecropper/imagecropper.controller.js index b5131e9938..453347bc1b 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/imagecropper/imagecropper.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/imagecropper/imagecropper.controller.js @@ -236,9 +236,8 @@ angular.module('umbraco') if (property.value && property.value.src) { if (thumbnail === true) { - return property.value.src + "?width=500"; - } - else { + return mediaHelper.getThumbnailFromPath(property.value.src); + } else { return property.value.src; } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Media/ImageSharpImageUrlGeneratorTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Media/ImageSharpImageUrlGeneratorTests.cs index 40f28322dc..346bbc8a32 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Media/ImageSharpImageUrlGeneratorTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Media/ImageSharpImageUrlGeneratorTests.cs @@ -1,7 +1,14 @@ // Copyright (c) Umbraco. // See LICENSE for more details. +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using NUnit.Framework; +using SixLabors.ImageSharp.Web; +using SixLabors.ImageSharp.Web.Commands; +using SixLabors.ImageSharp.Web.Commands.Converters; +using SixLabors.ImageSharp.Web.Middleware; +using SixLabors.ImageSharp.Web.Processors; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Imaging.ImageSharp.Media; @@ -14,19 +21,22 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common.Media; public class ImageSharpImageUrlGeneratorTests { private const string MediaPath = "/media/1005/img_0671.jpg"; + private static readonly ImageUrlGenerationOptions.CropCoordinates _crop = new ImageUrlGenerationOptions.CropCoordinates(0.58729977382575338m, 0.055768992440203169m, 0m, 0.32457553600198386m); + private static readonly ImageUrlGenerationOptions.FocalPointPosition _focus1 = new ImageUrlGenerationOptions.FocalPointPosition(0.96m, 0.80827067669172936m); + private static readonly ImageUrlGenerationOptions.FocalPointPosition _focus2 = new ImageUrlGenerationOptions.FocalPointPosition(0.4275m, 0.41m); + private static readonly ImageSharpImageUrlGenerator _generator = new ImageSharpImageUrlGenerator(new string[0]); - private static readonly ImageUrlGenerationOptions.CropCoordinates _sCrop = new(0.58729977382575338m, 0.055768992440203169m, 0m, 0.32457553600198386m); - private static readonly ImageUrlGenerationOptions.FocalPointPosition _sFocus = new(0.96m, 0.80827067669172936m); - private static readonly ImageSharpImageUrlGenerator _sGenerator = new(Array.Empty()); - - /// - /// Tests that the media path is returned if no options are provided. - /// [Test] public void GivenMediaPath_AndNoOptions_ReturnsMediaPath() { - var actual = _sGenerator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath)); - Assert.AreEqual(MediaPath, actual); + var urlString = _generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) + { + Crop = _crop, + Width = 100, + Height = 100, + }); + + Assert.AreEqual(MediaPath + "?cc=0.58729977382575338,0.055768992440203169,0,0.32457553600198386&width=100&height=100", urlString); } /// @@ -35,8 +45,14 @@ public class ImageSharpImageUrlGeneratorTests [Test] public void GivenNullOptions_ReturnsNull() { - var actual = _sGenerator.GetImageUrl(null); - Assert.IsNull(actual); + var urlString = _generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) + { + FocalPoint = _focus1, + Width = 200, + Height = 300, + }); + + Assert.AreEqual(MediaPath + "?rxy=0.96,0.80827067669172936&width=200&height=300", urlString); } /// @@ -45,14 +61,34 @@ public class ImageSharpImageUrlGeneratorTests [Test] public void GivenNullImageUrl_ReturnsNull() { - var actual = _sGenerator.GetImageUrl(new ImageUrlGenerationOptions(null)); - Assert.IsNull(actual); + var urlString = _generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) + { + FocalPoint = _focus1, + Width = 100, + Height = 100, + }); + + Assert.AreEqual(MediaPath + "?rxy=0.96,0.80827067669172936&width=100&height=100", urlString); + } + + [Test] + public void GetImageUrlFurtherOptionsTest() + { + var urlString = _generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) + { + FocalPoint = _focus1, + Width = 200, + Height = 300, + FurtherOptions = "&filter=comic&roundedcorners=radius-26|bgcolor-fff", + }); + + Assert.AreEqual(MediaPath + "?rxy=0.96,0.80827067669172936&width=200&height=300&filter=comic&roundedcorners=radius-26%7Cbgcolor-fff", urlString); } [Test] public void GetImageUrlFurtherOptionsModeAndQualityTest() { - var urlString = _sGenerator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) + var urlString = _generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { Quality = 10, FurtherOptions = "format=webp", @@ -66,7 +102,7 @@ public class ImageSharpImageUrlGeneratorTests [Test] public void GetImageUrlFurtherOptionsWithModeAndQualityTest() { - var urlString = _sGenerator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) + var urlString = _generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { FurtherOptions = "quality=10&format=webp", }); @@ -77,62 +113,101 @@ public class ImageSharpImageUrlGeneratorTests } /// - /// Test that if an empty string image url is given, null is returned. + /// Test that if options is null, the generated image URL is also null. /// [Test] public void GivenEmptyStringImageUrl_ReturnsEmptyString() { - var actual = _sGenerator.GetImageUrl(new ImageUrlGenerationOptions(string.Empty)); - Assert.AreEqual(actual, string.Empty); + var urlString = _generator.GetImageUrl(null); + Assert.AreEqual(null, urlString); } /// - /// Tests the correct query string is returned when given a crop. + /// Test that if the image URL is null, the generated image URL is also null. /// [Test] public void GivenCrop_ReturnsExpectedQueryString() { - const string expected = "?cc=0.58729977382575338,0.055768992440203169,0,0.32457553600198386"; - var actual = _sGenerator.GetImageUrl(new ImageUrlGenerationOptions(string.Empty) { Crop = _sCrop }); - Assert.AreEqual(expected, actual); + var urlString = _generator.GetImageUrl(new ImageUrlGenerationOptions(null)); + Assert.AreEqual(null, urlString); } /// - /// Tests the correct query string is returned when given a width. + /// Test that if the image URL is empty, the generated image URL is empty. /// [Test] public void GivenWidth_ReturnsExpectedQueryString() { - const string expected = "?width=200"; - var actual = _sGenerator.GetImageUrl(new ImageUrlGenerationOptions(string.Empty) { Width = 200 }); - Assert.AreEqual(expected, actual); + var urlString = _generator.GetImageUrl(new ImageUrlGenerationOptions(string.Empty)); + Assert.AreEqual(string.Empty, urlString); } /// - /// Tests the correct query string is returned when given a height. + /// Test the GetImageUrl method on the ImageCropDataSet Model /// [Test] public void GivenHeight_ReturnsExpectedQueryString() { - const string expected = "?height=200"; - var actual = _sGenerator.GetImageUrl(new ImageUrlGenerationOptions(string.Empty) { Height = 200 }); - Assert.AreEqual(expected, actual); + var urlString = _generator.GetImageUrl(new ImageUrlGenerationOptions(string.Empty) + { + Crop = _crop, + Width = 100, + Height = 100, + }); + + Assert.AreEqual("?cc=0.58729977382575338,0.055768992440203169,0,0.32457553600198386&width=100&height=100", urlString); } /// - /// Tests the correct query string is returned when provided a focal point. + /// Test that if Crop mode is specified as anything other than Crop the image doesn't use the crop /// [Test] public void GivenFocalPoint_ReturnsExpectedQueryString() { - const string expected = "?rxy=0.96,0.80827067669172936"; - var actual = _sGenerator.GetImageUrl(new ImageUrlGenerationOptions(string.Empty) { FocalPoint = _sFocus }); - Assert.AreEqual(expected, actual); + var urlStringMin = _generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) + { + ImageCropMode = ImageCropMode.Min, + Width = 300, + Height = 150, + }); + + var urlStringBoxPad = _generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) + { + ImageCropMode = ImageCropMode.BoxPad, + Width = 300, + Height = 150, + }); + + var urlStringPad = _generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) + { + ImageCropMode = ImageCropMode.Pad, + Width = 300, + Height = 150, + }); + + var urlStringMax = _generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) + { + ImageCropMode = ImageCropMode.Max, + Width = 300, + Height = 150, + }); + + var urlStringStretch = _generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) + { + ImageCropMode = ImageCropMode.Stretch, + Width = 300, + Height = 150, + }); + + Assert.AreEqual(MediaPath + "?rmode=min&width=300&height=150", urlStringMin); + Assert.AreEqual(MediaPath + "?rmode=boxpad&width=300&height=150", urlStringBoxPad); + Assert.AreEqual(MediaPath + "?rmode=pad&width=300&height=150", urlStringPad); + Assert.AreEqual(MediaPath + "?rmode=max&width=300&height=150", urlStringMax); + Assert.AreEqual(MediaPath + "?rmode=stretch&width=300&height=150", urlStringStretch); } /// - /// Tests the correct query string is returned when given further options. - /// There are a few edge case inputs here to ensure thorough testing in future versions. + /// Test for upload property type /// [TestCase("&filter=comic&roundedcorners=radius-26%7Cbgcolor-fff", "?filter=comic&roundedcorners=radius-26%7Cbgcolor-fff")] [TestCase("testoptions", "?testoptions=")] @@ -140,100 +215,84 @@ public class ImageSharpImageUrlGeneratorTests [TestCase("should=encode&$^%()", "?should=encode&$%5E%25()=")] public void GivenFurtherOptions_ReturnsExpectedQueryString(string input, string expected) { - var actual = _sGenerator.GetImageUrl(new ImageUrlGenerationOptions(string.Empty) + var urlString = _generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { FurtherOptions = input, }); - Assert.AreEqual(expected, actual); + + Assert.AreEqual(MediaPath + expected, urlString); } /// - /// Test that the correct query string is returned for all image crop modes. + /// Test for preferFocalPoint when focal point is centered /// - [TestCase(ImageCropMode.Min, "?rmode=min")] - [TestCase(ImageCropMode.BoxPad, "?rmode=boxpad")] - [TestCase(ImageCropMode.Pad, "?rmode=pad")] - [TestCase(ImageCropMode.Max, "?rmode=max")] - [TestCase(ImageCropMode.Stretch, "?rmode=stretch")] - public void GivenCropMode_ReturnsExpectedQueryString(ImageCropMode cropMode, string expectedQueryString) + [Test] + public void GetImageUrl_PreferFocalPointCenter() { - var cropUrl = _sGenerator.GetImageUrl(new ImageUrlGenerationOptions(string.Empty) + var urlString = _generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { - ImageCropMode = cropMode, + Width = 300, + Height = 150, }); - Assert.AreEqual(expectedQueryString, cropUrl); + Assert.AreEqual(MediaPath + "?width=300&height=150", urlString); } /// - /// Test that the correct query string is returned for all image crop anchors. + /// Test to check if crop ratio is ignored if useCropDimensions is true /// - [TestCase(ImageCropAnchor.Bottom, "?ranchor=bottom")] - [TestCase(ImageCropAnchor.BottomLeft, "?ranchor=bottomleft")] - [TestCase(ImageCropAnchor.BottomRight, "?ranchor=bottomright")] - [TestCase(ImageCropAnchor.Center, "?ranchor=center")] - [TestCase(ImageCropAnchor.Left, "?ranchor=left")] - [TestCase(ImageCropAnchor.Right, "?ranchor=right")] - [TestCase(ImageCropAnchor.Top, "?ranchor=top")] - [TestCase(ImageCropAnchor.TopLeft, "?ranchor=topleft")] - [TestCase(ImageCropAnchor.TopRight, "?ranchor=topright")] - public void GivenCropAnchor_ReturnsExpectedQueryString(ImageCropAnchor imageCropAnchor, string expectedQueryString) + [Test] + public void GetImageUrl_PreDefinedCropNoCoordinatesWithWidthAndFocalPointIgnore() { - var actual = _sGenerator.GetImageUrl(new ImageUrlGenerationOptions(string.Empty) + var urlString = _generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { - ImageCropAnchor = imageCropAnchor, + FocalPoint = _focus2, + Width = 270, + Height = 161, }); - Assert.AreEqual(expectedQueryString, actual); + + Assert.AreEqual(MediaPath + "?rxy=0.4275,0.41&width=270&height=161", urlString); } /// - /// Tests that the quality query string always returns the input number regardless of value. + /// Test to check result when only a width parameter is passed, effectivly a resize only /// - [TestCase(int.MinValue)] - [TestCase(-50)] - [TestCase(0)] - [TestCase(50)] - [TestCase(int.MaxValue)] - public void GivenQuality_ReturnsExpectedQueryString(int quality) + [Test] + public void GetImageUrl_WidthOnlyParameter() { - var expected = "?quality=" + quality; - var actual = _sGenerator.GetImageUrl(new ImageUrlGenerationOptions(string.Empty) + var urlString = _generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { - Quality = quality, + Width = 200, }); - Assert.AreEqual(expected, actual); + + Assert.AreEqual(MediaPath + "?width=200", urlString); } /// - /// Tests that the correct query string is returned for cache buster. - /// There are some edge case tests here to ensure thorough testing in future versions. + /// Test to check result when only a height parameter is passed, effectivly a resize only /// - [TestCase("test-buster", "?rnd=test-buster")] - [TestCase("test-buster&&^-value", "?rnd=test-buster%26%26%5E-value")] - public void GivenCacheBusterValue_ReturnsExpectedQueryString(string input, string expected) + [Test] + public void GetImageUrl_HeightOnlyParameter() { - var actual = _sGenerator.GetImageUrl(new ImageUrlGenerationOptions(string.Empty) + var urlString = _generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { - CacheBusterValue = input, + Height = 200, }); - Assert.AreEqual(expected, actual); + + Assert.AreEqual(MediaPath + "?height=200", urlString); } /// - /// Tests that an expected query string is returned when all options are given. - /// This will be a good test to see if something breaks with ordering of query string parameters. + /// Test to check result when using a background color with padding /// [Test] public void GivenAllOptions_ReturnsExpectedQueryString() { - const string expected = - "/media/1005/img_0671.jpg?cc=0.58729977382575338,0.055768992440203169,0,0.32457553600198386&rxy=0.96,0.80827067669172936&rmode=stretch&ranchor=right&width=200&height=200&quality=50&more=options&rnd=buster"; - - var actual = _sGenerator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) + var urlString = _generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { Quality = 50, - Crop = _sCrop, - FocalPoint = _sFocus, + Crop = _crop, + FocalPoint = _focus1, CacheBusterValue = "buster", FurtherOptions = "more=options", Height = 200, @@ -242,6 +301,47 @@ public class ImageSharpImageUrlGeneratorTests ImageCropMode = ImageCropMode.Stretch, }); - Assert.AreEqual(expected, actual); + Assert.AreEqual(MediaPath + "?cc=0.58729977382575338,0.055768992440203169,0,0.32457553600198386&rxy=0.96,0.80827067669172936&rmode=stretch&ranchor=right&width=200&height=200&quality=50&more=options&v=buster", urlString); + } + + /// + /// Test to check result when using a HMAC security key. + /// + [Test] + public void GetImageUrl_HMACSecurityKey() + { + var requestAuthorizationUtilities = new RequestAuthorizationUtilities( + Options.Create(new ImageSharpMiddlewareOptions() + { + HMACSecretKey = new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31 } + }), + new QueryCollectionRequestParser(), + new[] + { + new ResizeWebProcessor() + }, + new CommandParser(Enumerable.Empty()), + new ServiceCollection().BuildServiceProvider()); + + var generator = new ImageSharpImageUrlGenerator(new string[0], requestAuthorizationUtilities); + var options = new ImageUrlGenerationOptions(MediaPath) + { + Width = 400, + Height = 400, + }; + + Assert.AreEqual(MediaPath + "?width=400&height=400&hmac=6335195986da0663e23eaadfb9bb32d537375aaeec253aae66b8f4388506b4b2", generator.GetImageUrl(options)); + + // CacheBusterValue isn't included in HMAC generation + options.CacheBusterValue = "not-included-in-hmac"; + Assert.AreEqual(MediaPath + "?width=400&height=400&v=not-included-in-hmac&hmac=6335195986da0663e23eaadfb9bb32d537375aaeec253aae66b8f4388506b4b2", generator.GetImageUrl(options)); + + // Removing height should generate a different HMAC + options.Height = null; + Assert.AreEqual(MediaPath + "?width=400&v=not-included-in-hmac&hmac=5bd24a05de5ea068533579863773ddac9269482ad515575be4aace7e9e50c88c", generator.GetImageUrl(options)); + + // But adding it again using FurtherOptions should include it (and produce the same HMAC as before) + options.FurtherOptions = "height=400"; + Assert.AreEqual(MediaPath + "?width=400&height=400&v=not-included-in-hmac&hmac=6335195986da0663e23eaadfb9bb32d537375aaeec253aae66b8f4388506b4b2", generator.GetImageUrl(options)); } }