diff --git a/src/ImageSharp/Formats/Jpeg/GolangPort/Components/Decoder/InputProcessor.cs b/src/ImageSharp/Formats/Jpeg/GolangPort/Components/Decoder/InputProcessor.cs index fd6a7833a..cb4b63cff 100644 --- a/src/ImageSharp/Formats/Jpeg/GolangPort/Components/Decoder/InputProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/GolangPort/Components/Decoder/InputProcessor.cs @@ -380,5 +380,13 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort.Components.Decoder this.LastErrorCode = this.Bits.ReceiveExtendUnsafe(t, ref this, out x); return this.LastErrorCode; } + + /// + /// Reset the Huffman decoder. + /// + public void ResetHuffmanDecoder() + { + this.Bits = default(Bits); + } } } \ No newline at end of file diff --git a/src/ImageSharp/Formats/Jpeg/GolangPort/Components/Decoder/OrigJpegScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/GolangPort/Components/Decoder/OrigJpegScanDecoder.cs index 67abba9f3..d10def3ce 100644 --- a/src/ImageSharp/Formats/Jpeg/GolangPort/Components/Decoder/OrigJpegScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/GolangPort/Components/Decoder/OrigJpegScanDecoder.cs @@ -94,6 +94,21 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort.Components.Decoder /// private int eobRun; + /// + /// The block counter + /// + private int blockCounter; + + /// + /// The MCU counter + /// + private int mcuCounter; + + /// + /// The expected RST marker value + /// + private byte expectedRst; + /// /// Initializes a default-constructed instance for reading data from -s stream. /// @@ -139,100 +154,136 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort.Components.Decoder { decoder.InputProcessor.ResetErrorState(); - int blockCount = 0; - int mcu = 0; - byte expectedRst = OrigJpegConstants.Markers.RST0; + this.blockCounter = 0; + this.mcuCounter = 0; + this.expectedRst = OrigJpegConstants.Markers.RST0; for (int my = 0; my < decoder.MCUCountY; my++) { for (int mx = 0; mx < decoder.MCUCountX; mx++) { - for (int scanIndex = 0; scanIndex < this.componentScanCount; scanIndex++) - { - this.ComponentIndex = this.pointers.ComponentScan[scanIndex].ComponentIndex; - OrigComponent component = decoder.Components[this.ComponentIndex]; + this.DecodeBlocksAtMcuIndex(decoder, mx, my); - this.hi = component.HorizontalSamplingFactor; - int vi = component.VerticalSamplingFactor; - - for (int j = 0; j < this.hi * vi; j++) - { - if (this.componentScanCount != 1) - { - this.bx = (this.hi * mx) + (j % this.hi); - this.by = (vi * my) + (j / this.hi); - } - else - { - int q = decoder.MCUCountX * this.hi; - this.bx = blockCount % q; - this.by = blockCount / q; - blockCount++; - if (this.bx * 8 >= decoder.ImageWidth || this.by * 8 >= decoder.ImageHeight) - { - continue; - } - } + this.mcuCounter++; - // Find the block at (bx,by) in the component's buffer: - ref Block8x8 blockRefOnHeap = ref component.GetBlockReference(this.bx, this.by); + // Handling restart intervals + // Useful info: https://stackoverflow.com/a/8751802 + if (decoder.IsAtRestartInterval(this.mcuCounter)) + { + this.ProcessRSTMarker(decoder); + this.Reset(decoder); + } + } + } + } - // Copy block to stack - this.data.Block = blockRefOnHeap; + private void DecodeBlocksAtMcuIndex(OrigJpegDecoderCore decoder, int mx, int my) + { + for (int scanIndex = 0; scanIndex < this.componentScanCount; scanIndex++) + { + this.ComponentIndex = this.pointers.ComponentScan[scanIndex].ComponentIndex; + OrigComponent component = decoder.Components[this.ComponentIndex]; - if (!decoder.InputProcessor.ReachedEOF) - { - this.DecodeBlock(decoder, scanIndex); - } + this.hi = component.HorizontalSamplingFactor; + int vi = component.VerticalSamplingFactor; - // Store the result block: - blockRefOnHeap = this.data.Block; + for (int j = 0; j < this.hi * vi; j++) + { + if (this.componentScanCount != 1) + { + this.bx = (this.hi * mx) + (j % this.hi); + this.by = (vi * my) + (j / this.hi); + } + else + { + int q = decoder.MCUCountX * this.hi; + this.bx = this.blockCounter % q; + this.by = this.blockCounter / q; + this.blockCounter++; + if (this.bx * 8 >= decoder.ImageWidth || this.by * 8 >= decoder.ImageHeight) + { + continue; } + } + + // Find the block at (bx,by) in the component's buffer: + ref Block8x8 blockRefOnHeap = ref component.GetBlockReference(this.bx, this.by); - // for j + // Copy block to stack + this.data.Block = blockRefOnHeap; + + if (!decoder.InputProcessor.ReachedEOF) + { + this.DecodeBlock(decoder, scanIndex); } - // for i - mcu++; + // Store the result block: + blockRefOnHeap = this.data.Block; + } + } + } - if (decoder.RestartInterval > 0 && mcu % decoder.RestartInterval == 0 && mcu < decoder.TotalMCUCount) + private void ProcessRSTMarker(OrigJpegDecoderCore decoder) + { + // Attempt to look for RST[0-7] markers to resynchronize from corrupt input. + if (!decoder.InputProcessor.ReachedEOF) + { + decoder.InputProcessor.ReadFullUnsafe(decoder.Temp, 0, 2); + if (decoder.InputProcessor.CheckEOFEnsureNoError()) + { + if (decoder.Temp[0] != 0xFF || decoder.Temp[1] != this.expectedRst) { - // A more sophisticated decoder could use RST[0-7] markers to resynchronize from corrupt input, - // but this one assumes well-formed input, and hence the restart marker follows immediately. - if (!decoder.InputProcessor.ReachedEOF) + bool invalidRst = true; + + // Most jpeg's containing well-formed input will have a RST[0-7] marker following immediately + // but some, see Issue #481, contain padding bytes "0xFF" before the RST[0-7] marker. + // If we identify that case we attempt to read until we have bypassed the padded bytes. + // We then check again for our RST marker and throw if invalid. + // No other methods are attempted to resynchronize from corrupt input. + if (decoder.Temp[0] == 0xFF && decoder.Temp[1] == 0xFF) { - decoder.InputProcessor.ReadFullUnsafe(decoder.Temp, 0, 2); - if (decoder.InputProcessor.CheckEOFEnsureNoError()) + while (decoder.Temp[0] == 0xFF && decoder.InputProcessor.CheckEOFEnsureNoError()) { - if (decoder.Temp[0] != 0xff || decoder.Temp[1] != expectedRst) - { - throw new ImageFormatException("Bad RST marker"); - } - - expectedRst++; - if (expectedRst == OrigJpegConstants.Markers.RST7 + 1) + decoder.InputProcessor.ReadFullUnsafe(decoder.Temp, 0, 1); + if (!decoder.InputProcessor.CheckEOFEnsureNoError()) { - expectedRst = OrigJpegConstants.Markers.RST0; + break; } } - } - // Reset the Huffman decoder. - decoder.InputProcessor.Bits = default(Bits); + // Have we found a valid restart marker? + invalidRst = decoder.Temp[0] != this.expectedRst; + } - // Reset the DC components, as per section F.2.1.3.1. - this.ResetDc(); + if (invalidRst) + { + throw new ImageFormatException("Bad RST marker"); + } + } - // Reset the progressive decoder state, as per section G.1.2.2. - this.eobRun = 0; + this.expectedRst++; + if (this.expectedRst == OrigJpegConstants.Markers.RST7 + 1) + { + this.expectedRst = OrigJpegConstants.Markers.RST0; } } - - // for mx } } - private void ResetDc() + private void Reset(OrigJpegDecoderCore decoder) + { + decoder.InputProcessor.ResetHuffmanDecoder(); + + this.ResetDcValues(); + + // Reset the progressive decoder state, as per section G.1.2.2. + this.eobRun = 0; + } + + /// + /// Reset the DC components, as per section F.2.1.3.1. + /// + private void ResetDcValues() { Unsafe.InitBlock(this.pointers.Dc, default(byte), sizeof(int) * OrigJpegDecoderCore.MaxComponents); } diff --git a/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs index 58513fd29..6cc275d49 100644 --- a/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs @@ -411,6 +411,15 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort this.InitDerivedMetaDataProperties(); } + /// + /// Returns true if 'mcuCounter' is at restart interval + /// + public bool IsAtRestartInterval(int mcuCounter) + { + return this.RestartInterval > 0 && mcuCounter % this.RestartInterval == 0 + && mcuCounter < this.TotalMCUCount; + } + /// /// Assigns derived metadata properties to , eg. horizontal and vertical resolution if it has a JFIF header. /// diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs index 9e245ea2c..c6f6ac270 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs @@ -154,7 +154,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components ushort marker = fileMarker.Marker; - // RSTn - We've alread read the bytes and altered the position so no need to skip + // RSTn - We've already read the bytes and altered the position so no need to skip if (marker >= PdfJsJpegConstants.Markers.RST0 && marker <= PdfJsJpegConstants.Markers.RST7) { continue; diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs index 4fa0bc281..54e2833b1 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs @@ -137,7 +137,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort return new PdfJsFileMarker(PdfJsJpegConstants.Markers.EOI, (int)stream.Length - 2); } - marker[1] = (byte)value; + marker[1] = (byte)suffix; } return new PdfJsFileMarker((ushort)((marker[0] << 8) | marker[1]), (int)(stream.Position - 2)); diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 139fa351b..95ee40e80 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -42,7 +42,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg TestImages.Jpeg.Baseline.Jpeg444, TestImages.Jpeg.Baseline.Bad.BadEOF, TestImages.Jpeg.Issues.MultiHuffmanBaseline394, - TestImages.Jpeg.Baseline.MultiScanBaselineCMYK + TestImages.Jpeg.Baseline.MultiScanBaselineCMYK, + TestImages.Jpeg.Baseline.Bad.BadRST }; public static string[] ProgressiveTestJpegs = @@ -61,6 +62,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg [TestImages.Jpeg.Baseline.Calliphora] = 0.00002f / 100, [TestImages.Jpeg.Baseline.Bad.BadEOF] = 0.38f / 100, [TestImages.Jpeg.Baseline.Testorig420] = 0.38f / 100, + [TestImages.Jpeg.Baseline.Bad.BadRST] = 0.0589f / 100, // Progressive: [TestImages.Jpeg.Issues.MissingFF00ProgressiveGirl159] = 0.34f / 100, @@ -119,7 +121,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg } public const string DecodeBaselineJpegOutputName = "DecodeBaselineJpeg"; - + [Theory] [WithFile(TestImages.Jpeg.Baseline.Calliphora, CommonNonDefaultPixelTypes, false)] [WithFile(TestImages.Jpeg.Baseline.Calliphora, CommonNonDefaultPixelTypes, true)] diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index f1f989581..db469f87e 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -96,6 +96,7 @@ namespace SixLabors.ImageSharp.Tests public static class Bad { public const string BadEOF = "Jpg/baseline/badeof.jpg"; + public const string BadRST = "Jpg/baseline/badrst.jpg"; } public const string Cmyk = "Jpg/baseline/cmyk.jpg"; diff --git a/tests/Images/External b/tests/Images/External index 8714b94dc..653f0c7e3 160000 --- a/tests/Images/External +++ b/tests/Images/External @@ -1 +1 @@ -Subproject commit 8714b94dc4bab6788fcbb6254174db2b9c8f69c9 +Subproject commit 653f0c7e3c84657f68dd46e5a380186b3696b956 diff --git a/tests/Images/Input/Jpg/baseline/badrst.jpg b/tests/Images/Input/Jpg/baseline/badrst.jpg new file mode 100644 index 000000000..61805b42d --- /dev/null +++ b/tests/Images/Input/Jpg/baseline/badrst.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:af18f0bf30231d2c4c0e6b80d4636237c2851f67763788de12931fd1960c4ff3 +size 74497