Skip to content

Commit

Permalink
Remove async HMAC operations.
Browse files Browse the repository at this point in the history
  • Loading branch information
JimBobSquarePants committed Jun 10, 2023
1 parent 67e82ab commit 58ffdf5
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 88 deletions.
4 changes: 2 additions & 2 deletions src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,9 @@ private async Task Invoke(HttpContext httpContext, bool retry)
//
// As a rule all image requests should contain valid commands only.
// Key generation uses string.Create under the hood with very low allocation so should be good enough as a cache key.
hmac = await HMACTokenLru.GetOrAddAsync(
hmac = HMACTokenLru.GetOrAdd(
httpContext.Request.GetEncodedUrl(),
_ => this.authorizationUtilities.ComputeHMACAsync(imageCommandContext));
_ => this.authorizationUtilities.ComputeHMAC(imageCommandContext));
}

await this.options.OnParseCommandsAsync.Invoke(imageCommandContext);
Expand Down
12 changes: 6 additions & 6 deletions src/ImageSharp.Web/Middleware/ImageSharpMiddlewareOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ namespace SixLabors.ImageSharp.Web.Middleware;
/// </summary>
public class ImageSharpMiddlewareOptions
{
private Func<ImageCommandContext, byte[], Task<string>> onComputeHMACAsync = (context, secret) =>
private Func<ImageCommandContext, byte[], string> onComputeHMAC = (context, secret) =>
{
string uri = CaseHandlingUriBuilder.BuildRelative(
CaseHandlingUriBuilder.CaseHandling.LowerInvariant,
context.Context.Request.PathBase,
context.Context.Request.Path,
QueryString.Create(context.Commands));
return Task.FromResult(HMACUtilities.ComputeHMACSHA256(uri, secret));
return HMACUtilities.ComputeHMACSHA256(uri, secret);
};

private Func<ImageCommandContext, Task> onParseCommandsAsync = _ => Task.CompletedTask;
Expand Down Expand Up @@ -88,14 +88,14 @@ public class ImageSharpMiddlewareOptions
/// Defaults to <see cref="HMACUtilities.ComputeHMACSHA256(string, byte[])"/> using an invariant lowercase relative Uri
/// generated using <see cref="CaseHandlingUriBuilder.BuildRelative(CaseHandlingUriBuilder.CaseHandling, PathString, PathString, QueryString)"/>.
/// </summary>
public Func<ImageCommandContext, byte[], Task<string>> OnComputeHMACAsync
public Func<ImageCommandContext, byte[], string> OnComputeHMAC
{
get => this.onComputeHMACAsync;
get => this.onComputeHMAC;

set
{
Guard.NotNull(value, nameof(this.onComputeHMACAsync));
this.onComputeHMACAsync = value;
Guard.NotNull(value, nameof(this.onComputeHMAC));
this.onComputeHMAC = value;
}
}

Expand Down
64 changes: 3 additions & 61 deletions src/ImageSharp.Web/RequestAuthorizationUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,6 @@ public void StripUnknownCommands(CommandCollection commands)
public string? ComputeHMAC(string uri, CommandHandling handling)
=> this.ComputeHMAC(new Uri(uri, UriKind.RelativeOrAbsolute), handling);

/// <summary>
/// Compute a Hash-based Message Authentication Code (HMAC) for request authentication.
/// </summary>
/// <param name="uri">The uri to compute the code from.</param>
/// <param name="handling">The command collection handling.</param>
/// <returns>The computed HMAC.</returns>
public Task<string?> ComputeHMACAsync(string uri, CommandHandling handling)
=> this.ComputeHMACAsync(new Uri(uri, UriKind.RelativeOrAbsolute), handling);

/// <summary>
/// Compute a Hash-based Message Authentication Code (HMAC) for request authentication.
/// </summary>
Expand All @@ -124,23 +115,6 @@ public void StripUnknownCommands(CommandCollection commands)
return this.ComputeHMAC(host, path, queryString, handling);
}

/// <summary>
/// Compute a Hash-based Message Authentication Code (HMAC) for request authentication.
/// </summary>
/// <param name="uri">The uri to compute the code from.</param>
/// <param name="handling">The command collection handling.</param>
/// <returns>The computed HMAC.</returns>
public Task<string?> ComputeHMACAsync(Uri uri, CommandHandling handling)
{
ToComponents(
uri,
out HostString host,
out PathString path,
out QueryString queryString);

return this.ComputeHMACAsync(host, path, queryString, handling);
}

/// <summary>
/// Compute a Hash-based Message Authentication Code (HMAC) for request authentication.
/// </summary>
Expand All @@ -152,17 +126,6 @@ public void StripUnknownCommands(CommandCollection commands)
public string? ComputeHMAC(HostString host, PathString path, QueryString queryString, CommandHandling handling)
=> this.ComputeHMAC(host, path, queryString, new(QueryHelpers.ParseQuery(queryString.Value)), handling);

/// <summary>
/// Compute a Hash-based Message Authentication Code (HMAC) for request authentication.
/// </summary>
/// <param name="host">The host header.</param>
/// <param name="path">The path or pathbase.</param>
/// <param name="queryString">The querystring.</param>
/// <param name="handling">The command collection handling.</param>
/// <returns>The computed HMAC.</returns>
public Task<string?> ComputeHMACAsync(HostString host, PathString path, QueryString queryString, CommandHandling handling)
=> this.ComputeHMACAsync(host, path, queryString, new(QueryHelpers.ParseQuery(queryString.Value)), handling);

/// <summary>
/// Compute a Hash-based Message Authentication Code (HMAC) for request authentication.
/// </summary>
Expand All @@ -175,34 +138,13 @@ public void StripUnknownCommands(CommandCollection commands)
public string? ComputeHMAC(HostString host, PathString path, QueryString queryString, QueryCollection query, CommandHandling handling)
=> this.ComputeHMAC(this.ToHttpContext(host, path, queryString, query), handling);

/// <summary>
/// Compute a Hash-based Message Authentication Code (HMAC) for request authentication.
/// </summary>
/// <param name="host">The host header.</param>
/// <param name="path">The path or pathbase.</param>
/// <param name="queryString">The querystring.</param>
/// <param name="query">The query collection.</param>
/// <param name="handling">The command collection handling.</param>
/// <returns>The computed HMAC.</returns>
public Task<string?> ComputeHMACAsync(HostString host, PathString path, QueryString queryString, QueryCollection query, CommandHandling handling)
=> this.ComputeHMACAsync(this.ToHttpContext(host, path, queryString, query), handling);

/// <summary>
/// Compute a Hash-based Message Authentication Code (HMAC) for request authentication.
/// </summary>
/// <param name="context">The request HTTP context.</param>
/// <param name="handling">The command collection handling.</param>
/// <returns>The computed HMAC.</returns>
public string? ComputeHMAC(HttpContext context, CommandHandling handling)
=> AsyncHelper.RunSync(() => this.ComputeHMACAsync(context, handling));

/// <summary>
/// Compute a Hash-based Message Authentication Code (HMAC) for request authentication.
/// </summary>
/// <param name="context">The request HTTP context.</param>
/// <param name="handling">The command collection handling.</param>
/// <returns>The computed HMAC.</returns>
public async Task<string?> ComputeHMACAsync(HttpContext context, CommandHandling handling)
{
byte[] secret = this.options.HMACSecretKey;
if (secret is null || secret.Length == 0)
Expand All @@ -222,7 +164,7 @@ public void StripUnknownCommands(CommandCollection commands)
}

ImageCommandContext imageCommandContext = new(context, commands, this.commandParser, this.parserCulture);
return await this.options.OnComputeHMACAsync(imageCommandContext, secret);
return this.options.OnComputeHMAC(imageCommandContext, secret);
}

/// <summary>
Expand All @@ -234,14 +176,14 @@ public void StripUnknownCommands(CommandCollection commands)
/// </remarks>
/// <param name="context">Contains information about the current image request and parsed commands.</param>
/// <returns>The computed HMAC.</returns>
internal async Task<string?> ComputeHMACAsync(ImageCommandContext context)
internal string? ComputeHMAC(ImageCommandContext context)
{
if (context.Commands.Count == 0)
{
return null;
}

return await this.options.OnComputeHMACAsync(context, this.options.HMACSecretKey);
return this.options.OnComputeHMAC(context, this.options.HMACSecretKey);
}

private static void ToComponents(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

using System.Globalization;

namespace SixLabors.ImageSharp.Web;
namespace SixLabors.ImageSharp.Web.Tests.TestUtilities;

/// <summary>
/// <see href="https://github.com/aspnet/AspNetIdentity/blob/b7826741279450c58b230ece98bd04b4815beabf/src/Microsoft.AspNet.Identity.Core/AsyncHelper.cs"/>
Expand All @@ -20,7 +20,7 @@ private static readonly TaskFactory TaskFactory
/// <summary>
/// Executes an async <see cref="Task"/> method synchronously.
/// </summary>
/// <param name="task">The task to excecute.</param>
/// <param name="task">The task to execute.</param>
public static void RunSync(Func<Task> task)
{
CultureInfo cultureUi = CultureInfo.CurrentUICulture;
Expand All @@ -38,7 +38,7 @@ public static void RunSync(Func<Task> task)
/// a <paramref name="task"/> return type synchronously.
/// </summary>
/// <typeparam name="TResult">The type of result to return.</typeparam>
/// <param name="task">The task to excecute.</param>
/// <param name="task">The task to execute.</param>
/// <returns>The <typeparamref name="TResult"/>.</returns>
public static TResult RunSync<TResult>(Func<Task<TResult>> task)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ public abstract class AuthenticatedServerTestBase<TFixture> : ServerTestBase<TFi
where TFixture : AuthenticatedTestServerFixture
{
private readonly RequestAuthorizationUtilities authorizationUtilities;
private readonly string relativeImageSouce;
private readonly string relativeImageSource;

protected AuthenticatedServerTestBase(TFixture fixture, ITestOutputHelper outputHelper, string imageSource)
: base(fixture, outputHelper, imageSource)
{
this.authorizationUtilities =
this.Fixture.Services.GetRequiredService<RequestAuthorizationUtilities>();

this.relativeImageSouce = this.ImageSource.Replace("http://localhost", string.Empty);
this.relativeImageSource = this.ImageSource.Replace("http://localhost", string.Empty);
}

[Fact]
Expand All @@ -40,19 +40,17 @@ public async Task CanRejectUnauthorizedRequestAsync()
Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
}

protected override async Task<string> AugmentCommandAsync(string command)
protected override string AugmentCommand(string command)
{
string uri = this.relativeImageSouce + command;
string token = await this.GetTokenAsync(uri);
string uri = this.relativeImageSource + command;
string token = this.GetToken(uri);
return command + "&" + RequestAuthorizationUtilities.TokenCommand + "=" + token;
}

private async Task<string> GetTokenAsync(string uri)
private string GetToken(string uri)
{
string tokenSync = this.authorizationUtilities.ComputeHMAC(uri, CommandHandling.Sanitize);
string tokenAsync = await this.authorizationUtilities.ComputeHMACAsync(uri, CommandHandling.Sanitize);

Assert.Equal(tokenSync, tokenAsync);
Assert.NotNull(tokenSync);
return tokenSync;
}
}
14 changes: 7 additions & 7 deletions tests/ImageSharp.Web.Tests/TestUtilities/ServerTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public async Task CanProcessAndResolveImageAsync()
Assert.True(Configuration.Default.ImageFormatsManager.TryFindFormatByFileExtension(ext, out IImageFormat format));

// First response
HttpResponseMessage response = await this.HttpClient.GetAsync(url + await this.AugmentCommandAsync(this.Fixture.Commands[0]));
HttpResponseMessage response = await this.HttpClient.GetAsync(url + this.AugmentCommand(this.Fixture.Commands[0]));

Assert.NotNull(response);
Assert.True(response.IsSuccessStatusCode);
Expand All @@ -60,7 +60,7 @@ public async Task CanProcessAndResolveImageAsync()
response.Dispose();

// Cached Response
response = await this.HttpClient.GetAsync(url + await this.AugmentCommandAsync(this.Fixture.Commands[0]));
response = await this.HttpClient.GetAsync(url + this.AugmentCommand(this.Fixture.Commands[0]));

Assert.NotNull(response);
Assert.True(response.IsSuccessStatusCode);
Expand All @@ -77,7 +77,7 @@ public async Task CanProcessAndResolveImageAsync()
// 304 response
HttpRequestMessage request = new()
{
RequestUri = new Uri(url + await this.AugmentCommandAsync(this.Fixture.Commands[0])),
RequestUri = new Uri(url + this.AugmentCommand(this.Fixture.Commands[0])),
Method = HttpMethod.Get,
};

Expand All @@ -100,7 +100,7 @@ public async Task CanProcessAndResolveImageAsync()
// 412 response
request = new HttpRequestMessage
{
RequestUri = new Uri(url + await this.AugmentCommandAsync(this.Fixture.Commands[0])),
RequestUri = new Uri(url + this.AugmentCommand(this.Fixture.Commands[0])),
Method = HttpMethod.Get,
};

Expand All @@ -119,8 +119,8 @@ public async Task CanProcessAndResolveImageAsync()
public async Task CanProcessMultipleIdenticalQueriesAsync()
{
string url = this.ImageSource;
string command1 = await this.AugmentCommandAsync(this.Fixture.Commands[0]);
string command2 = await this.AugmentCommandAsync(this.Fixture.Commands[1]);
string command1 = this.AugmentCommand(this.Fixture.Commands[0]);
string command2 = this.AugmentCommand(this.Fixture.Commands[1]);

Task[] tasks = Enumerable.Range(0, 100).Select(i => Task.Run(async () =>
{
Expand All @@ -136,5 +136,5 @@ public async Task CanProcessMultipleIdenticalQueriesAsync()
Assert.True(all.IsCompletedSuccessfully);
}

protected virtual Task<string> AugmentCommandAsync(string command) => Task.FromResult(command);
protected virtual string AugmentCommand(string command) => command;
}

0 comments on commit 58ffdf5

Please sign in to comment.