Merge pull request #829 from dlcs/feature/prevent_zero_size

Prevent 0 size image request
dlcs · Apr 25, 2024 · 9b11e82 · 9b11e82
2 parents 5e5d943 + 927a018
commit 9b11e82
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 6 deletions.
diff --git a/src/protagonist/Orchestrator.Tests/Features/Images/ImageRequestHandlerTests.cs b/src/protagonist/Orchestrator.Tests/Features/Images/ImageRequestHandlerTests.cs
@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Net;
+using System.Text.Json.Nodes;
 using System.Threading;
 using DLCS.Core.Types;
 using DLCS.Model.Assets.CustomHeaders;
@@ -17,6 +18,7 @@
 using Orchestrator.Infrastructure.Auth;
 using Orchestrator.Infrastructure.ReverseProxy;
 using Orchestrator.Settings;
+using Test.Helpers.Data;
 using Version = IIIF.ImageApi.Version;
 
 namespace Orchestrator.Tests.Features.Images;
@@ -126,6 +128,31 @@ public async Task HandleRequest_Returns400_IfAssetPathParserThrowsException()
             .Which.StatusCode.Should().Be(HttpStatusCode.BadRequest);
     }
 
+    [Theory]
+    [InlineData("0,")]
+    [InlineData(",0")]
+    [InlineData("!0,0")]
+    [InlineData("20,0")]
+    [InlineData("0,20")]
+    public async Task HandleRequest_Returns400_IfInvalidSize(string size)
+    {
+        // Arrange
+        var id = AssetIdGenerator.GetAssetId();
+
+        // Act
+        var context = new DefaultHttpContext();
+        context.Request.Path = $"/iiif-img/{id}/full/{size}/0/default.jpg";
+
+        var sut = GetImageRequestHandlerWithMockPathParser();
+
+        // Act
+        var result = await sut.HandleRequest(context);
+
+        // Assert
+        result.Should().BeOfType<StatusCodeResult>()
+            .Which.StatusCode.Should().Be(HttpStatusCode.BadRequest);
+    }
+
     [Theory]
     [InlineData(AvailableDeliveryChannel.File)]
     [InlineData(AvailableDeliveryChannel.Timebased)]

diff --git a/src/protagonist/Orchestrator.Tests/Integration/ImageHandlingTests.cs b/src/protagonist/Orchestrator.Tests/Integration/ImageHandlingTests.cs
@@ -1606,9 +1606,9 @@ public async Task Get_Returns500_IfRedirectsImageServer_ButOrchestratorError()
     }
 
     [Theory]
-    [InlineData("/info.json")]
-    [InlineData("/full/max/0/default.jpg")]
-    [InlineData("/0,0,1000,1000/200,200/0/default.jpg")]
+    [InlineData("info.json")]
+    [InlineData("full/max/0/default.jpg")]
+    [InlineData("0,0,1000,1000/200,200/0/default.jpg")]
     public async Task Get_404_IfNotForDelivery(string path)
     {
         // Arrange
@@ -1630,9 +1630,9 @@ public async Task Get_404_IfNotForDelivery(string path)
     }
 
     [Theory]
-    [InlineData("/info.json")]
-    [InlineData("/full/max/0/default.jpg")]
-    [InlineData("/0,0,1000,1000/200,200/0/default.jpg")]
+    [InlineData("info.json")]
+    [InlineData("full/max/0/default.jpg")]
+    [InlineData("0,0,1000,1000/200,200/0/default.jpg")]
     public async Task Get_404_IfNotForImageDeliveryChannel(string path)
     {
         // Arrange

diff --git a/src/protagonist/Orchestrator/Features/Images/ImageRequestHandler.cs b/src/protagonist/Orchestrator/Features/Images/ImageRequestHandler.cs
@@ -70,6 +70,12 @@ public async Task<IProxyActionResult> HandleRequest(HttpContext httpContext)
         {
             return new StatusCodeResult(statusCode ?? HttpStatusCode.InternalServerError);
         }
+
+        if (!IsSizeValid(assetRequest.IIIFImageRequest.Size))
+        {
+            logger.LogDebug("Request for {Path}: invalid size", httpContext.Request.Path);
+            return new StatusCodeResult(HttpStatusCode.BadRequest);
+        }
 
         var orchestrationImage = await assetRequestProcessor.GetAsset<OrchestrationImage>(httpContext, assetRequest);
         if (orchestrationImage == null)
@@ -105,6 +111,8 @@ public async Task<IProxyActionResult> HandleRequest(HttpContext httpContext)
         return proxyActionResult;
     }
 
+    private bool IsSizeValid(SizeParameter size) => (size.Width ?? 1) > 0 && (size.Height ?? 1) > 0;
+
     private async Task<IProxyActionResult> HandleRequestInternal(HttpContext httpContext,
         OrchestrationImage orchestrationImage, ImageAssetDeliveryRequest assetRequest)
     {