From 382ee7fb2351979a194684707491a88a22a9070b Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 19 Mar 2024 00:30:20 +1000 Subject: [PATCH 1/3] Only exit after multiple EOF hits --- .../Jpeg/Components/Decoder/JpegBitReader.cs | 20 +++++++++++++++---- .../Formats/Jpg/JpegDecoderTests.cs | 11 ++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Jpg/issues/Issue2638.jpg | 3 +++ 4 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 tests/Images/Input/Jpg/issues/Issue2638.jpg diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs index 0877dbc922..c2b0cb6e07 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs @@ -22,6 +22,9 @@ internal struct JpegBitReader // Whether there is no more good data to pull from the stream for the current mcu. private bool badData; + // How many times have we hit the eof. + private int eofHitCount; + public JpegBitReader(BufferedReadStream stream) { this.stream = stream; @@ -31,6 +34,7 @@ public JpegBitReader(BufferedReadStream stream) this.MarkerPosition = 0; this.badData = false; this.NoData = false; + this.eofHitCount = 0; } /// @@ -80,6 +84,9 @@ public void Reset() [MethodImpl(InliningOptions.ShortMethod)] public bool HasBadMarker() => this.Marker != JpegConstants.Markers.XFF && !this.HasRestartMarker(); + [MethodImpl(InliningOptions.ShortMethod)] + public bool HasEndMarker() => this.Marker == JpegConstants.Markers.EOI; + [MethodImpl(InliningOptions.AlwaysInline)] public void FillBuffer() { @@ -219,11 +226,16 @@ private int ReadStream() // we know we have hit the EOI and completed decoding the scan buffer. if (value == -1 || (this.badData && this.data == 0 && this.stream.Position >= this.stream.Length)) { - // We've encountered the end of the file stream which means there's no EOI marker + // We've passed the end of the file stream which means there's no EOI marker // in the image or the SOS marker has the wrong dimensions set. - this.badData = true; - this.NoData = true; - value = 0; + if (this.eofHitCount > JpegConstants.Huffman.FetchLoop) + { + this.badData = true; + this.NoData = true; + value = 0; + } + + this.eofHitCount++; } return value; diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index c8d93f6e9e..6a94a98ac6 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -364,4 +364,15 @@ public void Issue2517_DecodeWorks(TestImageProvider provider) image.DebugSave(provider); image.CompareToOriginal(provider); } + + // https://github.com/SixLabors/ImageSharp/issues/2638 + [Theory] + [WithFile(TestImages.Jpeg.Issues.Issue2638, PixelTypes.Rgba32)] + public void Issue2638_DecodeWorks(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(JpegDecoder.Instance); + image.DebugSave(provider); + image.CompareToOriginal(provider); + } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 6be8ff6a68..5da581e52f 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -316,6 +316,7 @@ public static class Issues public const string HangBadScan = "Jpg/issues/Hang_C438A851.jpg"; public const string Issue2517 = "Jpg/issues/issue2517-bad-d7.jpg"; public const string Issue2067_CommentMarker = "Jpg/issues/issue-2067-comment.jpg"; + public const string Issue2638 = "Jpg/issues/Issue2638.jpg"; public static class Fuzz { diff --git a/tests/Images/Input/Jpg/issues/Issue2638.jpg b/tests/Images/Input/Jpg/issues/Issue2638.jpg new file mode 100644 index 0000000000..f42d67b0e8 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/Issue2638.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:208d5b0b727bbef120a7e090e020a48f99c9e264c2d3939ba749f8620853c1fe +size 70876 From 45bc54ac4486ada9c0fe3b9ec35f33f23c174ce5 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 19 Mar 2024 00:31:57 +1000 Subject: [PATCH 2/3] Remove unused code. --- .../Formats/Jpeg/Components/Decoder/JpegBitReader.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs index c2b0cb6e07..babd2ff4df 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs @@ -84,9 +84,6 @@ public void Reset() [MethodImpl(InliningOptions.ShortMethod)] public bool HasBadMarker() => this.Marker != JpegConstants.Markers.XFF && !this.HasRestartMarker(); - [MethodImpl(InliningOptions.ShortMethod)] - public bool HasEndMarker() => this.Marker == JpegConstants.Markers.EOI; - [MethodImpl(InliningOptions.AlwaysInline)] public void FillBuffer() { From 8acecdaf3d1d7796c606667f856f79111f2506bb Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 19 Mar 2024 00:35:23 +1000 Subject: [PATCH 3/3] Update comment --- src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs index babd2ff4df..e71d86a1d9 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs @@ -223,7 +223,7 @@ private int ReadStream() // we know we have hit the EOI and completed decoding the scan buffer. if (value == -1 || (this.badData && this.data == 0 && this.stream.Position >= this.stream.Length)) { - // We've passed the end of the file stream which means there's no EOI marker + // We've hit the end of the file stream more times than allowed which means there's no EOI marker // in the image or the SOS marker has the wrong dimensions set. if (this.eofHitCount > JpegConstants.Huffman.FetchLoop) {