Skip to content

Commit

Permalink
Merge pull request #2786 from SixLabors/js/rework-2589
Browse files Browse the repository at this point in the history
Replace PngCrcChunkHandling
  • Loading branch information
JimBobSquarePants authored Aug 12, 2024
2 parents 350e439 + d0fbaeb commit 98437a0
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 48 deletions.
5 changes: 5 additions & 0 deletions src/ImageSharp/Formats/DecoderOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,10 @@ public sealed class DecoderOptions
/// </summary>
public uint MaxFrames { get => this.maxFrames; init => this.maxFrames = Math.Clamp(value, 1, int.MaxValue); }

/// <summary>
/// Gets the segment error handling strategy to use during decoding.
/// </summary>
public SegmentIntegrityHandling SegmentIntegrityHandling { get; init; } = SegmentIntegrityHandling.IgnoreNonCritical;

internal void SetConfiguration(Configuration configuration) => this.configuration = configuration;
}
10 changes: 5 additions & 5 deletions src/ImageSharp/Formats/Png/PngChunk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ public PngChunk(int length, PngChunkType type, IMemoryOwner<byte> data = null)
/// <summary>
/// Gets a value indicating whether the given chunk is critical to decoding
/// </summary>
/// <param name="handling">The chunk CRC handling behavior.</param>
public bool IsCritical(PngCrcChunkHandling handling)
/// <param name="handling">The segment handling behavior.</param>
public bool IsCritical(SegmentIntegrityHandling handling)
=> handling switch
{
PngCrcChunkHandling.IgnoreNone => true,
PngCrcChunkHandling.IgnoreNonCritical => this.Type is PngChunkType.Header or PngChunkType.Palette or PngChunkType.Data or PngChunkType.FrameData,
PngCrcChunkHandling.IgnoreData => this.Type is PngChunkType.Header or PngChunkType.Palette,
SegmentIntegrityHandling.IgnoreNone => true,
SegmentIntegrityHandling.IgnoreNonCritical => this.Type is PngChunkType.Header or PngChunkType.Palette or PngChunkType.Data or PngChunkType.FrameData,
SegmentIntegrityHandling.IgnoreData => this.Type is PngChunkType.Header or PngChunkType.Palette,
_ => false,
};
}
30 changes: 0 additions & 30 deletions src/ImageSharp/Formats/Png/PngCrcChunkHandling.cs

This file was deleted.

12 changes: 6 additions & 6 deletions src/ImageSharp/Formats/Png/PngDecoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore
/// <summary>
/// How to handle CRC errors.
/// </summary>
private readonly PngCrcChunkHandling pngCrcChunkHandling;
private readonly SegmentIntegrityHandling segmentIntegrityHandling;

/// <summary>
/// A reusable Crc32 hashing instance.
Expand All @@ -142,7 +142,7 @@ public PngDecoderCore(PngDecoderOptions options)
this.maxFrames = options.GeneralOptions.MaxFrames;
this.skipMetadata = options.GeneralOptions.SkipMetadata;
this.memoryAllocator = this.configuration.MemoryAllocator;
this.pngCrcChunkHandling = options.PngCrcChunkHandling;
this.segmentIntegrityHandling = options.GeneralOptions.SegmentIntegrityHandling;
this.maxUncompressedLength = options.MaxUncompressedAncillaryChunkSizeBytes;
}

Expand All @@ -154,7 +154,7 @@ internal PngDecoderCore(PngDecoderOptions options, bool colorMetadataOnly)
this.skipMetadata = true;
this.configuration = options.GeneralOptions.Configuration;
this.memoryAllocator = this.configuration.MemoryAllocator;
this.pngCrcChunkHandling = options.PngCrcChunkHandling;
this.segmentIntegrityHandling = options.GeneralOptions.SegmentIntegrityHandling;
this.maxUncompressedLength = options.MaxUncompressedAncillaryChunkSizeBytes;
}

Expand Down Expand Up @@ -833,7 +833,7 @@ private void DecodePixelData<TPixel>(
break;

default:
if (this.pngCrcChunkHandling is PngCrcChunkHandling.IgnoreData or PngCrcChunkHandling.IgnoreAll)
if (this.segmentIntegrityHandling is SegmentIntegrityHandling.IgnoreData or SegmentIntegrityHandling.IgnoreAll)
{
goto EXIT;
}
Expand Down Expand Up @@ -939,7 +939,7 @@ private void DecodeInterlacedPixelData<TPixel>(
break;

default:
if (this.pngCrcChunkHandling is PngCrcChunkHandling.IgnoreData or PngCrcChunkHandling.IgnoreAll)
if (this.segmentIntegrityHandling is SegmentIntegrityHandling.IgnoreData or SegmentIntegrityHandling.IgnoreAll)
{
goto EXIT;
}
Expand Down Expand Up @@ -1927,7 +1927,7 @@ private bool TryReadChunk(Span<byte> buffer, out PngChunk chunk)
private void ValidateChunk(in PngChunk chunk, Span<byte> buffer)
{
uint inputCrc = this.ReadChunkCrc(buffer);
if (chunk.IsCritical(this.pngCrcChunkHandling))
if (chunk.IsCritical(this.segmentIntegrityHandling))
{
Span<byte> chunkType = stackalloc byte[4];
BinaryPrimitives.WriteUInt32BigEndian(chunkType, (uint)chunk.Type);
Expand Down
5 changes: 0 additions & 5 deletions src/ImageSharp/Formats/Png/PngDecoderOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,6 @@ public sealed class PngDecoderOptions : ISpecializedDecoderOptions
/// <inheritdoc/>
public DecoderOptions GeneralOptions { get; init; } = new DecoderOptions();

/// <summary>
/// Gets a value indicating how to handle validation of any CRC (Cyclic Redundancy Check) data within the encoded PNG.
/// </summary>
public PngCrcChunkHandling PngCrcChunkHandling { get; init; } = PngCrcChunkHandling.IgnoreNonCritical;

/// <summary>
/// Gets the maximum memory in bytes that a zTXt, sPLT, iTXt, iCCP, or unknown chunk can occupy when decompressed.
/// Defaults to 8MB
Expand Down
30 changes: 30 additions & 0 deletions src/ImageSharp/Formats/SegmentIntegrityHandling.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright (c) Six Labors.
// Licensed under the Six Labors Split License.

namespace SixLabors.ImageSharp.Formats;

/// <summary>
/// Specifies how to handle validation of errors in different segments of encoded image files.
/// </summary>
public enum SegmentIntegrityHandling
{
/// <summary>
/// Do not ignore any errors.
/// </summary>
IgnoreNone,

/// <summary>
/// Ignore errors in non-critical segments of the encoded image.
/// </summary>
IgnoreNonCritical,

/// <summary>
/// Ignore errors in data segments (e.g., image data, metadata).
/// </summary>
IgnoreData,

/// <summary>
/// Ignore errors in all segments.
/// </summary>
IgnoreAll
}
18 changes: 16 additions & 2 deletions tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,20 @@ public void Identify(string imagePath, int expectedPixelSize)
Assert.Equal(expectedPixelSize, imageInfo.PixelType.BitsPerPixel);
}

[Theory]
[InlineData(TestImages.Png.Bad.WrongCrcDataChunk, 1)]
[InlineData(TestImages.Png.Bad.Issue2589, 24)]
public void Identify_IgnoreCrcErrors(string imagePath, int expectedPixelSize)
{
TestFile testFile = TestFile.Create(imagePath);
using MemoryStream stream = new(testFile.Bytes, false);

ImageInfo imageInfo = Image.Identify(new DecoderOptions() { SegmentIntegrityHandling = SegmentIntegrityHandling.IgnoreData }, stream);

Assert.NotNull(imageInfo);
Assert.Equal(expectedPixelSize, imageInfo.PixelType.BitsPerPixel);
}

[Theory]
[WithFile(TestImages.Png.Bad.MissingDataChunk, PixelTypes.Rgba32)]
public void Decode_MissingDataChunk_ThrowsException<TPixel>(TestImageProvider<TPixel> provider)
Expand Down Expand Up @@ -479,7 +493,7 @@ public void Decode_InvalidDataChunkCrc_ThrowsException<TPixel>(TestImageProvider
public void Decode_InvalidDataChunkCrc_IgnoreCrcErrors<TPixel>(TestImageProvider<TPixel> provider, bool compare)
where TPixel : unmanaged, IPixel<TPixel>
{
using Image<TPixel> image = provider.GetImage(PngDecoder.Instance, new PngDecoderOptions() { PngCrcChunkHandling = PngCrcChunkHandling.IgnoreData });
using Image<TPixel> image = provider.GetImage(PngDecoder.Instance, new DecoderOptions() { SegmentIntegrityHandling = SegmentIntegrityHandling.IgnoreData });

image.DebugSave(provider);
if (compare)
Expand Down Expand Up @@ -660,7 +674,7 @@ static void RunTest(string providerDump, string nonContiguousBuffersStr)
public void Binary_PrematureEof()
{
PngDecoder decoder = PngDecoder.Instance;
PngDecoderOptions options = new() { PngCrcChunkHandling = PngCrcChunkHandling.IgnoreData };
PngDecoderOptions options = new() { GeneralOptions = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.IgnoreData } };
using EofHitCounter eofHitCounter = EofHitCounter.RunDecoder(TestImages.Png.Bad.FlagOfGermany0000016446, decoder, options);

// TODO: Try to reduce this to 1.
Expand Down

0 comments on commit 98437a0

Please sign in to comment.