diff --git a/src/Umbraco.Cms.Imaging.ImageSharp/Media/ImageSharpImageUrlGenerator.cs b/src/Umbraco.Cms.Imaging.ImageSharp/Media/ImageSharpImageUrlGenerator.cs index 33c0d862b5..5b437902f5 100644 --- a/src/Umbraco.Cms.Imaging.ImageSharp/Media/ImageSharpImageUrlGenerator.cs +++ b/src/Umbraco.Cms.Imaging.ImageSharp/Media/ImageSharpImageUrlGenerator.cs @@ -1,7 +1,9 @@ using System.Globalization; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; +using SixLabors.ImageSharp.Web.Middleware; using SixLabors.ImageSharp.Web.Processors; using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Media; @@ -18,15 +20,21 @@ namespace Umbraco.Cms.Imaging.ImageSharp.Media; public sealed class ImageSharpImageUrlGenerator : IImageUrlGenerator { private readonly RequestAuthorizationUtilities? _requestAuthorizationUtilities; + private readonly ImageSharpMiddlewareOptions _options; /// /// Initializes a new instance of the class. /// /// The ImageSharp configuration. /// Contains helpers that allow authorization of image requests. - public ImageSharpImageUrlGenerator(Configuration configuration, RequestAuthorizationUtilities? requestAuthorizationUtilities) - : this(configuration.ImageFormats.SelectMany(f => f.FileExtensions).ToArray(), requestAuthorizationUtilities) - { } + /// + public ImageSharpImageUrlGenerator( + Configuration configuration, + RequestAuthorizationUtilities? requestAuthorizationUtilities, + IOptions options) + : this(configuration.ImageFormats.SelectMany(f => f.FileExtensions).ToArray(), options, requestAuthorizationUtilities) + { + } /// /// Initializes a new instance of the class. @@ -34,7 +42,7 @@ public sealed class ImageSharpImageUrlGenerator : IImageUrlGenerator /// 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()) + : this(configuration, StaticServiceProvider.Instance.GetService(), StaticServiceProvider.Instance.GetRequiredService>()) { } /// @@ -45,10 +53,11 @@ public sealed class ImageSharpImageUrlGenerator : IImageUrlGenerator /// /// This constructor is only used for testing. /// - internal ImageSharpImageUrlGenerator(IEnumerable supportedImageFileTypes, RequestAuthorizationUtilities? requestAuthorizationUtilities = null) + internal ImageSharpImageUrlGenerator(IEnumerable supportedImageFileTypes, IOptions options, RequestAuthorizationUtilities? requestAuthorizationUtilities = null) { SupportedImageFileTypes = supportedImageFileTypes; _requestAuthorizationUtilities = requestAuthorizationUtilities; + _options = options.Value; } /// @@ -115,10 +124,17 @@ public sealed class ImageSharpImageUrlGenerator : IImageUrlGenerator queryString.Add("v", cacheBusterValue); } - if (_requestAuthorizationUtilities is not null) + // If no secret is we'll completely skip this whole thing, in theory the ComputeHMACAsync should just return null imidiately, but still no reason to create + if (_options.HMACSecretKey.Length != 0 && _requestAuthorizationUtilities is not null) { var uri = QueryHelpers.AddQueryString(options.ImageUrl, queryString); - if (_requestAuthorizationUtilities.ComputeHMAC(uri, CommandHandling.Sanitize) is string token && !string.IsNullOrEmpty(token)) + + // It's important that we call the async version here. + // This is because if we call the synchronous version, we ImageSharp will start a new Task ever single time. + // This becomes a huge problem if the site is under load, and will result in massive spikes in response time. + // See https://github.com/SixLabors/ImageSharp.Web/blob/main/src/ImageSharp.Web/AsyncHelper.cs#L24 + var token = _requestAuthorizationUtilities.ComputeHMACAsync(uri, CommandHandling.Sanitize).GetAwaiter().GetResult(); + if (string.IsNullOrEmpty(token) is false) { queryString.Add(RequestAuthorizationUtilities.TokenCommand, token); } 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 346bbc8a32..49249d7bf2 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Media/ImageSharpImageUrlGeneratorTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Web.Common/Media/ImageSharpImageUrlGeneratorTests.cs @@ -24,7 +24,7 @@ public class ImageSharpImageUrlGeneratorTests 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 ImageSharpImageUrlGenerator _generator = new ImageSharpImageUrlGenerator(Array.Empty(), Options.Create(new ImageSharpMiddlewareOptions())); [Test] public void GivenMediaPath_AndNoOptions_ReturnsMediaPath() @@ -310,11 +310,16 @@ public class ImageSharpImageUrlGeneratorTests [Test] public void GetImageUrl_HMACSecurityKey() { - var requestAuthorizationUtilities = new RequestAuthorizationUtilities( - Options.Create(new ImageSharpMiddlewareOptions() + var middleWareOptions = Options.Create(new ImageSharpMiddlewareOptions() + { + HMACSecretKey = new byte[] { - 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 } - }), + 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 + } + }); + + var requestAuthorizationUtilities = new RequestAuthorizationUtilities( + middleWareOptions, new QueryCollectionRequestParser(), new[] { @@ -323,14 +328,15 @@ public class ImageSharpImageUrlGeneratorTests new CommandParser(Enumerable.Empty()), new ServiceCollection().BuildServiceProvider()); - var generator = new ImageSharpImageUrlGenerator(new string[0], requestAuthorizationUtilities); + var generator = new ImageSharpImageUrlGenerator(new string[0], middleWareOptions, requestAuthorizationUtilities); var options = new ImageUrlGenerationOptions(MediaPath) { Width = 400, Height = 400, }; - Assert.AreEqual(MediaPath + "?width=400&height=400&hmac=6335195986da0663e23eaadfb9bb32d537375aaeec253aae66b8f4388506b4b2", generator.GetImageUrl(options)); + var actual = generator.GetImageUrl(options); + Assert.AreEqual(MediaPath + "?width=400&height=400&hmac=6335195986da0663e23eaadfb9bb32d537375aaeec253aae66b8f4388506b4b2", actual); // CacheBusterValue isn't included in HMAC generation options.CacheBusterValue = "not-included-in-hmac";