From 4ab58326b52a2dd5e56e21eb379ea668e072253e Mon Sep 17 00:00:00 2001 From: griffri Date: Thu, 11 Jul 2024 16:31:43 +0100 Subject: [PATCH 1/4] Update `ImageRequest.Parse` to reject empty or a wrong number of values --- .../IIIF.Tests/ImageApi/ImageRequestXTests.cs | 64 ++++++++++++++++++- src/IIIF/IIIF/ImageApi/ImageRequest.cs | 13 +++- 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/src/IIIF/IIIF.Tests/ImageApi/ImageRequestXTests.cs b/src/IIIF/IIIF.Tests/ImageApi/ImageRequestXTests.cs index 3b2f183..1e939c9 100644 --- a/src/IIIF/IIIF.Tests/ImageApi/ImageRequestXTests.cs +++ b/src/IIIF/IIIF.Tests/ImageApi/ImageRequestXTests.cs @@ -1,6 +1,6 @@ -using FluentAssertions; +using System; +using FluentAssertions; using IIIF.ImageApi; -using Xunit; namespace IIIF.Tests.ImageApi; @@ -221,4 +221,64 @@ public void Parse_CorrectPercentageScaled() // Assert result.Should().BeEquivalentTo(expected); } + + [Theory] + [InlineData("my-asset")] + [InlineData("my-asset/")] + public void Parse_IsBase(string path) + { + // Arrange and Act + const string prefix = "iiif-img/27/1/"; + var result = ImageRequest.Parse($"{prefix}{path}", prefix); + + // Assert + result.IsBase.Should().BeTrue(); + } + + [Theory] + [InlineData("iiif-img/27/1/")] + [InlineData("/iiif-img/27/1/")] + [InlineData("/iiif-img/27/1")] + [InlineData("iiif-img/27/1")] + public void Parse_Validate_Succeeds(string prefix) + { + // Arrange and Act + const string request = $"iiif-img/27/1/my-asset/full/800,/0/default.jpg"; + var action = () => ImageRequest.Parse(request, prefix, true); + + // Assert + action.Should().NotThrow(); + } + + [Theory] + [InlineData("my-asset//full/800,/0/default.jpg")] + [InlineData("my-asset/full//800,/0/default.jpg")] + [InlineData("my-asset/full/800,//0/default.jpg")] + [InlineData("my-asset/full/800,/0//default.jpg")] + public void Parse_Validate_Fails_WhenGivenExtraSegments(string path) + { + // Arrange and Act + const string prefix = "iiif-img/27/1/"; + var action = () => ImageRequest.Parse($"{prefix}{path}", prefix, true); + + // Assert + action.Should().ThrowExactly(); + } + + [Theory] + [InlineData("my-asset//800,/0/default.jpg")] + [InlineData("my-asset/full//0/default.jpg")] + [InlineData("my-asset/full/800,//default.jpg")] + [InlineData("my-asset/full/800,/0/")] + [InlineData("my-asset////default.jpg")] + [InlineData("my-asset////")] + public void Parse_TryParse_Fails_WhenGivenEmptyParameters(string path) + { + // Arrange and Act + const string prefix = "iiif-img/27/1/"; + var action = () => ImageRequest.Parse($"{prefix}{path}", prefix, true); + + // Assert + action.Should().ThrowExactly(); + } } \ No newline at end of file diff --git a/src/IIIF/IIIF/ImageApi/ImageRequest.cs b/src/IIIF/IIIF/ImageApi/ImageRequest.cs index ce22863..68d0821 100644 --- a/src/IIIF/IIIF/ImageApi/ImageRequest.cs +++ b/src/IIIF/IIIF/ImageApi/ImageRequest.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; namespace IIIF.ImageApi; @@ -25,7 +26,7 @@ public class ImageRequest /// public string ImageRequestPath => OriginalPath.Replace(Identifier, string.Empty); - public static ImageRequest Parse(string path, string prefix) + public static ImageRequest Parse(string path, string prefix, bool validateSegments = false) { if (path[0] == '/') path = path[1..]; @@ -35,12 +36,15 @@ public static ImageRequest Parse(string path, string prefix) if (prefix != path[..prefix.Length]) throw new ArgumentException("Path does not start with prefix", nameof(prefix)); path = path[prefix.Length..]; + if (path[0] == '/') path = path[1..]; } var request = new ImageRequest { Prefix = prefix }; + var parts = path.Split('/'); + request.Identifier = parts[0]; - if (parts.Length == 1 || parts[1] == string.Empty) + if (parts.Length == 1 || (parts.Length == 2 && parts[1] == string.Empty)) { // likely the server will want to redirect this request.IsBase = true; @@ -52,6 +56,11 @@ public static ImageRequest Parse(string path, string prefix) request.IsInformationRequest = true; return request; } + + if (validateSegments && (parts.Length != 5 || parts.Any(string.IsNullOrEmpty))) + { + throw new ArgumentException("Path contains empty or an invalid number of parameters"); + } request.OriginalPath = path; request.Region = RegionParameter.Parse(parts[1]); From 3324df152855da1c046cfa5a3e9f26bdeaaedbb6 Mon Sep 17 00:00:00 2001 From: griffri Date: Fri, 12 Jul 2024 13:42:03 +0100 Subject: [PATCH 2/4] Add info.json related tests --- .../IIIF.Tests/ImageApi/ImageRequestXTests.cs | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/IIIF/IIIF.Tests/ImageApi/ImageRequestXTests.cs b/src/IIIF/IIIF.Tests/ImageApi/ImageRequestXTests.cs index 1e939c9..a2b1268 100644 --- a/src/IIIF/IIIF.Tests/ImageApi/ImageRequestXTests.cs +++ b/src/IIIF/IIIF.Tests/ImageApi/ImageRequestXTests.cs @@ -234,6 +234,28 @@ public void Parse_IsBase(string path) // Assert result.IsBase.Should().BeTrue(); } + + [Fact] + public void Parse_InfoJson() + { + // Arrange and Act + const string prefix = "iiif-img/27/1/"; + var action = () => ImageRequest.Parse($"{prefix}my-asset/info.json", prefix); + + // Assert + action.Should().NotThrow(); + } + + [Fact] + public void Parse_Fails_WhenInfoHasInvalidExtension() + { + // Arrange and Act + const string prefix = "iiif-img/27/1/"; + var action = () => ImageRequest.Parse($"{prefix}my-asset/info.jsonll", prefix); + + // Assert + action.Should().ThrowExactly(); + } [Theory] [InlineData("iiif-img/27/1/")] @@ -272,7 +294,7 @@ public void Parse_Validate_Fails_WhenGivenExtraSegments(string path) [InlineData("my-asset/full/800,/0/")] [InlineData("my-asset////default.jpg")] [InlineData("my-asset////")] - public void Parse_TryParse_Fails_WhenGivenEmptyParameters(string path) + public void Parse_Validate_Fails_WhenGivenEmptyParameters(string path) { // Arrange and Act const string prefix = "iiif-img/27/1/"; From 77e1f1467c8dc56e83e51a0a671742f4a6148c47 Mon Sep 17 00:00:00 2001 From: griffri Date: Fri, 12 Jul 2024 14:30:43 +0100 Subject: [PATCH 3/4] Update Parse_InfoJson() test to expect ArgumentException to not be thrown --- src/IIIF/IIIF/ImageApi/ImageRequest.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/IIIF/IIIF/ImageApi/ImageRequest.cs b/src/IIIF/IIIF/ImageApi/ImageRequest.cs index 68d0821..1f3744c 100644 --- a/src/IIIF/IIIF/ImageApi/ImageRequest.cs +++ b/src/IIIF/IIIF/ImageApi/ImageRequest.cs @@ -44,6 +44,7 @@ public static ImageRequest Parse(string path, string prefix, bool validateSegmen var parts = path.Split('/'); request.Identifier = parts[0]; + if (parts.Length == 1 || (parts.Length == 2 && parts[1] == string.Empty)) { // likely the server will want to redirect this From edb2dc0b24008d89bf54acf2b8849bc6bc3a21dd Mon Sep 17 00:00:00 2001 From: griffri Date: Fri, 12 Jul 2024 14:45:21 +0100 Subject: [PATCH 4/4] Implement changes suggested in review Add XML comments to Parse() Ensure that ArgumentException contains correct message in WhenGivenExtraSegments/WhenGivenEmptyParameters tests Give Parse_Validate_Succeeds a more descriptive name Change message returned on validateSegments failure --- src/IIIF/IIIF.Tests/ImageApi/ImageRequestXTests.cs | 14 ++++++++------ src/IIIF/IIIF/ImageApi/ImageRequest.cs | 10 +++++++++- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/IIIF/IIIF.Tests/ImageApi/ImageRequestXTests.cs b/src/IIIF/IIIF.Tests/ImageApi/ImageRequestXTests.cs index a2b1268..9ef29d3 100644 --- a/src/IIIF/IIIF.Tests/ImageApi/ImageRequestXTests.cs +++ b/src/IIIF/IIIF.Tests/ImageApi/ImageRequestXTests.cs @@ -240,10 +240,10 @@ public void Parse_InfoJson() { // Arrange and Act const string prefix = "iiif-img/27/1/"; - var action = () => ImageRequest.Parse($"{prefix}my-asset/info.json", prefix); + var result = ImageRequest.Parse($"{prefix}my-asset/info.json", prefix); // Assert - action.Should().NotThrow(); + result.IsInformationRequest.Should().BeTrue(); } [Fact] @@ -262,14 +262,14 @@ public void Parse_Fails_WhenInfoHasInvalidExtension() [InlineData("/iiif-img/27/1/")] [InlineData("/iiif-img/27/1")] [InlineData("iiif-img/27/1")] - public void Parse_Validate_Succeeds(string prefix) + public void Parse_Validate_HandlesPrefixFormats(string prefix) { // Arrange and Act const string request = $"iiif-img/27/1/my-asset/full/800,/0/default.jpg"; var action = () => ImageRequest.Parse(request, prefix, true); // Assert - action.Should().NotThrow(); + action.Should().NotThrow(); } [Theory] @@ -284,7 +284,8 @@ public void Parse_Validate_Fails_WhenGivenExtraSegments(string path) var action = () => ImageRequest.Parse($"{prefix}{path}", prefix, true); // Assert - action.Should().ThrowExactly(); + action.Should().ThrowExactly() + .WithMessage("Path contains empty or an invalid number of segments"); } [Theory] @@ -301,6 +302,7 @@ public void Parse_Validate_Fails_WhenGivenEmptyParameters(string path) var action = () => ImageRequest.Parse($"{prefix}{path}", prefix, true); // Assert - action.Should().ThrowExactly(); + action.Should().ThrowExactly() + .WithMessage("Path contains empty or an invalid number of segments"); } } \ No newline at end of file diff --git a/src/IIIF/IIIF/ImageApi/ImageRequest.cs b/src/IIIF/IIIF/ImageApi/ImageRequest.cs index 1f3744c..4d54eb5 100644 --- a/src/IIIF/IIIF/ImageApi/ImageRequest.cs +++ b/src/IIIF/IIIF/ImageApi/ImageRequest.cs @@ -26,6 +26,14 @@ public class ImageRequest /// public string ImageRequestPath => OriginalPath.Replace(Identifier, string.Empty); + /// + /// Parses an image request path as a IIIF ImageRequest object + /// + /// A ImageRequest object + /// The image request path + /// The image request prefix + /// If true, throws an ArgumentException if the image request contains empty values, + /// or an invalid number of segments public static ImageRequest Parse(string path, string prefix, bool validateSegments = false) { if (path[0] == '/') path = path[1..]; @@ -60,7 +68,7 @@ public static ImageRequest Parse(string path, string prefix, bool validateSegmen if (validateSegments && (parts.Length != 5 || parts.Any(string.IsNullOrEmpty))) { - throw new ArgumentException("Path contains empty or an invalid number of parameters"); + throw new ArgumentException("Path contains empty or an invalid number of segments"); } request.OriginalPath = path;