From 37d8c479237182372551b4a97f63d7c2e18f25dc Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 2 Mar 2018 17:19:51 +1100 Subject: [PATCH] Can now read padded RSTn markers. Fix #481 --- .../Components/Decoder/OrigJpegScanDecoder.cs | 28 ++++++++++++++++--- .../PdfJsPort/Components/PdfJsScanDecoder.cs | 2 +- .../Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs | 2 +- .../Formats/Jpg/JpegDecoderTests.cs | 6 ++-- tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/External | 2 +- tests/Images/Input/Jpg/baseline/badrst.jpg | 3 ++ 7 files changed, 35 insertions(+), 9 deletions(-) create mode 100644 tests/Images/Input/Jpg/baseline/badrst.jpg diff --git a/src/ImageSharp/Formats/Jpeg/GolangPort/Components/Decoder/OrigJpegScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/GolangPort/Components/Decoder/OrigJpegScanDecoder.cs index 67abba9f33..7d58d168a6 100644 --- a/src/ImageSharp/Formats/Jpeg/GolangPort/Components/Decoder/OrigJpegScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/GolangPort/Components/Decoder/OrigJpegScanDecoder.cs @@ -197,16 +197,36 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort.Components.Decoder if (decoder.RestartInterval > 0 && mcu % decoder.RestartInterval == 0 && mcu < decoder.TotalMCUCount) { - // 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. + // 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] != expectedRst) + if (decoder.Temp[0] != 0xFF || decoder.Temp[1] != expectedRst) { - throw new ImageFormatException("Bad RST marker"); + 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) + { + while (decoder.Temp[0] == 0xFF && decoder.InputProcessor.CheckEOFEnsureNoError()) + { + decoder.InputProcessor.ReadFullUnsafe(decoder.Temp, 0, 1); + } + + // Have we found a valid restart marker? + invalidRst = decoder.Temp[0] != expectedRst; + } + + if (invalidRst) + { + throw new ImageFormatException("Bad RST marker"); + } } expectedRst++; diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs index 9e245ea2c6..c6f6ac270f 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 4fa0bc281d..54e2833b11 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 139fa351bb..95ee40e807 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 f1f989581f..db469f87e1 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 8714b94dc4..653f0c7e3c 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 0000000000..61805b42d3 --- /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