Can now read padded RSTn markers. Fix #481

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,
+                        // Attempt to look for 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)
                         {
                             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++;

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;

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));

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)]

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";

tests/Images/External

@@ -1 +1 @@
-Subproject commit 8714b94dc4bab6788fcbb6254174db2b9c8f69c9
+Subproject commit 653f0c7e3c84657f68dd46e5a380186b3696b956

tests/Images/Input/Jpg/baseline/badrst.jpg

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:af18f0bf30231d2c4c0e6b80d4636237c2851f67763788de12931fd1960c4ff3
+size 74497