Repository navigation
Conversation
…TFM (CVE fixes incl. CVE-2026-106114), migrate v4 API surface, and wire Six Labors license into CI build
…TFM (CVE fixes incl. CVE-2026-106114), migrate v4 API surface, and wire Six Labors license into CI build
mee-ironsoftware
left a comment
mee-ironsoftware
left a comment
There was a problem hiding this comment.
🤖 Automated
/scrutinizereview — not yet read by@mee-ironsoftware.
Automated review — verdict: fix-then-ship
Blockers: 0 · Major: 3 · Nits: 1 — 4 inline, 0 in this body.
This is deliberately a comment only: it never approves and never requests changes. The verdict above is advisory; @mee-ironsoftware will follow up.
Adds a net80 TFM that builds against ImageSharp 4.1.2 / Drawing 3.1.2 to pick up CVE fixes; the conditional-TFM approach is the right shape (v4 is net8-only), but for net8 consumers this is a breaking dependency bump that downstream repos aren't ready for, and the new PNG encoder mapping doesn't preserve every depth it claims to.
Reviewed head SHA 0e7d703a83ebf5f3c21a616177557f75a402c268. Re-request review on this PR to get a fresh pass.
| <dependency id="SixLabors.ImageSharp.Drawing" version="2.1.7" /> | ||
| <dependency id="BitMiracle.LibTiff.NET" version="2.4.649" /> | ||
| </group> | ||
| <group targetFramework="net80"> |
There was a problem hiding this comment.
Major: net80 group forces ImageSharp 4 on every net8+ consumer; downstream pins 3.x
Finding: the new net80 dependency group makes every .NET 8+ consumer resolve SixLabors.ImageSharp >= 4.1.2. The PR says "Other targets remain unchanged", but for net8+ consumers this is a breaking change.
Why it matters: a consumer that references ImageSharp 3.x directly gets NU1605 (package downgrade, an error by default) once it picks up this Drawing version through IronPdf/IronOcr. If it upgrades to 4.x to fix that, its own direct reference then runs Six Labors' SixLabors_ValidateLicense target (build/SixLabors.ImageSharp.targets in 4.1.2, BeforeTargets=CoreCompile, a hard error outside Debug* configurations), and it also has to absorb the v4 API breaks this PR works around (Color.FromRgba, BmpBitsPerPixel.Pixel32, ctx.Fill, ...).
Evidence: iron-software/Universal.IronPdf front-end/csharp/tests/IronSoftware.IronPdf.IntegrationTests/IronSoftware.IronPdf.IntegrationTests.csproj targets net10.0;net8.0 and pins SixLabors.ImageSharp 3.1.11. Transitive consumers are fine: a nuspec <dependency> defaults to exclude="Build,Analyzers", so the license target doesn't flow through Drawing.
Suggested change: coordinate the bump with the downstream repos that pin ImageSharp on net8+, and add a release-notes "Breaking" line saying net8+ consumers now get ImageSharp 4 and, if they reference it directly, need a Six Labors license file or key at build time.
| BitDepth = bpp >= 48 ? PngBitDepth.Bit16 : PngBitDepth.Bit8, | ||
| ColorType = bpp switch | ||
| { | ||
| 8 => PngColorType.Grayscale, | ||
| 24 => PngColorType.Rgb, | ||
| 48 => PngColorType.Rgb, | ||
| _ => PngColorType.RgbWithAlpha | ||
| } |
There was a problem hiding this comment.
Major: GetDefaultPngEncoder bpp switch drops 16-bit gray / high-depth alpha
Finding: the net8 PNG encoder works out its settings from InMemoryBitsPerPixel (GetFirstInternalImage().PixelType.BitsPerPixel, line 1098) with a switch on bpp alone. Only 8/24/48 get a specific color type, and only bpp >= 48 gets 16-bit depth.
Why it matters: the comment says this keeps the PNG round-trip depth that ImageSharp 3 gave on net6, but several decoded pixel types fall through to the wrong branch:
L16(16-bit gray, 16bpp) → 8-bitRgbWithAlpha: the gray channel and 8 bits of precision are lost.La32(32bpp) → 8-bitRgbWithAlpha: precision is lost.La16(16bpp) →RgbWithAlphainstead ofGrayscaleWithAlpha.A8(8bpp) →Grayscale: the alpha is dropped.
So on these inputs net8 and net6 now give different output.
Evidence: trace ExportStream/SaveAs → GetDefaultImageExportEncoder (line 3739) → GetDefaultPngEncoder() → bpp switch. Only the Rgba64 case (Resize_ShouldPreserveDepthForRepresentableFormats) is tested on net80.
Suggested change: derive ColorType/BitDepth from the pixel type's component info (gray vs color, has alpha, bits per component) or from the source PngMetadata, not from total bpp. Add a net80 round-trip test for an L16 PNG.
There was a problem hiding this comment.
Fixed. Instead of the bpp switch, the encoder now follows ImageSharp 3’s logic: it keeps the source PNG’s color type and bit depth, and otherwise uses ImageSharp 3’s exact per-pixel-type table. I didn’t derive it from component info because that differs from net6 for 11 of 29 pixel types. Also fixed: the old code overrode the source PNG’s settings (palette/1-bit). Added L16/La16/La32/A8 and source-PNG round-trip tests. They fail on the old code and pass on net48 and net80.
| --no-restore | ||
| --verbosity normal | ||
| --property:AssemblyVersion=$(finalAssemblyVersion) | ||
| --property:SixLaborsLicenseFile=$(sixLaborsLicense.secureFilePath) |
There was a problem hiding this comment.
Major: License validation is only enforced in Release; PR CI can't prove it
Finding: PR validation builds Debug (CI/pr-validation.yml:39). In that configuration the Six Labors ValidateLicenseTask runs with ContinueOnError="$(Configuration.StartsWith('Debug'))", i.e. it only warns. Only the release pipeline builds Release (CI/azure-pipelines-build.yml:31), and there an invalid, expired or wrong-product license fails the build.
Why it matters: a green PR check doesn't show that sixlabors.lic is a valid license for ImageSharp 4 and ImageSharp.Drawing 3. The first place it can fail is the release build. Devs running local Release builds also need a sixlabors.lic somewhere under the tree (the target globs **/sixlabors.lic), and .gitignore doesn't exclude it, so it could get committed by accident.
Suggested change: run one Release build of this branch (or check the Debug build log for a license warning) before merging. Add sixlabors.lic to .gitignore.
| Bug Fixes | ||
| - Fixed an issue where derived AnyBitmap operations incorrectly reported a decoded 32bpp color depth instead of the original source pixel depth | ||
| </releaseNotes> | ||
| - Added a .NET 8 target using ImageSharp 4.1.2 and Drawing 3.1.2, enabling security fixes including CVE-2026-106114. Other targets remain unchanged. </releaseNotes> |
There was a problem hiding this comment.
Nit: Release notes: closing tag glued to the line; no breaking/security call-out
</releaseNotes> sits on the same line as the bullet, after a tab. The note also doesn't flag the net8+ dependency change as breaking (see the nuspec dependency finding), even though the PR template ticks "Breaking change". Put the closing tag on its own line and add a short Breaking/Security section.
… type/bit depth, per-pixel-type fallback for L16/La16/La32/A8 etc.) and add round-trip tests
3 participants
Title
Added a dedicated .NET 8 target that uses SixLabors.ImageSharp 4.1.2 and SixLabors.ImageSharp.Drawing 3.1.2, allowing .NET 8+ projects to upgrade to ImageSharp v4 and pick up its security fixes (including CVE-2026-106114). .NET 6 and other targets are unchanged
Description
Added a .NET 8 target using ImageSharp 4.1.2 and Drawing 3.1.2, enabling security fixes including CVE-2026-106114. Other targets remain unchanged.
Fixes #(issue number)
Fixes DW-38
Type of change
Please select the relevant option by placing an 'x' inside the brackets, like this: [x].
How Has This Been Tested?
Run tests on .Net8
Checklist:
Please run through the checklist as much as possible and mark the items completed by placing an 'x' inside the brackets, like this: [x].
Additional Context
Add any other context, screenshots, or information about the pull request here.