V12: Fix ImageSharp3 performance issue

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,23 +20,29 @@ namespace Umbraco.Cms.Imaging.ImageSharp.Media;
 public sealed class ImageSharpImageUrlGenerator : IImageUrlGenerator
 {
     private readonly RequestAuthorizationUtilities? _requestAuthorizationUtilities;
+    private readonly ImageSharpMiddlewareOptions _options;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="ImageSharpImageUrlGenerator" /> class.
     /// </summary>
     /// <param name="configuration">The ImageSharp configuration.</param>
     /// <param name="requestAuthorizationUtilities">Contains helpers that allow authorization of image requests.</param>
-    public ImageSharpImageUrlGenerator(Configuration configuration, RequestAuthorizationUtilities? requestAuthorizationUtilities)
-        : this(configuration.ImageFormats.SelectMany(f => f.FileExtensions).ToArray(), requestAuthorizationUtilities)
-    { }
+    /// <param name="options"></param>
+    public ImageSharpImageUrlGenerator(
+        Configuration configuration,
+        RequestAuthorizationUtilities? requestAuthorizationUtilities,
+        IOptions<ImageSharpMiddlewareOptions> options)
+        : this(configuration.ImageFormats.SelectMany(f => f.FileExtensions).ToArray(), options, requestAuthorizationUtilities)
+    {
+    }
 
     /// <summary>
     /// Initializes a new instance of the <see cref="ImageSharpImageUrlGenerator" /> class.
     /// </summary>
     /// <param name="configuration">The ImageSharp configuration.</param>
     [Obsolete("Use ctor with all params - This will be removed in Umbraco 13.")]
     public ImageSharpImageUrlGenerator(Configuration configuration)
-        : this(configuration, StaticServiceProvider.Instance.GetService<RequestAuthorizationUtilities>())
+        : this(configuration, StaticServiceProvider.Instance.GetService<RequestAuthorizationUtilities>(), StaticServiceProvider.Instance.GetRequiredService<IOptions<ImageSharpMiddlewareOptions>>())
     { }
 
     /// <summary>
@@ -45,10 +53,11 @@ public ImageSharpImageUrlGenerator(Configuration configuration)
     /// <remarks>
     /// This constructor is only used for testing.
     /// </remarks>
-    internal ImageSharpImageUrlGenerator(IEnumerable<string> supportedImageFileTypes, RequestAuthorizationUtilities? requestAuthorizationUtilities = null)
+    internal ImageSharpImageUrlGenerator(IEnumerable<string> supportedImageFileTypes, IOptions<ImageSharpMiddlewareOptions> options, RequestAuthorizationUtilities? requestAuthorizationUtilities = null)
     {
         SupportedImageFileTypes = supportedImageFileTypes;
         _requestAuthorizationUtilities = requestAuthorizationUtilities;
+        _options = options.Value;
     }
 
     /// <inheritdoc />
@@ -115,10 +124,17 @@ internal ImageSharpImageUrlGenerator(IEnumerable<string> supportedImageFileTypes
             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);
             }

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<string>(), Options.Create(new ImageSharpMiddlewareOptions()));
 
     [Test]
     public void GivenMediaPath_AndNoOptions_ReturnsMediaPath()
@@ -310,11 +310,16 @@ public void GivenAllOptions_ReturnsExpectedQueryString()
     [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 void GetImageUrl_HMACSecurityKey()
             new CommandParser(Enumerable.Empty<ICommandConverter>()),
             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";