From f65a29535f0fd2f14556f1104d1f9d5679b6e5a2 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 26 May 2022 22:24:47 +1000 Subject: [PATCH 1/2] Optimize key removal, prevent duplicates. --- .../PresetOnlyQueryCollectionRequestParser.cs | 6 ++-- .../Commands/QueryCollectionRequestParser.cs | 3 +- .../Middleware/ImageSharpMiddleware.cs | 25 +++-------------- .../Commands/CommandCollectionTests.cs | 28 +++++++++++++++++++ .../Commands/QueryCollectionUriParserTests.cs | 4 ++- 5 files changed, 41 insertions(+), 25 deletions(-) diff --git a/src/ImageSharp.Web/Commands/PresetOnlyQueryCollectionRequestParser.cs b/src/ImageSharp.Web/Commands/PresetOnlyQueryCollectionRequestParser.cs index dfebe3bc..7f6c5f45 100644 --- a/src/ImageSharp.Web/Commands/PresetOnlyQueryCollectionRequestParser.cs +++ b/src/ImageSharp.Web/Commands/PresetOnlyQueryCollectionRequestParser.cs @@ -39,7 +39,8 @@ public CommandCollection ParseRequestCommands(HttpContext context) return new(); } - string requestedPreset = context.Request.Query[QueryKey][0]; + StringValues query = context.Request.Query[QueryKey]; + string requestedPreset = query[query.Count - 1]; if (this.presets.TryGetValue(requestedPreset, out CommandCollection collection)) { return collection; @@ -63,7 +64,8 @@ private static CommandCollection ParsePreset(string unparsedPresetValue) CommandCollection transformed = new(); foreach (KeyValuePair pair in parsed) { - transformed.Add(new(pair.Key, pair.Value.ToString())); + // Use the indexer for both set and query. This replaces any previously parsed values. + transformed[pair.Key] = pair.Value[pair.Value.Count - 1]; } return transformed; diff --git a/src/ImageSharp.Web/Commands/QueryCollectionRequestParser.cs b/src/ImageSharp.Web/Commands/QueryCollectionRequestParser.cs index 57264cd5..cf31b149 100644 --- a/src/ImageSharp.Web/Commands/QueryCollectionRequestParser.cs +++ b/src/ImageSharp.Web/Commands/QueryCollectionRequestParser.cs @@ -28,7 +28,8 @@ public CommandCollection ParseRequestCommands(HttpContext context) CommandCollection transformed = new(); foreach (KeyValuePair pair in parsed) { - transformed.Add(new(pair.Key, pair.Value.ToString())); + // Use the indexer for both set and query. This replaces any previously parsed values. + transformed[pair.Key] = pair.Value[pair.Value.Count - 1]; } return transformed; diff --git a/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs b/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs index 4f626267..f22d34f7 100644 --- a/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs +++ b/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs @@ -212,17 +212,13 @@ private async Task Invoke(HttpContext httpContext, bool retry) if (commands.Count > 0) { // Strip out any unknown commands, if needed. - int index = 0; - foreach (string command in commands.Keys) + var keys = new List(commands.Keys); + for (int i = keys.Count - 1; i >= 0; i--) { - if (!this.knownCommands.Contains(command)) + if (!this.knownCommands.Contains(keys[i])) { - // Need to actually remove, allocates new list to allow modifications - this.StripUnknownCommands(commands, index); - break; + commands.RemoveAt(i); } - - index++; } } @@ -307,19 +303,6 @@ private static void SetBadRequest(HttpContext httpContext) httpContext.Response.StatusCode = StatusCodes.Status400BadRequest; } - private void StripUnknownCommands(CommandCollection commands, int startAtIndex) - { - var keys = new List(commands.Keys); - for (int index = startAtIndex; index < keys.Count; index++) - { - string command = keys[index]; - if (!this.knownCommands.Contains(command)) - { - commands.Remove(command); - } - } - } - private async Task ProcessRequestAsync( HttpContext httpContext, IImageResolver sourceImageResolver, diff --git a/tests/ImageSharp.Web.Tests/Commands/CommandCollectionTests.cs b/tests/ImageSharp.Web.Tests/Commands/CommandCollectionTests.cs index 88d3f52a..31f40543 100644 --- a/tests/ImageSharp.Web.Tests/Commands/CommandCollectionTests.cs +++ b/tests/ImageSharp.Web.Tests/Commands/CommandCollectionTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. +using System; using System.Collections.Generic; using SixLabors.ImageSharp.Web.Commands; using Xunit; @@ -17,6 +18,15 @@ public void CanAddCommands() Assert.Single(collection); } + [Fact] + public void CannotAddDuplicateCommands() + { + CommandCollection collection = new(); + collection.Add(new("a", "b")); + Assert.Throws(() => collection.Add(new("a", "b"))); + Assert.Single(collection); + } + [Fact] public void CanReadCommands() { @@ -48,6 +58,15 @@ public void CanInsertCommands() Assert.Equal(0, collection.IndexOf(kv2)); } + [Fact] + public void CannotInsertDuplicateCommands() + { + CommandCollection collection = new(); + collection.Add(new("a", "b")); + Assert.Throws(() => collection.Insert(0, new("a", "b"))); + Assert.Single(collection); + } + [Fact] public void CanInsertCommandsViaKey() { @@ -68,6 +87,15 @@ public void CanInsertCommandsViaKey() Assert.Equal(0, collection.IndexOf(kv2)); } + [Fact] + public void CannotInsertDuplicateCommandsViaKey() + { + CommandCollection collection = new(); + collection.Add(new("a", "b")); + Assert.Throws(() => collection.Insert(0, "a", "b")); + Assert.Single(collection); + } + [Fact] public void CanSetCommandsViaIndex() { diff --git a/tests/ImageSharp.Web.Tests/Commands/QueryCollectionUriParserTests.cs b/tests/ImageSharp.Web.Tests/Commands/QueryCollectionUriParserTests.cs index e083473a..009b7931 100644 --- a/tests/ImageSharp.Web.Tests/Commands/QueryCollectionUriParserTests.cs +++ b/tests/ImageSharp.Web.Tests/Commands/QueryCollectionUriParserTests.cs @@ -28,7 +28,9 @@ private static HttpContext CreateHttpContext() { var httpContext = new DefaultHttpContext(); httpContext.Request.Path = "/testwebsite.com/image-12345.jpeg"; - httpContext.Request.QueryString = new QueryString("?width=400&height=200"); + + // Duplicate height param to test replacements + httpContext.Request.QueryString = new QueryString("?width=400&height=100&height=200"); return httpContext; } } From f8fdde6bee1a6672b817f59ea62d0add07cf8320 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 26 May 2022 22:40:00 +1000 Subject: [PATCH 2/2] Bump ImageSharp ref --- src/ImageSharp.Web/ImageSharp.Web.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp.Web/ImageSharp.Web.csproj b/src/ImageSharp.Web/ImageSharp.Web.csproj index 3a6c5d3d..72230135 100644 --- a/src/ImageSharp.Web/ImageSharp.Web.csproj +++ b/src/ImageSharp.Web/ImageSharp.Web.csproj @@ -46,7 +46,7 @@ - +