From 5fca9ced3e8010e36593c41cedfb51815e051a2b Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 23 Mar 2022 00:18:34 +1100 Subject: [PATCH 1/2] Allow runtime physical deletion of cached images --- .../Middleware/ImageSharpMiddleware.cs | 67 +++++++++++++------ 1 file changed, 46 insertions(+), 21 deletions(-) diff --git a/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs b/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs index 7fac00bc..73880283 100644 --- a/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs +++ b/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs @@ -184,12 +184,14 @@ public ImageSharpMiddleware( /// /// Performs operations upon the current request. /// - /// The current HTTP request context. + /// The current HTTP request context. /// The . - public async Task Invoke(HttpContext context) + public Task Invoke(HttpContext httpContext) => this.Invoke(httpContext, false); + + private async Task Invoke(HttpContext httpContext, bool retry) { // We expect to get concrete collection type which removes virtual dispatch concerns and enumerator allocations - CommandCollection commands = this.requestParser.ParseRequestCommands(context); + CommandCollection commands = this.requestParser.ParseRequestCommands(httpContext); if (commands.Count > 0) { @@ -209,13 +211,13 @@ public async Task Invoke(HttpContext context) } await this.options.OnParseCommandsAsync.Invoke( - new ImageCommandContext(context, commands, this.commandParser, this.parserCulture)); + new ImageCommandContext(httpContext, commands, this.commandParser, this.parserCulture)); // Get the correct service for the request IImageProvider provider = null; foreach (IImageProvider resolver in this.providers) { - if (resolver.Match(context)) + if (resolver.Match(httpContext)) { provider = resolver; break; @@ -223,30 +225,31 @@ await this.options.OnParseCommandsAsync.Invoke( } if ((commands.Count == 0 && provider?.ProcessingBehavior != ProcessingBehavior.All) - || provider?.IsValidRequest(context) != true) + || provider?.IsValidRequest(httpContext) != true) { // Nothing to do. call the next delegate/middleware in the pipeline - await this.next(context); + await this.next(httpContext); return; } - IImageResolver sourceImageResolver = await provider.GetAsync(context); + IImageResolver sourceImageResolver = await provider.GetAsync(httpContext); if (sourceImageResolver is null) { // Log the error but let the pipeline handle the 404 // by calling the next delegate/middleware in the pipeline. - var imageContext = new ImageContext(context, this.options); + var imageContext = new ImageContext(httpContext, this.options); this.logger.LogImageResolveFailed(imageContext.GetDisplayUrl()); - await this.next(context); + await this.next(httpContext); return; } await this.ProcessRequestAsync( - context, + httpContext, sourceImageResolver, - new ImageContext(context, this.options), - commands); + new ImageContext(httpContext, this.options), + commands, + retry); } private void StripUnknownCommands(CommandCollection commands, int startAtIndex) @@ -263,14 +266,15 @@ private void StripUnknownCommands(CommandCollection commands, int startAtIndex) } private async Task ProcessRequestAsync( - HttpContext context, + HttpContext httpContext, IImageResolver sourceImageResolver, ImageContext imageContext, - CommandCollection commands) + CommandCollection commands, + bool retry) { // Create a hashed cache key string key = this.cacheHash.Create( - this.cacheKey.Create(context, commands), + this.cacheKey.Create(httpContext, commands), this.options.CacheHashLength); // Check the cache, if present, not out of date and not requiring an update @@ -283,7 +287,7 @@ private async Task ProcessRequestAsync( if (!readResult.IsNewOrUpdated) { - await this.SendResponseAsync(imageContext, key, readResult.CacheImageMetadata, readResult.Resolver, null); + await this.SendResponseAsync(httpContext, imageContext, key, readResult.CacheImageMetadata, readResult.Resolver, null, retry); return; } @@ -379,7 +383,7 @@ private async Task ProcessRequestAsync( outStream.Position = 0; string contentType = format.DefaultMimeType; string extension = this.formatUtilities.GetExtensionFromContentType(contentType); - await this.options.OnProcessedAsync.Invoke(new ImageProcessingContext(context, outStream, commands, contentType, extension)); + await this.options.OnProcessedAsync.Invoke(new ImageProcessingContext(httpContext, outStream, commands, contentType, extension)); outStream.Position = 0; cachedImageMetadata = new ImageCacheMetadata( @@ -409,7 +413,7 @@ private async Task ProcessRequestAsync( } } - await this.SendResponseAsync(imageContext, key, readResult.CacheImageMetadata, readResult.Resolver, outStream); + await this.SendResponseAsync(httpContext, imageContext, key, readResult.CacheImageMetadata, readResult.Resolver, outStream, retry); } finally { @@ -490,11 +494,13 @@ private async Task IsNewOrUpdatedAsync( } private async Task SendResponseAsync( + HttpContext httpContext, ImageContext imageContext, string key, ImageCacheMetadata metadata, IImageCacheResolver cacheResolver, - Stream stream) + Stream stream, + bool retry) { imageContext.ComprehendRequestHeaders(metadata.CacheLastWriteTimeUtc, metadata.ContentLength); @@ -518,10 +524,29 @@ private async Task SendResponseAsync( } else { - using (Stream cacheStream = await cacheResolver.OpenReadAsync()) + try { + using Stream cacheStream = await cacheResolver.OpenReadAsync(); await imageContext.SendAsync(cacheStream, metadata); } + catch (Exception ex) + { + if (!retry) + { + // The image has failed to be returned from the cache. + // This can happen if the cached image has been physically deleted but the item is still in the LRU cache. + // We'll retry running the request again in it's entirety. This ensures any changes to the source are tracked also. + CacheResolverLru.TryRemove(key); + await this.Invoke(httpContext); + return; + } + + // We've already tried to run this request before. + // Log the error internally then rethrow. + // We don't call next here, the pipeline will automatically handle it + this.logger.LogImageProcessingFailed(imageContext.GetDisplayUrl(), ex); + throw; + } } return; From 8bfe51fc5b112a3d2aaa375bf6eb96ffca71bd67 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 23 Mar 2022 00:57:57 +1100 Subject: [PATCH 2/2] No need to create a new context. --- src/ImageSharp.Web/Middleware/ImageContext.cs | 7 +++++-- src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp.Web/Middleware/ImageContext.cs b/src/ImageSharp.Web/Middleware/ImageContext.cs index ea04f1ae..69224d06 100644 --- a/src/ImageSharp.Web/Middleware/ImageContext.cs +++ b/src/ImageSharp.Web/Middleware/ImageContext.cs @@ -86,9 +86,12 @@ internal enum PreconditionState } /// - /// Returns the current HTTP request display url. + /// Returns the current HTTP image request display url. /// - /// The. + /// + /// The combined components of the image request URL in a fully un-escaped form (except + /// for the QueryString) suitable only for display. + /// public string GetDisplayUrl() => this.request.GetDisplayUrl(); /// diff --git a/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs b/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs index 73880283..378fbdbd 100644 --- a/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs +++ b/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs @@ -9,6 +9,7 @@ using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Extensions; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Microsoft.IO; @@ -238,8 +239,7 @@ await this.options.OnParseCommandsAsync.Invoke( { // Log the error but let the pipeline handle the 404 // by calling the next delegate/middleware in the pipeline. - var imageContext = new ImageContext(httpContext, this.options); - this.logger.LogImageResolveFailed(imageContext.GetDisplayUrl()); + this.logger.LogImageResolveFailed(httpContext.Request.GetDisplayUrl()); await this.next(httpContext); return; }