From e408e411b8fe694da41448d9d4fd69dfe655df2f Mon Sep 17 00:00:00 2001 From: Jason Finch Date: Sun, 18 Oct 2026 09:21:21 +1000 Subject: [PATCH 1/3] Skip stray TEM markers and reserved markers before restart markers when decoding jpegs A TEM marker (FF 01) between header segments was read as a marker with a length field, so the next segment header was consumed as its length and decoding failed. TEM stands alone like RSTn, so skip it the same way. Some producers write extraneous bytes before restart markers. Those that form a TEM or reserved marker (FF 01 to FF BF) were taken as the end of the scan, which corrupted the rest of the image or made the marker parser fail. Skip them while looking for the marker that ends a restart interval and resync at the next marker, as libjpeg does. --- .../Components/Decoder/HuffmanScanDecoder.cs | 6 +- .../Jpeg/Components/Decoder/JpegBitReader.cs | 7 + src/ImageSharp/Formats/Jpeg/JpegConstants.cs | 9 + .../Formats/Jpeg/JpegDecoderCore.cs | 6 +- .../Formats/Jpg/JpegDecoderTests.Malformed.cs | 165 ++++++++++++++++++ 5 files changed, 190 insertions(+), 3 deletions(-) create mode 100644 tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Malformed.cs diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs index 9ee43a2c83..41b4552c1b 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs @@ -745,7 +745,11 @@ private bool HandleRestart() { if (this.restartInterval > 0 && (--this.todo) == 0) { - if (this.scanBuffer.Marker == JpegConstants.Markers.XFF) + // Find the marker that ends this interval. FindNextMarker skips the extraneous data bytes some + // producers write before it. Those bytes can also form a TEM or reserved marker (FF 01 to FF BF) + // which cannot end an interval, so skip those too and resync at the next marker, as libjpeg does. + // Treating such a marker as the end of the scan would lose the rest of the scan data. + while (this.scanBuffer.Marker == JpegConstants.Markers.XFF || this.scanBuffer.HasReservedMarker()) { if (!this.scanBuffer.FindNextMarker()) { diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs index f888a8fb7b..c3fbdda22c 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs @@ -84,6 +84,13 @@ public void Reset() [MethodImpl(InliningOptions.ShortMethod)] public readonly bool HasBadMarker() => this.Marker != JpegConstants.Markers.XFF && !this.HasRestartMarker(); + /// + /// Whether a TEM or reserved marker has been detected, I.E. One below SOF0. + /// Such a marker can neither end a restart interval nor the scan. + /// + [MethodImpl(InliningOptions.ShortMethod)] + public readonly bool HasReservedMarker() => this.Marker < JpegConstants.Markers.SOF0; + [MethodImpl(InliningOptions.AlwaysInline)] public void FillBuffer() { diff --git a/src/ImageSharp/Formats/Jpeg/JpegConstants.cs b/src/ImageSharp/Formats/Jpeg/JpegConstants.cs index 50b7de68e4..9e34f1130c 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegConstants.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegConstants.cs @@ -271,6 +271,15 @@ internal static class Markers /// /// public const byte RST7 = 0xD7; + + /// + /// Temporary private use in arithmetic coding + /// + /// Like SOI, EOI and RSTn, this marker stands alone and is not followed by a length field. + /// It is the lowest marker code, every code between it and is reserved. + /// + /// + public const byte TEM = 0x01; } /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 65b0f4d5d3..af1d9e6e9d 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -188,8 +188,10 @@ public static JpegFileMarker FindNextFileMarker(BufferedReadStream stream) } } - // Found a valid marker. Exit loop - if (b is not 0 and (< JpegConstants.Markers.RST0 or > JpegConstants.Markers.RST7)) + // Found a valid marker. Exit loop. + // FF 00 is a stuffed data byte. RSTn and TEM stand alone without a length field (ITU T.81 B.1.1.4), + // so stray ones between marker segments carry no data and are skipped. + if (b is not 0 and not JpegConstants.Markers.TEM and (< JpegConstants.Markers.RST0 or > JpegConstants.Markers.RST7)) { return new JpegFileMarker((byte)(uint)b, stream.Position - 2); } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Malformed.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Malformed.cs new file mode 100644 index 0000000000..e0871ea164 --- /dev/null +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Malformed.cs @@ -0,0 +1,165 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using SixLabors.ImageSharp.Formats.Jpeg; +using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; + +// ReSharper disable InconsistentNaming +namespace SixLabors.ImageSharp.Tests.Formats.Jpg; + +/// +/// Tests for streams written by imperfect producers which libjpeg based decoders still decode. +/// +[Trait("Format", "Jpg")] +public partial class JpegDecoderTests +{ + /// + /// Gets the encoder layouts paired with the junk inserted before every restart marker. + /// The junk forms a TEM or reserved marker (FF 01 to FF BF) which cannot end a restart interval. + /// + public static TheoryData ReservedMarkerBeforeRestartData { get; } = new() + { + // Baseline interleaved. + { false, true, [0xFF, 0x12] }, + { false, true, [0xFF, 0x01] }, + { false, true, [0xFF, 0xBF] }, + { false, true, [0xFF, 0x12, 0xFF, 0x34] }, + + // Baseline non-interleaved. + { false, false, [0xFF, 0x12] }, + + // Progressive. + { true, false, [0xFF, 0x12] }, + }; + + [Theory] + [MemberData(nameof(ReservedMarkerBeforeRestartData))] + public void Decode_ReservedMarkerBeforeRestartMarkers_IsSkipped(bool progressive, bool interleaved, byte[] junk) + { + byte[] original = EncodeGradientJpeg(new JpegEncoder + { + ColorType = JpegColorType.YCbCrRatio420, + Progressive = progressive, + Interleaved = interleaved, + RestartInterval = 1 + }); + + byte[] corrupt = InsertBeforeRestartMarkers(original, junk); + + using Image expected = Image.Load(original); + using Image actual = Image.Load(corrupt); + ImageComparer.Exact.VerifySimilarity(expected, actual); + } + + [Fact] + public void Decode_TemMarkerBetweenHeaderSegments_IsSkipped() + { + byte[] original = EncodeGradientJpeg(new JpegEncoder { ColorType = JpegColorType.YCbCrRatio420 }); + + // TEM has no length field. Insert one straight after the APP0 segment which directly follows SOI. + byte[] corrupt = Insert(original, SegmentEnd(original, 2), JpegConstants.Markers.XFF, JpegConstants.Markers.TEM); + + ImageInfo info = Image.Identify(corrupt); + Assert.Equal(new Size(32, 24), info.Size); + + using Image expected = Image.Load(original); + using Image actual = Image.Load(corrupt); + ImageComparer.Exact.VerifySimilarity(expected, actual); + } + + /// + /// Encodes a 32x24 gradient, which spans several MCUs in every layout, with the given encoder. + /// + /// The encoder. + /// The encoded jpeg. + private static byte[] EncodeGradientJpeg(JpegEncoder encoder) + { + using Image image = new(32, 24); + for (int y = 0; y < image.Height; y++) + { + for (int x = 0; x < image.Width; x++) + { + image[x, y] = new Rgba32((byte)(x * 8), (byte)(y * 10), (byte)((x + y) * 4)); + } + } + + using MemoryStream ms = new(); + image.SaveAsJpeg(ms, encoder); + return ms.ToArray(); + } + + /// + /// Inserts the junk bytes before every RSTn marker in the entropy-coded data following the first SOS. + /// + /// The jpeg. + /// The bytes to insert. + /// The corrupted jpeg. + private static byte[] InsertBeforeRestartMarkers(byte[] data, byte[] junk) + { + int start = SegmentEnd(data, FindSegment(data, JpegConstants.Markers.SOS)); + using MemoryStream ms = new(); + ms.Write(data, 0, start); + + int count = 0; + for (int i = start; i < data.Length; i++) + { + if (data[i] == JpegConstants.Markers.XFF && i + 1 < data.Length && data[i + 1] is >= JpegConstants.Markers.RST0 and <= JpegConstants.Markers.RST7) + { + ms.Write(junk, 0, junk.Length); + count++; + } + + ms.WriteByte(data[i]); + } + + Assert.True(count > 0, "The test image has no restart markers."); + return ms.ToArray(); + } + + /// + /// Walks the marker segments following SOI up to the first SOS. + /// + /// The jpeg. + /// The marker code to find. + /// The offset of the first segment with the given marker code. + private static int FindSegment(byte[] data, byte marker) + { + int offset = 2; + while (data[offset + 1] != marker) + { + Assert.NotEqual(JpegConstants.Markers.SOS, data[offset + 1]); + offset = SegmentEnd(data, offset); + } + + return offset; + } + + /// + /// Gets the offset just past the marker segment (FF xx and its length field) starting at the given offset. + /// + /// The jpeg. + /// The offset of the segment. + /// The offset of the following segment. + private static int SegmentEnd(byte[] data, int offset) + { + Assert.Equal(JpegConstants.Markers.XFF, data[offset]); + return offset + 2 + ((data[offset + 2] << 8) | data[offset + 3]); + } + + /// + /// Inserts bytes at the given offset. + /// + /// The source data. + /// The offset to insert at. + /// The bytes to insert. + /// The new data. + private static byte[] Insert(byte[] data, int offset, params byte[] bytes) + { + byte[] result = new byte[data.Length + bytes.Length]; + Buffer.BlockCopy(data, 0, result, 0, offset); + Buffer.BlockCopy(bytes, 0, result, offset, bytes.Length); + Buffer.BlockCopy(data, offset, result, offset + bytes.Length, data.Length - offset); + return result; + } +} From 012d15c09d9367908b2696ca17dd9fcf3508284f Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 27 Oct 2026 17:18:34 +1000 Subject: [PATCH 2/3] Apply JPEG restart recovery through the shared reader and use stored inputs --- .../Decoder/ArithmeticScanDecoder.cs | 11 +- .../Components/Decoder/HuffmanScanDecoder.cs | 12 +- .../Components/Decoder/IJpegScanDecoder.cs | 5 + .../Jpeg/Components/Decoder/JpegBitReader.cs | 59 ++++--- src/ImageSharp/Formats/Jpeg/JpegConstants.cs | 7 +- .../Formats/Jpeg/JpegDecoderCore.cs | 14 +- .../Formats/Jpg/JpegDecoderTests.Malformed.cs | 165 ------------------ .../Formats/Jpg/JpegDecoderTests.cs | 93 ++++++++++ tests/ImageSharp.Tests/TestImages.cs | 5 + .../arithmetic-progressive-reserved-12.jpg | 3 + .../Jpg/markers/arithmetic-progressive.jpg | 3 + .../Jpg/markers/arithmetic-reserved-12.jpg | 3 + tests/Images/Input/Jpg/markers/arithmetic.jpg | 3 + .../Jpg/markers/baseline-reserved-12.jpg | 3 + .../Jpg/markers/baseline-reserved-bf.jpg | 3 + .../Jpg/markers/baseline-reserved-chain.jpg | 3 + .../Images/Input/Jpg/markers/baseline-tem.jpg | 3 + tests/Images/Input/Jpg/markers/baseline.jpg | 3 + .../markers/header-tem-invalid-dqt-length.jpg | 3 + tests/Images/Input/Jpg/markers/header-tem.jpg | 3 + .../markers/noninterleaved-reserved-12.jpg | 3 + .../Input/Jpg/markers/noninterleaved.jpg | 3 + .../Jpg/markers/progressive-reserved-12.jpg | 3 + .../Images/Input/Jpg/markers/progressive.jpg | 3 + 24 files changed, 212 insertions(+), 204 deletions(-) delete mode 100644 tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Malformed.cs create mode 100644 tests/Images/Input/Jpg/markers/arithmetic-progressive-reserved-12.jpg create mode 100644 tests/Images/Input/Jpg/markers/arithmetic-progressive.jpg create mode 100644 tests/Images/Input/Jpg/markers/arithmetic-reserved-12.jpg create mode 100644 tests/Images/Input/Jpg/markers/arithmetic.jpg create mode 100644 tests/Images/Input/Jpg/markers/baseline-reserved-12.jpg create mode 100644 tests/Images/Input/Jpg/markers/baseline-reserved-bf.jpg create mode 100644 tests/Images/Input/Jpg/markers/baseline-reserved-chain.jpg create mode 100644 tests/Images/Input/Jpg/markers/baseline-tem.jpg create mode 100644 tests/Images/Input/Jpg/markers/baseline.jpg create mode 100644 tests/Images/Input/Jpg/markers/header-tem-invalid-dqt-length.jpg create mode 100644 tests/Images/Input/Jpg/markers/header-tem.jpg create mode 100644 tests/Images/Input/Jpg/markers/noninterleaved-reserved-12.jpg create mode 100644 tests/Images/Input/Jpg/markers/noninterleaved.jpg create mode 100644 tests/Images/Input/Jpg/markers/progressive-reserved-12.jpg create mode 100644 tests/Images/Input/Jpg/markers/progressive.jpg diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ArithmeticScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ArithmeticScanDecoder.cs index 75f2c63713..0d5d0ad439 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ArithmeticScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ArithmeticScanDecoder.cs @@ -202,6 +202,9 @@ public ArithmeticScanDecoder(BufferedReadStream stream, SpectralConverter conver this.ct = -16; // Force reading 2 initial bytes to fill C. } + /// + public SegmentIntegrityHandling SegmentIntegrityHandling { get; set; } + /// public int ResetInterval { @@ -1100,9 +1103,10 @@ private bool HandleRestart() { if (this.restartInterval > 0 && (--this.todo) == 0) { - if (this.scanBuffer.Marker == JpegConstants.Markers.XFF) + // Use the same marker recovery and integrity policy as Huffman-coded scans. + if (this.scanBuffer.Marker == JpegConstants.Markers.XFF || this.scanBuffer.HasReservedMarker()) { - if (!this.scanBuffer.FindNextMarker()) + if (!this.scanBuffer.FindNextMarker(this.SegmentIntegrityHandling)) { return false; } @@ -1119,8 +1123,7 @@ private bool HandleRestart() component.AcStatistics?.Reset(); } - this.Reset(); - + // Reset clears the detected marker. Classify it first so a real segment marker can be read again. if (this.scanBuffer.HasRestartMarker()) { this.Reset(); diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs index 41b4552c1b..2aa485ceab 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs @@ -85,6 +85,9 @@ public HuffmanScanDecoder( this.acHuffmanTables = new HuffmanTable[maxTables]; } + /// + public SegmentIntegrityHandling SegmentIntegrityHandling { get; set; } + /// /// Sets reset interval determined by RST markers. /// @@ -745,13 +748,10 @@ private bool HandleRestart() { if (this.restartInterval > 0 && (--this.todo) == 0) { - // Find the marker that ends this interval. FindNextMarker skips the extraneous data bytes some - // producers write before it. Those bytes can also form a TEM or reserved marker (FF 01 to FF BF) - // which cannot end an interval, so skip those too and resync at the next marker, as libjpeg does. - // Treating such a marker as the end of the scan would lose the rest of the scan data. - while (this.scanBuffer.Marker == JpegConstants.Markers.XFF || this.scanBuffer.HasReservedMarker()) + // A reserved marker cannot complete a restart interval. The shared reader applies the recovery policy. + if (this.scanBuffer.Marker == JpegConstants.Markers.XFF || this.scanBuffer.HasReservedMarker()) { - if (!this.scanBuffer.FindNextMarker()) + if (!this.scanBuffer.FindNextMarker(this.SegmentIntegrityHandling)) { return false; } diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/IJpegScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/IJpegScanDecoder.cs index 588090deb6..1540885d97 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/IJpegScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/IJpegScanDecoder.cs @@ -10,6 +10,11 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder; /// internal interface IJpegScanDecoder { + /// + /// Sets the recovery policy for errors in compressed scan data. + /// + public SegmentIntegrityHandling SegmentIntegrityHandling { set; } + /// /// Sets the reset interval. /// diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs index c3fbdda22c..c77e2e0762 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs @@ -85,9 +85,9 @@ public void Reset() public readonly bool HasBadMarker() => this.Marker != JpegConstants.Markers.XFF && !this.HasRestartMarker(); /// - /// Whether a TEM or reserved marker has been detected, I.E. One below SOF0. - /// Such a marker can neither end a restart interval nor the scan. + /// Determines whether the detected marker is TEM or a reserved code before SOF0. /// + /// True for TEM and reserved marker codes. [MethodImpl(InliningOptions.ShortMethod)] public readonly bool HasReservedMarker() => this.Marker < JpegConstants.Markers.SOF0; @@ -184,39 +184,56 @@ private ulong GetBytes() return temp; } + /// + /// Finds the next marker at a restart boundary and applies the selected image-data recovery policy. + /// + /// The recovery policy for compressed image data. + /// True if a marker was found. False if the stream ended first. [MethodImpl(InliningOptions.ShortMethod)] - public bool FindNextMarker() + public bool FindNextMarker(SegmentIntegrityHandling integrityHandling) { - while (true) + do { - int b = this.stream.ReadByte(); - if (b == -1) + // The bit reader can already hold a marker from its entropy-data lookahead. + // Libjpeg treats codes before SOF0 as damaged image data and scans forward to another marker. + // Only IgnoreImageData permits that recovery. Real segment markers stay with the segment parser. + if (this.HasReservedMarker() && integrityHandling != SegmentIntegrityHandling.IgnoreImageData) { - return false; + JpegThrowHelper.ThrowInvalidImageContentException("Unexpected marker before a JPEG restart marker."); } - // Found a marker. - if (b == JpegConstants.Markers.XFF) + while (true) { - while (b == JpegConstants.Markers.XFF) + int b = this.stream.ReadByte(); + if (b == -1) { - // Loop here to discard any padding FF bytes on terminating marker. - b = this.stream.ReadByte(); - if (b == -1) - { - return false; - } + return false; } - // Found a valid marker. Exit loop - if (b != 0) + if (b == JpegConstants.Markers.XFF) { - this.Marker = (byte)b; - this.MarkerPosition = this.stream.Position - 2; - return true; + // Multiple FF bytes are marker padding. FF 00 is a stuffed data byte, not a marker. + while (b == JpegConstants.Markers.XFF) + { + b = this.stream.ReadByte(); + if (b == -1) + { + return false; + } + } + + if (b != 0) + { + this.Marker = (byte)b; + this.MarkerPosition = this.stream.Position - 2; + break; + } } } } + while (this.HasReservedMarker()); + + return true; } [MethodImpl(InliningOptions.AlwaysInline)] diff --git a/src/ImageSharp/Formats/Jpeg/JpegConstants.cs b/src/ImageSharp/Formats/Jpeg/JpegConstants.cs index 9e34f1130c..6a8eb07643 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegConstants.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegConstants.cs @@ -273,12 +273,11 @@ internal static class Markers public const byte RST7 = 0xD7; /// - /// Temporary private use in arithmetic coding + /// Temporary private marker for arithmetic coding. + /// /// - /// Like SOI, EOI and RSTn, this marker stands alone and is not followed by a length field. - /// It is the lowest marker code, every code between it and is reserved. + /// This marker stands alone. It has no length field or payload. /// - /// public const byte TEM = 0x01; } diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index af1d9e6e9d..a5cc7d2ca5 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -188,9 +188,8 @@ public static JpegFileMarker FindNextFileMarker(BufferedReadStream stream) } } - // Found a valid marker. Exit loop. - // FF 00 is a stuffed data byte. RSTn and TEM stand alone without a length field (ITU T.81 B.1.1.4), - // so stray ones between marker segments carry no data and are skipped. + // FF 00 is a stuffed data byte. RSTn and TEM have no length field (ITU T.81, Table B.1). + // Skip these standalone markers so the next marker's bytes are not read as a length. if (b is not 0 and not JpegConstants.Markers.TEM and (< JpegConstants.Markers.RST0 or > JpegConstants.Markers.RST7)) { return new JpegFileMarker((byte)(uint)b, stream.Position - 2); @@ -326,6 +325,9 @@ internal void ParseStream(BufferedReadStream stream, SpectralConverter spectralC this.scanDecoder ??= new HuffmanScanDecoder(stream, spectralConverter, cancellationToken); + // Apply the image-data policy to the active decoder, including one supplied by TIFF. + this.scanDecoder.SegmentIntegrityHandling = this.Options.SegmentIntegrityHandling; + this.Metadata ??= new ImageMetadata(); Span markerBuffer = stackalloc byte[2]; @@ -388,7 +390,11 @@ internal void ParseStream(BufferedReadStream stream, SpectralConverter spectralC case JpegConstants.Markers.SOF10: case JpegConstants.Markers.SOF13: case JpegConstants.Markers.SOF14: - this.scanDecoder = new ArithmeticScanDecoder(stream, spectralConverter, cancellationToken); + this.scanDecoder = new ArithmeticScanDecoder(stream, spectralConverter, cancellationToken) + { + SegmentIntegrityHandling = this.Options.SegmentIntegrityHandling + }; + if (this.resetInterval.HasValue) { this.scanDecoder.ResetInterval = this.resetInterval.Value; diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Malformed.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Malformed.cs deleted file mode 100644 index e0871ea164..0000000000 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Malformed.cs +++ /dev/null @@ -1,165 +0,0 @@ -// Copyright (c) Six Labors. -// Licensed under the Six Labors Split License. - -using SixLabors.ImageSharp.Formats.Jpeg; -using SixLabors.ImageSharp.PixelFormats; -using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; - -// ReSharper disable InconsistentNaming -namespace SixLabors.ImageSharp.Tests.Formats.Jpg; - -/// -/// Tests for streams written by imperfect producers which libjpeg based decoders still decode. -/// -[Trait("Format", "Jpg")] -public partial class JpegDecoderTests -{ - /// - /// Gets the encoder layouts paired with the junk inserted before every restart marker. - /// The junk forms a TEM or reserved marker (FF 01 to FF BF) which cannot end a restart interval. - /// - public static TheoryData ReservedMarkerBeforeRestartData { get; } = new() - { - // Baseline interleaved. - { false, true, [0xFF, 0x12] }, - { false, true, [0xFF, 0x01] }, - { false, true, [0xFF, 0xBF] }, - { false, true, [0xFF, 0x12, 0xFF, 0x34] }, - - // Baseline non-interleaved. - { false, false, [0xFF, 0x12] }, - - // Progressive. - { true, false, [0xFF, 0x12] }, - }; - - [Theory] - [MemberData(nameof(ReservedMarkerBeforeRestartData))] - public void Decode_ReservedMarkerBeforeRestartMarkers_IsSkipped(bool progressive, bool interleaved, byte[] junk) - { - byte[] original = EncodeGradientJpeg(new JpegEncoder - { - ColorType = JpegColorType.YCbCrRatio420, - Progressive = progressive, - Interleaved = interleaved, - RestartInterval = 1 - }); - - byte[] corrupt = InsertBeforeRestartMarkers(original, junk); - - using Image expected = Image.Load(original); - using Image actual = Image.Load(corrupt); - ImageComparer.Exact.VerifySimilarity(expected, actual); - } - - [Fact] - public void Decode_TemMarkerBetweenHeaderSegments_IsSkipped() - { - byte[] original = EncodeGradientJpeg(new JpegEncoder { ColorType = JpegColorType.YCbCrRatio420 }); - - // TEM has no length field. Insert one straight after the APP0 segment which directly follows SOI. - byte[] corrupt = Insert(original, SegmentEnd(original, 2), JpegConstants.Markers.XFF, JpegConstants.Markers.TEM); - - ImageInfo info = Image.Identify(corrupt); - Assert.Equal(new Size(32, 24), info.Size); - - using Image expected = Image.Load(original); - using Image actual = Image.Load(corrupt); - ImageComparer.Exact.VerifySimilarity(expected, actual); - } - - /// - /// Encodes a 32x24 gradient, which spans several MCUs in every layout, with the given encoder. - /// - /// The encoder. - /// The encoded jpeg. - private static byte[] EncodeGradientJpeg(JpegEncoder encoder) - { - using Image image = new(32, 24); - for (int y = 0; y < image.Height; y++) - { - for (int x = 0; x < image.Width; x++) - { - image[x, y] = new Rgba32((byte)(x * 8), (byte)(y * 10), (byte)((x + y) * 4)); - } - } - - using MemoryStream ms = new(); - image.SaveAsJpeg(ms, encoder); - return ms.ToArray(); - } - - /// - /// Inserts the junk bytes before every RSTn marker in the entropy-coded data following the first SOS. - /// - /// The jpeg. - /// The bytes to insert. - /// The corrupted jpeg. - private static byte[] InsertBeforeRestartMarkers(byte[] data, byte[] junk) - { - int start = SegmentEnd(data, FindSegment(data, JpegConstants.Markers.SOS)); - using MemoryStream ms = new(); - ms.Write(data, 0, start); - - int count = 0; - for (int i = start; i < data.Length; i++) - { - if (data[i] == JpegConstants.Markers.XFF && i + 1 < data.Length && data[i + 1] is >= JpegConstants.Markers.RST0 and <= JpegConstants.Markers.RST7) - { - ms.Write(junk, 0, junk.Length); - count++; - } - - ms.WriteByte(data[i]); - } - - Assert.True(count > 0, "The test image has no restart markers."); - return ms.ToArray(); - } - - /// - /// Walks the marker segments following SOI up to the first SOS. - /// - /// The jpeg. - /// The marker code to find. - /// The offset of the first segment with the given marker code. - private static int FindSegment(byte[] data, byte marker) - { - int offset = 2; - while (data[offset + 1] != marker) - { - Assert.NotEqual(JpegConstants.Markers.SOS, data[offset + 1]); - offset = SegmentEnd(data, offset); - } - - return offset; - } - - /// - /// Gets the offset just past the marker segment (FF xx and its length field) starting at the given offset. - /// - /// The jpeg. - /// The offset of the segment. - /// The offset of the following segment. - private static int SegmentEnd(byte[] data, int offset) - { - Assert.Equal(JpegConstants.Markers.XFF, data[offset]); - return offset + 2 + ((data[offset + 2] << 8) | data[offset + 3]); - } - - /// - /// Inserts bytes at the given offset. - /// - /// The source data. - /// The offset to insert at. - /// The bytes to insert. - /// The new data. - private static byte[] Insert(byte[] data, int offset, params byte[] bytes) - { - byte[] result = new byte[data.Length + bytes.Length]; - Buffer.BlockCopy(data, 0, result, 0, offset); - Buffer.BlockCopy(bytes, 0, result, offset, bytes.Length); - Buffer.BlockCopy(data, offset, result, offset + bytes.Length, data.Length - offset); - return result; - } -} diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index a1f5f74be9..658b4e2f30 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -9,6 +9,7 @@ using SixLabors.ImageSharp.Tests.Formats.Jpg.Utils; using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; using SixLabors.ImageSharp.Tests.TestUtilities.ReferenceCodecs; +using static SixLabors.ImageSharp.Tests.TestImages.Jpeg; // ReSharper disable InconsistentNaming namespace SixLabors.ImageSharp.Tests.Formats.Jpg; @@ -299,6 +300,98 @@ public void Issue2136_DecodeWorks(TestImageProvider provider) image.CompareToOriginal(provider); } + /// + /// Checks restart recovery against clean stored files whose pixels also match in libjpeg. + /// + /// The clean JPEG input. + /// The input with extra markers before restart markers. + [Theory] + [InlineData(MarkerDirectory + "baseline.jpg", MarkerDirectory + "baseline-reserved-12.jpg")] + [InlineData(MarkerDirectory + "baseline.jpg", MarkerDirectory + "baseline-tem.jpg")] + [InlineData(MarkerDirectory + "baseline.jpg", MarkerDirectory + "baseline-reserved-bf.jpg")] + [InlineData(MarkerDirectory + "baseline.jpg", MarkerDirectory + "baseline-reserved-chain.jpg")] + [InlineData(MarkerDirectory + "noninterleaved.jpg", MarkerDirectory + "noninterleaved-reserved-12.jpg")] + [InlineData(MarkerDirectory + "progressive.jpg", MarkerDirectory + "progressive-reserved-12.jpg")] + [InlineData(MarkerDirectory + "arithmetic.jpg", MarkerDirectory + "arithmetic-reserved-12.jpg")] + [InlineData(MarkerDirectory + "arithmetic-progressive.jpg", MarkerDirectory + "arithmetic-progressive-reserved-12.jpg")] + public void Decode_ReservedMarkerBeforeRestartMarkers_RecoversWithIgnoreImageData(string originalFile, string malformedFile) + { + // These stored files have complete restart intervals. Libjpeg warns about the extra markers, + // then finds the restart marker and produces the same pixels as the clean file. + // The arithmetic inputs declare their conditioning tables to isolate restart recovery. + DecoderOptions options = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.IgnoreImageData }; + using Image expected = Image.Load(TestFile.Create(originalFile).Bytes); + using Image actual = Image.Load(options, TestFile.Create(malformedFile).Bytes); + + ImageComparer.Exact.VerifySimilarity(expected, actual); + } + + /// + /// Rejects image-data errors when the selected policy permits only ancillary recovery or no recovery. + /// + /// The input with reserved markers before restart markers. + [Theory] + [InlineData(MarkerDirectory + "baseline-reserved-12.jpg")] + [InlineData(MarkerDirectory + "noninterleaved-reserved-12.jpg")] + [InlineData(MarkerDirectory + "progressive-reserved-12.jpg")] + [InlineData(MarkerDirectory + "arithmetic-reserved-12.jpg")] + [InlineData(MarkerDirectory + "arithmetic-progressive-reserved-12.jpg")] + public void Decode_ReservedMarkerBeforeRestartMarkers_RespectsIntegrity(string fileName) + { + byte[] data = TestFile.Create(fileName).Bytes; + DecoderOptions strict = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.Strict }; + DecoderOptions ancillary = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.IgnoreAncillary }; + + Assert.Throws(() => + { + using Image image = Image.Load(strict, data); + }); + + Assert.Throws(() => + { + using Image image = Image.Load(ancillary, data); + }); + } + + /// + /// Accepts a standalone TEM marker between header segments under every integrity policy. + /// + [Fact] + public void Decode_TemMarkerBetweenHeaderSegments_IsSkipped() + { + byte[] data = TestFile.Create(MarkerDirectory + "header-tem.jpg").Bytes; + using Image expected = Image.Load(TestFile.Create(MarkerDirectory + "baseline.jpg").Bytes); + + // TEM has no length field. The following bytes belong to the next marker, not to TEM. + foreach (SegmentIntegrityHandling integrity in Enum.GetValues()) + { + DecoderOptions options = new() { SegmentIntegrityHandling = integrity }; + Assert.Equal(new Size(32, 24), Image.Identify(options, data).Size); + + using Image actual = Image.Load(options, data); + ImageComparer.Exact.VerifySimilarity(expected, actual); + } + } + + /// + /// Keeps required header lengths valid after a standalone TEM marker. + /// + [Fact] + public void Decode_InvalidQuantizationSegmentAfterTem_RemainsFatal() + { + // TEM precedes DQT in this stored file. DQT declares more bytes than the file contains. + // No image data has been read, so there is no safe point for recovery. + byte[] data = TestFile.Create(MarkerDirectory + "header-tem-invalid-dqt-length.jpg").Bytes; + foreach (SegmentIntegrityHandling integrity in Enum.GetValues()) + { + DecoderOptions options = new() { SegmentIntegrityHandling = integrity }; + Assert.Throws(() => + { + using Image image = Image.Load(options, data); + }); + } + } + // https://github.com/SixLabors/ImageSharp/issues/2315 // https://github.com/SixLabors/ImageSharp/issues/2334 [Theory] diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 13e31e7079..6966a32a78 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -264,6 +264,11 @@ public static class Bad public static class Jpeg { + /// + /// Stored JPEG marker and restart-recovery inputs. + /// + public const string MarkerDirectory = "Jpg/markers/"; + public static class ICC { public const string SRgb = "Jpg/icc-profiles/Momiji-sRGB-yes.jpg"; diff --git a/tests/Images/Input/Jpg/markers/arithmetic-progressive-reserved-12.jpg b/tests/Images/Input/Jpg/markers/arithmetic-progressive-reserved-12.jpg new file mode 100644 index 0000000000..49f770afd6 --- /dev/null +++ b/tests/Images/Input/Jpg/markers/arithmetic-progressive-reserved-12.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:66841d2705f2bd9ccdb0e77a7f71965507313556452d4e3fb26e32853e85e4ee +size 778 diff --git a/tests/Images/Input/Jpg/markers/arithmetic-progressive.jpg b/tests/Images/Input/Jpg/markers/arithmetic-progressive.jpg new file mode 100644 index 0000000000..54820d9fd8 --- /dev/null +++ b/tests/Images/Input/Jpg/markers/arithmetic-progressive.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:fe8367c26bce4840a14d41c3a0f64c562722e5caf751bba4082d0c2839482d11 +size 654 diff --git a/tests/Images/Input/Jpg/markers/arithmetic-reserved-12.jpg b/tests/Images/Input/Jpg/markers/arithmetic-reserved-12.jpg new file mode 100644 index 0000000000..02ff57b0eb --- /dev/null +++ b/tests/Images/Input/Jpg/markers/arithmetic-reserved-12.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:0f40aa4188c0c094bda8c2d02fb243700239eb492886fc03616eb37b27dd87af +size 344 diff --git a/tests/Images/Input/Jpg/markers/arithmetic.jpg b/tests/Images/Input/Jpg/markers/arithmetic.jpg new file mode 100644 index 0000000000..437ebe7608 --- /dev/null +++ b/tests/Images/Input/Jpg/markers/arithmetic.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:412a2af2c4005d4526eae74d1270b935fdb816623414c2cec9009b2a1fe16b31 +size 338 diff --git a/tests/Images/Input/Jpg/markers/baseline-reserved-12.jpg b/tests/Images/Input/Jpg/markers/baseline-reserved-12.jpg new file mode 100644 index 0000000000..8ceeefe4ff --- /dev/null +++ b/tests/Images/Input/Jpg/markers/baseline-reserved-12.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:71da350023e239ccfb89e800c856db8880469c7fedae8e35b82327a9d27ffbd8 +size 775 diff --git a/tests/Images/Input/Jpg/markers/baseline-reserved-bf.jpg b/tests/Images/Input/Jpg/markers/baseline-reserved-bf.jpg new file mode 100644 index 0000000000..d099466872 --- /dev/null +++ b/tests/Images/Input/Jpg/markers/baseline-reserved-bf.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:68949614b2d283f6d086d5559e775abb2c409da605bffaac0cedc7ac81479dcb +size 775 diff --git a/tests/Images/Input/Jpg/markers/baseline-reserved-chain.jpg b/tests/Images/Input/Jpg/markers/baseline-reserved-chain.jpg new file mode 100644 index 0000000000..619695d781 --- /dev/null +++ b/tests/Images/Input/Jpg/markers/baseline-reserved-chain.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:60ed63792bcf73bfcc2c87330fd178bd5a39d7b675d8bbc895f53d186859b3a6 +size 781 diff --git a/tests/Images/Input/Jpg/markers/baseline-tem.jpg b/tests/Images/Input/Jpg/markers/baseline-tem.jpg new file mode 100644 index 0000000000..9b55cbd663 --- /dev/null +++ b/tests/Images/Input/Jpg/markers/baseline-tem.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:6499a4154e3a93523dd77af5e719b3d144e92485fa37ba02e95bad86f402b81b +size 775 diff --git a/tests/Images/Input/Jpg/markers/baseline.jpg b/tests/Images/Input/Jpg/markers/baseline.jpg new file mode 100644 index 0000000000..792cd12205 --- /dev/null +++ b/tests/Images/Input/Jpg/markers/baseline.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:576c8e79709961c68f49ff3cde01d8b1ba5c1326bdd36fc1b487cdb5df4533ca +size 769 diff --git a/tests/Images/Input/Jpg/markers/header-tem-invalid-dqt-length.jpg b/tests/Images/Input/Jpg/markers/header-tem-invalid-dqt-length.jpg new file mode 100644 index 0000000000..124ee59ddc --- /dev/null +++ b/tests/Images/Input/Jpg/markers/header-tem-invalid-dqt-length.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:ceada1c48585463c4293dd33c35476d970df41673a9ea9ba5aa11026fd64282d +size 771 diff --git a/tests/Images/Input/Jpg/markers/header-tem.jpg b/tests/Images/Input/Jpg/markers/header-tem.jpg new file mode 100644 index 0000000000..e00564d2d5 --- /dev/null +++ b/tests/Images/Input/Jpg/markers/header-tem.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d9f4fdb4b11c9556c3a531a64e7d4e8db88b2a2c169cf877561b9c060ee69279 +size 771 diff --git a/tests/Images/Input/Jpg/markers/noninterleaved-reserved-12.jpg b/tests/Images/Input/Jpg/markers/noninterleaved-reserved-12.jpg new file mode 100644 index 0000000000..9959107fb4 --- /dev/null +++ b/tests/Images/Input/Jpg/markers/noninterleaved-reserved-12.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:1e426b63bbb8d96511877087a367296c0cb867b7578cc896c1c508e6feb0242c +size 850 diff --git a/tests/Images/Input/Jpg/markers/noninterleaved.jpg b/tests/Images/Input/Jpg/markers/noninterleaved.jpg new file mode 100644 index 0000000000..a9598b2281 --- /dev/null +++ b/tests/Images/Input/Jpg/markers/noninterleaved.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:0881301e40bd562f501a6296f7b9843779b0df6f59d2f2b2a789d634c169f835 +size 816 diff --git a/tests/Images/Input/Jpg/markers/progressive-reserved-12.jpg b/tests/Images/Input/Jpg/markers/progressive-reserved-12.jpg new file mode 100644 index 0000000000..2ffbc03290 --- /dev/null +++ b/tests/Images/Input/Jpg/markers/progressive-reserved-12.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:fb95b8d19e7e3f51b446b6b801e2f21902dcf4d442a7002b16169c93a051b19e +size 903 diff --git a/tests/Images/Input/Jpg/markers/progressive.jpg b/tests/Images/Input/Jpg/markers/progressive.jpg new file mode 100644 index 0000000000..1eefde2d33 --- /dev/null +++ b/tests/Images/Input/Jpg/markers/progressive.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:c4433ae4df863cf8cc0d5751e34cd64cf33e180551830e5fd1b1b1918064ed11 +size 779 From bf76f1d87c8753a6bafcf4390d645345bf5f9b0b Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 27 Oct 2026 17:22:54 +1000 Subject: [PATCH 3/3] Separate marker fixture setup from assertions --- tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 658b4e2f30..ee5cec5036 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -382,6 +382,7 @@ public void Decode_InvalidQuantizationSegmentAfterTem_RemainsFatal() // TEM precedes DQT in this stored file. DQT declares more bytes than the file contains. // No image data has been read, so there is no safe point for recovery. byte[] data = TestFile.Create(MarkerDirectory + "header-tem-invalid-dqt-length.jpg").Bytes; + foreach (SegmentIntegrityHandling integrity in Enum.GetValues()) { DecoderOptions options = new() { SegmentIntegrityHandling = integrity };