From 943cc3439398079e7eff22ac124f63339be8ea6e Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Sun, 5 Jun 2022 20:59:10 +0300 Subject: [PATCH 1/2] Fixed --- .../Jpeg/Components/Decoder/JpegBitReader.cs | 7 +-- .../Formats/Jpeg/JpegDecoderCore.cs | 54 +++++++++---------- .../Formats/Jpg/JpegDecoderTests.cs | 15 +++++- tests/ImageSharp.Tests/TestImages.cs | 3 +- ...ssue2136-scan-segment-extraneous-bytes.jpg | 3 ++ 5 files changed, 49 insertions(+), 33 deletions(-) create mode 100644 tests/Images/Input/Jpg/issues/Issue2136-scan-segment-extraneous-bytes.jpg diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs index 84013319e1..70968d5194 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs @@ -155,12 +155,14 @@ private ulong GetBytes() c = this.ReadStream(); } + // Found a marker // We accept multiple FF bytes followed by a 0 as meaning a single FF data byte. - // This data pattern is not valid according to the standard. + // even though it's considered 'invalid' according to the specs. if (c != 0) { - this.Marker = (byte)c; + // It's a trick so we won't read past actual marker this.badData = true; + this.Marker = (byte)c; this.MarkerPosition = this.stream.Position - 2; } } @@ -199,7 +201,6 @@ public bool FindNextMarker() if (b != 0) { this.Marker = (byte)b; - this.badData = true; this.MarkerPosition = this.stream.Position - 2; return true; } diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index a6ccb1b644..73254ace12 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -164,38 +164,38 @@ public JpegDecoderCore(Configuration configuration, IJpegDecoderOptions options) /// /// Finds the next file marker within the byte stream. /// - /// The buffer to read file markers to. /// The input stream. - /// The - public static JpegFileMarker FindNextFileMarker(byte[] marker, BufferedReadStream stream) + /// The . + public static JpegFileMarker FindNextFileMarker(BufferedReadStream stream) { - int value = stream.Read(marker, 0, 2); - - if (value == 0) + while (true) { - return new JpegFileMarker(JpegConstants.Markers.EOI, stream.Length - 2); - } + int b = stream.ReadByte(); + if (b == -1) + { + return new JpegFileMarker(JpegConstants.Markers.EOI, stream.Length - 2); + } - if (marker[0] == JpegConstants.Markers.XFF) - { - // According to Section B.1.1.2: - // "Any marker may optionally be preceded by any number of fill bytes, which are bytes assigned code 0xFF." - int m = marker[1]; - while (m == JpegConstants.Markers.XFF) + // Found a marker. + if (b == JpegConstants.Markers.XFF) { - int suffix = stream.ReadByte(); - if (suffix == -1) + while (b == JpegConstants.Markers.XFF) { - return new JpegFileMarker(JpegConstants.Markers.EOI, stream.Length - 2); + // Loop here to discard any padding FF bytes on terminating marker. + b = stream.ReadByte(); + if (b == -1) + { + return new JpegFileMarker(JpegConstants.Markers.EOI, stream.Length - 2); + } } - m = suffix; + // Found a valid marker. Exit loop + if (b is not 0 and (< JpegConstants.Markers.RST0 or > JpegConstants.Markers.RST7)) + { + return new JpegFileMarker((byte)(uint)b, stream.Position - 2); + } } - - return new JpegFileMarker((byte)m, stream.Position - 2); } - - return new JpegFileMarker(marker[1], stream.Position - 2, true); } /// @@ -331,15 +331,12 @@ internal void ParseStream(BufferedReadStream stream, SpectralConverter spectralC JpegThrowHelper.ThrowInvalidImageContentException("Missing SOI marker."); } - stream.Read(this.markerBuffer, 0, 2); - byte marker = this.markerBuffer[1]; - fileMarker = new JpegFileMarker(marker, (int)stream.Position - 2); + fileMarker = FindNextFileMarker(stream); this.QuantizationTables ??= new Block8x8F[4]; // Break only when we discover a valid EOI marker. // https://github.com/SixLabors/ImageSharp/issues/695 - while (fileMarker.Marker != JpegConstants.Markers.EOI - || (fileMarker.Marker == JpegConstants.Markers.EOI && fileMarker.Invalid)) + while (fileMarker.Marker != JpegConstants.Markers.EOI) { cancellationToken.ThrowIfCancellationRequested(); @@ -491,7 +488,8 @@ internal void ParseStream(BufferedReadStream stream, SpectralConverter spectralC } // Read on. - fileMarker = FindNextFileMarker(this.markerBuffer, stream); + fileMarker = FindNextFileMarker(stream); + Console.WriteLine($"Found marker: {fileMarker.Marker} at {fileMarker.Position}"); } } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index b105677bec..2b24597e35 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -220,7 +220,7 @@ public void Issue2057_DecodeWorks(TestImageProvider provider) // https://github.com/SixLabors/ImageSharp/issues/2133 [Theory] - [WithFile(TestImages.Jpeg.Issues.Issue2133DeduceColorSpace, PixelTypes.Rgba32)] + [WithFile(TestImages.Jpeg.Issues.Issue2133_DeduceColorSpace, PixelTypes.Rgba32)] public void Issue2133_DeduceColorSpace(TestImageProvider provider) where TPixel : unmanaged, IPixel { @@ -231,6 +231,19 @@ public void Issue2133_DeduceColorSpace(TestImageProvider provide } } + // https://github.com/SixLabors/ImageSharp/issues/2133 + [Theory] + [WithFile(TestImages.Jpeg.Issues.Issue2136_ScanMarkerExtraneousBytes, PixelTypes.Rgba32)] + public void Issue2136_DecodeWorks(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using (Image image = provider.GetImage(JpegDecoder)) + { + image.DebugSave(provider); + image.CompareToOriginal(provider); + } + } + // DEBUG ONLY! // The PDF.js output should be saved by "tests\ImageSharp.Tests\Formats\Jpg\pdfjs\jpeg-converter.htm" // into "\tests\Images\ActualOutput\JpegDecoderTests\" diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 403b42b254..ec65e2c65b 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -272,7 +272,8 @@ public static class Issues public const string Issue2057App1Parsing = "Jpg/issues/Issue2057-App1Parsing.jpg"; public const string ExifNullArrayTag = "Jpg/issues/issue-2056-exif-null-array.jpg"; public const string ValidExifArgumentNullExceptionOnEncode = "Jpg/issues/Issue2087-exif-null-reference-on-encode.jpg"; - public const string Issue2133DeduceColorSpace = "Jpg/issues/Issue2133.jpg"; + public const string Issue2133_DeduceColorSpace = "Jpg/issues/Issue2133.jpg"; + public const string Issue2136_ScanMarkerExtraneousBytes = "Jpg/issues/Issue2136-scan-segment-extraneous-bytes.jpg"; public static class Fuzz { diff --git a/tests/Images/Input/Jpg/issues/Issue2136-scan-segment-extraneous-bytes.jpg b/tests/Images/Input/Jpg/issues/Issue2136-scan-segment-extraneous-bytes.jpg new file mode 100644 index 0000000000..c759b93ce6 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/Issue2136-scan-segment-extraneous-bytes.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:b40cd36423a0602515abf411944016cc43169423b31b347953739dee91e15d38 +size 826538 From ef9830fd69b7b03ada0869a8654f5242640105b1 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Sun, 5 Jun 2022 21:18:00 +0300 Subject: [PATCH 2/2] Removed debug console log --- src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 73254ace12..58c85bd34e 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -489,7 +489,6 @@ internal void ParseStream(BufferedReadStream stream, SpectralConverter spectralC // Read on. fileMarker = FindNextFileMarker(stream); - Console.WriteLine($"Found marker: {fileMarker.Marker} at {fileMarker.Position}"); } }