Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove async HMAC operations. #327

Merged
merged 1 commit into from
Jun 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}