Replace ImageSharp.Web with NetVips for on-demand image processing#19369
Replace ImageSharp.Web with NetVips for on-demand image processing#19369Skrypt wants to merge 23 commits into
Conversation
Removes SixLabors.ImageSharp.Web (commercial v3 license) and rebuilds the entire resizing pipeline using NetVips (MIT) and custom ASP.NET Core middleware. Key changes: - Add NetVips + NetVips.Native packages; remove all SixLabors.ImageSharp.Web packages - New MediaImageProcessingMiddleware replaces app.UseImageSharp() - New VipsImageProcessingEngine handles resize (6 modes), EXIF auto-orient, format conversion - New IResizedImageCache abstraction with PhysicalFileSystemResizedImageCache (local), AzureBlobResizedImageCache, and AWSS3ResizedImageCache implementations - New MediaCommandParser handles query-string parsing, HMAC token validation, SupportedSizes enforcement - ResizedMediaCacheBackgroundTask updated to use IResizedImageCache.ClearStaleAsync - MediaTokenService rewritten without IImageWebProcessor dependency - Delete BackwardsCompatibleCacheKey, MediaImageSharpConfiguration, MediaResizingFileProvider, TokenCommandProcessor, ImageVersionProcessor and cloud ImageSharp bridge configs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes "ImageSharp" from every class name, constant, feature ID, and config section key that survived the NetVips migration, since this is already a breaking change and a second rename later would be unnecessary churn. Renames: - ImageSharpUrlFormatter → MediaImageUrlFormatter - ImageSharpBlobImageCacheOptions → MediaBlobImageCacheOptions - ImageSharpBlobImageCacheOptionsConfiguration → MediaBlobImageCacheOptionsConfiguration - ImageSharpBlobImageCacheTenantEvents → MediaBlobImageCacheTenantEvents - AwsImageSharpImageCacheOptions → AwsMediaImageCacheOptions - AwsImageSharpImageCacheOptionsConfiguration → AwsMediaImageCacheOptionsConfiguration - ImageSharpS3ImageCacheBucketTenantEvents → AwsS3MediaImageCacheTenantEvents - ImageSharpAzureBlobCacheStartup → MediaAzureImageCacheStartup - ImageSharpAmazonS3CacheStartup → MediaAmazonS3ImageCacheStartup - OrchardCoreConstants.ConfigureOrder.ImageSharpCache → ResizedImageCache - OrchardCoreConstants.ConfigureOrder.AzureImageSharpCache → AzureResizedImageCache - Feature ID OrchardCore.Media.Azure.ImageSharpImageCache → OrchardCore.Media.Azure.ImageCache - Feature ID OrchardCore.Media.AmazonS3.ImageSharpImageCache → OrchardCore.Media.AmazonS3.ImageCache - Config section OrchardCore_Media_Azure_ImageSharp_Cache → OrchardCore_Media_Azure_Image_Cache - Config section OrchardCore_Media_AmazonS3_ImageSharp_Cache → OrchardCore_Media_AmazonS3_Image_Cache Also fixes MediaTokenServiceBenchmark to use the new MediaTokenService constructor (no longer accepts IImageWebProcessor[]). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove ImageSharp.Web references in Azure and S3 module descriptions - Rename feature headings and IDs to match new code (ImageSharpImageCache → ImageCache) - Update config section keys (OrchardCore_Media_*_ImageSharp_Cache → OrchardCore_Media_*_Image_Cache) - Update Security/Permissions.md feature name table entry Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
To be clear, IS didn't move to a commercial license, rather, it's double-licensed now. For OSS like OC, nothing changes (neither consumer projects of OC packages). However, working with the source would become harder. See https://github.com/orgs/OrchardCMS/discussions/19306. |
|
True, but if I'm using Orchard Core in a commercial project then I need to register an ImageSharp licence. Which defeats the purpose of having Orchard Core as an open source project without having licencing issues later on. We can reintroduce ImageSharp later on as a feature. We could support both. But I would rather keep ImageSharp external to this repository unless we make it really evident that there is licencing fees for commercial use. |
|
My understanding is that for consumers of OC, i.e. who use IS only transitively, nothing changes, and thus they don't have to purchase a license either. |
|
I'm not sure 100% on that @Piedone . That's maybe the gripe that Jim has about it. |
- MediaCommandParserTests: 19 unit tests covering supported/unsupported sizes, token validation, focal point and bgcolor stripping, default resize mode, and tokenization-disabled mode - VipsImageProcessingEngineTests: 12 unit tests covering all 6 resize modes (Max/Crop/Stretch/Pad/Min + focal point), all output formats (PNG/WebP/GIF/JPEG), invalid quality fallback, and no-dimensions passthrough; uses NetVips.Image.Black() for synthetic inputs and PNG IHDR parsing for dimension verification - MediaImageTests (Playwright): 5 end-to-end tests using BlogFixture verifying media images render on the blog page, supported-size URLs return HTTP 200, resized images carry security tokens, and unsupported sizes fall through to static file serving - Add NetVips PackageReference to OrchardCore.Tests.csproj - Fix IDE0011 (missing braces) in all new production files Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Assert.Contains and Assert.StartsWith do not accept a string user-message as their third argument in xUnit v3 — the overloads resolve to StringComparison / IAsyncEnumerable parameters, causing CS1503. Replaced with Assert.True + .Contains/.StartsWith on the string directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
thumbnail_image rejects int.MaxValue as the width sentinel, returning "parameter width not set". When width is omitted, derive a proportional width from the source aspect ratio so libvips scales by height instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tests run against an S3-compatible emulator (S3_EMULATOR_URL env var)
and are automatically skipped when no emulator is configured, matching
the existing AwsFileStoreTests pattern.
Covers: Get miss, Set/Get round-trip for JPEG/PNG/WebP, overwrite,
Clear (empty + all entries + base-path isolation), ClearStale (retain
fresh entries, remove with negative maxAge).
Also adds InternalsVisibleTo("OrchardCore.Tests.Integration") to the
AmazonS3 module so the internal AWSS3ResizedImageCache is constructable
from the test project.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
I already did something quite similar with another library. I thought we would discuss the alternative before submitting a PR |
|
Discussion is always open. Why not making this extensible so that we can use any flavor? |
|
Sebastien prefers NetVips for PERF, so you can keep this open. Can I open a new PR to create an abstraction for Imaging, regardless of the implementation? |
Fixes from code review of the ImageSharp.Web -> NetVips migration:
- Dispose the cached stream on cache-hit serving (stream leak).
- Exclude the version ("v") cache-buster from the HMAC token so versioned,
tokenized URLs are processed instead of silently serving the original;
include "v" in the resized cache key for correct cache busting.
- Remove unsupported bmp/tga formats everywhere (libvips has no BMP encoder
or TGA decoder); unsupported formats fall back to JPEG and the obsolete
Format.Bmp/Format.Tga members are removed from the media profile editor.
- Dispose native NetVips images via `using` to avoid leaks on errors.
- Honor autoorient=false via NetVips noRotate.
- Distinguish Pad from BoxPad (BoxPad does not upscale).
- Match the pad background band count to the source image (RGBA/grayscale).
- Use shrink-on-load (Image.ThumbnailBuffer) for the common resize modes.
- Make the resized-image cache lookup deterministic: GetAsync now takes the
resolved file extension instead of probing multiple candidates.
- Make physical-cache writes stampede-safe via temp file + atomic rename.
- Move the cache-key computation into a neutral ResizedImageCacheKey helper
and share format/content-type/extension mapping via MediaResizingConstants.
Adds unit tests for the engine (RGBA/grayscale/BoxPad/autoorient/bmp fallback)
and a token+version regression test, plus end-to-end integration tests that
drive the middleware, engine, cache and token service over TestServer,
including a concurrent first-hit stampede test.
Documents the breaking changes in the 3.0.0 release notes and updates the
Media module reference docs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Renaming the Azure/S3 image cache feature ids disables the feature on upgraded sites, since enabled-feature state is persisted by id and there is no alias mechanism. Add an explicit warning to re-enable the renamed feature after upgrading. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Split the middleware into a non-async InvokeAsync that performs the cheap path and extension early-outs and returns the request delegate directly, delegating the actual image processing to an async helper. This avoids allocating an async state machine for the common case of non-image requests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Hash the key material through a pooled ZString UTF-8 builder and drop the LINQ OrderBy, so no intermediate string, StringBuilder or managed byte[] is allocated. The key remains a stable hashed string because it is used as a durable storage path/blob name, not an in-memory composite key. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the manual S3Mock/S3_EMULATOR_URL setup with Testcontainers that auto-start a LocalStack (S3) and an Azurite (Azure Blob) emulator when the tests run. Add Azure Blob resized image cache coverage mirroring the S3 cache tests. Tests skip gracefully when no Docker daemon is available, and the CI workflow no longer pre-pulls/starts containers manually. Emulator images are pinned to multi-architecture tags so the correct variant is pulled automatically on amd64 and arm64 hosts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Promote the image processing engine contract (IImageProcessingEngine, ImageProcessingCommands, ImageProcessingResult) to the public OrchardCore.Media.Abstractions assembly and move the ResizeMode/Format enums there (type-forwarded for binary compatibility). The middleware now parses requests into a typed, engine-agnostic ImageProcessingCommands. NetVips remains the default engine in OrchardCore.Media. A new opt-in OrchardCore.Media.ImageSharpV3 module provides an ImageSharp 3.1 based engine that replaces the default registration when enabled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Testing summaryPushed an update that introduces a pluggable image-processing engine abstraction with two concrete implementations, then validated it with unit tests and a full browser-driven functional run. What changed (high level)
Unit / integration tests
Functional verification (live site, browser flow)Provisioned a site (Blog recipe, SQLite) and exercised on-demand image processing end-to-end against Default engine — NetVips:
Opt-in engine — ImageSharp v3 (enabled via Admin → Features → "Media ImageSharp Image Processing", which triggers a shell restart and the
Definitive engine-swap proof — identical request
Different bytes for the same input confirm enabling the feature genuinely swapped the active Docs
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Concurrent cold-cache requests for the same resized image previously each ran the full decode/resize transform. Introduce a SingleFlight primitive (adapted from ASP.NET Core Output Caching's work dispatcher) so a burst of requests for the same cache key coalesces into a single transform; waiters serve their own stream read back from the cache the worker populated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 70249b702e88a48cca57857d64ea18b27679955a)
…on upgrade Renaming the Azure and Amazon S3 image cache feature ids would silently disable the cache on existing sites (enabled-feature state is persisted by id). Following the Elasticsearch rename precedent, retain the legacy ids as obsolete features that depend on and auto-enable the renamed features during upgrade via a deferred data migration, so upgraded sites keep the cache enabled. Update the release notes accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 328c9e5905f661ebe09d61631432e8922ba1e9eb)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit a01d61fdb54e88fca2779e1afb1db0aa81bce1f8)
The crop (focal-point/single-axis) and stretch (single-axis) paths decoded the source at full resolution via Image.NewFromBuffer before resampling, which under concurrency used more memory than ImageSharp. They now read the source size from the header and resample through Image.ThumbnailBuffer (shrink-on-load), and the no-resize re-encode path streams with sequential access when not auto-orienting. Measured (6000x4000 -> 300x300 focal crop, 8 concurrent ops): libvips native peak 790 MB -> 107 MB, peak working set 1020 MB -> 316 MB, ~1.8x faster. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 3c3301b696598d4f757663fadcba7f4bc9ada940)
|
I was wrong about the ms-cache folder. It will still be having 2 files cache folders. ms-cache = poor man CDN folder. |
|
To clarify, |
|
But if you are using a CDN then the ms-cache folder will be served from that CDN instead of this poor man CDN cache folder. Meaning that it is served in place at the same place than your website hosting instead of being deployed in different regions. So yes, it is a poor man CDN cache folder. As in local CDN. |
|
This pull request has merge conflicts. Please resolve those before requesting a review. |
|
@Skrypt we should not change that. It was done intentionally. The only thing I remember is that we discussed it with James and we were convinced of this use. If you want to enable a real cdn to put things closer to consumers you can still deploy a real cdn solution (azure cdn). If we find the actual meeting we could write the whole decision in the docs. |
|
To clarify, I am just passing along Claude’s insights regarding the ms-cache, media-cache, and is-cache folders; I don't intend to change their logic. According to Claude, ms-cache acts as a "poor man's CDN" because it serves as Orchard Core's default fallback when no external CDN is configured. Based on Dean’s previous CDN implementation, this is already functional, when a CDN is active, the local ms-cache folder is bypassed. My main point, despite diving into those CDN details, was simply to correct my earlier statement. I was wrong to assume that switching to the NetVips provider would eliminate the need for two separate cache folders. What this PR actually introduces is the NetVips equivalent of the is-cache folder, which will reside inside media-cache instead. Given this structure, should we rename it to nv-cache for better consistency? |
The main merge dropped the SixLabors.ImageSharp PackageVersion entry and kept the now-unreferenced SixLabors.ImageSharp.Web entry, breaking restore with NU1010 for OrchardCore.Media.ImageSharpV3. Restore the base package version (bumped to latest 3.x patch 3.1.12). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The default filesystem resized-image cache already writes to the generic 'media-cache' folder; this updates the stale build-time DefaultItemExcludes entry that still referenced the old ImageSharp-specific 'is-cache' name, and documents the rename and upgrade implications in the 3.0.0 release notes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
This pull request has merge conflicts. Please resolve those before requesting a review. |
Summary
SixLabors.ImageSharp.Web v4 moved to a using a license key, making it unsuitable for an open-source project. This PR removes it entirely and replaces the full image-processing pipeline with NetVips (MIT licensed C# bindings over libvips).
What changed
Core pipeline —
app.UseImageSharp()is replaced byMediaImageProcessingMiddleware, a custom ASP.NET Core middleware that owns the full request lifecycle: parse → validate → cache-check → process → cache-write → serve.Processing engine —
VipsImageProcessingEnginewraps NetVips. Key capabilities:Max,Min,Crop,Pad,BoxPad,Stretch)Image.Autorot()rxyquery parameterCaching abstraction — New
IResizedImageCacheinterface (inOrchardCore.Media.Abstractions) with three implementations:PhysicalFileSystemResizedImageCache— local filesystem (default)AzureBlobResizedImageCache— replaces the oldAzureBlobStorageImageCacheAWSS3ResizedImageCache— replaces the oldAWSS3StorageCacheCache keys use
SHA256(tenantName|path|sorted-commands)for cross-tenant isolation.Token validation —
MediaTokenServicerewritten withoutIImageWebProcessordiscovery; known commands are now a staticHashSetderived fromMediaCommandsconstants.Breaking changes
OrchardCore.Media.Azure.ImageSharpImageCacheOrchardCore.Media.Azure.ImageCacheOrchardCore.Media.AmazonS3.ImageSharpImageCacheOrchardCore.Media.AmazonS3.ImageCacheOrchardCore_Media_Azure_ImageSharp_CacheOrchardCore_Media_Azure_Image_CacheOrchardCore_Media_AmazonS3_ImageSharp_CacheOrchardCore_Media_AmazonS3_Image_CacheAwsImageSharpImageCacheOptionsAwsMediaImageCacheOptionsImageSharpBlobImageCacheOptionsMediaBlobImageCacheOptionsOrchardCoreConstants.ConfigureOrder.ImageSharpCacheResizedImageCacheOrchardCoreConstants.ConfigureOrder.AzureImageSharpCacheAzureResizedImageCacheTest plan
?width=300— confirm the middleware fires and the resized image is servedwwwroot/{tenant}/media-cache/rmode=crop,rmode=pad, etc.)OrchardCore_Media_Azure_Image_CacheconfigOrchardCore_Media_AmazonS3_Image_Cacheconfigdotnet test— existingMediaTokenTestspassNotes
NetVips.Nativeships the libvips binary as a NuGet package — no system dependency required on Windows or LinuxBackwardsCompatibleCacheKey,MediaImageSharpConfiguration,MediaResizingFileProvider,TokenCommandProcessor, andImageVersionProcessorfiles are deleted; their responsibilities are absorbed into the new middleware and parserVipsImageProcessingEngineandMediaCommandParserare a follow-upFeature Parity: NetVips vs ImageSharp.Web
Yes — full parity with everything OrchardCore ever exposed.
OrchardCore defined exactly 10 commands via
MediaCommands. All 10 are implemented:widthheightrmoderxyformatbgcolorqualityautoorientAutorot()reads EXIFtokenvWhere NetVips is ahead
Autorot()is unconditional and correct; ImageSharp's implementation had edge cases.What ImageSharp.Web could do that we don't (but OrchardCore never exposed)
rsampler— resampler selection (Lanczos3 is hardcoded, which is the best default anyway)ranchor— anchor positioning (superseded byrxyfocal point)compand— gamma correction during resize (rarely used, no OC UI for it)Any Razor template, tag helper, or media profile that worked before will work identically now.