started implementing mixed error management in JpegDecoderCore

src/ImageSharp.Formats.Jpeg/Components/Decoder/Bits.cs

@@ -39,11 +39,11 @@ namespace ImageSharp.Formats.Jpg
         /// <param name="decoder">Jpeg decoder</param>
         /// <returns>Error code</returns>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        internal JpegDecoderCore.ErrorCodes EnsureNBits(int n, JpegDecoderCore decoder)
+        internal DecoderErrorCode EnsureNBits(int n, JpegDecoderCore decoder)
         {
             while (true)
             {
-                JpegDecoderCore.ErrorCodes errorCode;
+                DecoderErrorCode errorCode;
 
                 // Grab the decode bytes, use them and then set them
                 // back on the decoder.
@@ -51,7 +51,7 @@ namespace ImageSharp.Formats.Jpg
                 byte c = decoderBytes.ReadByteStuffedByte(decoder.InputStream, out errorCode);
                 decoder.Bytes = decoderBytes;
 
-                if (errorCode != JpegDecoderCore.ErrorCodes.NoError)
+                if (errorCode != DecoderErrorCode.NoError)
                 {
                     return errorCode;
                 }
@@ -69,7 +69,7 @@ namespace ImageSharp.Formats.Jpg
 
                 if (this.UnreadBits >= n)
                 {
-                    return JpegDecoderCore.ErrorCodes.NoError;
+                    return DecoderErrorCode.NoError;
                 }
             }
         }
@@ -84,11 +84,8 @@ namespace ImageSharp.Formats.Jpg
         {
             if (this.UnreadBits < t)
             {
-                JpegDecoderCore.ErrorCodes errorCode = this.EnsureNBits(t, decoder);
+                DecoderErrorCode errorCode = this.EnsureNBits(t, decoder);
-                if (errorCode != JpegDecoderCore.ErrorCodes.NoError)
+                errorCode.EnsureNoError();
-                {
-                    throw new JpegDecoderCore.MissingFF00Exception();
-                }
             }
 
             this.UnreadBits -= t;

src/ImageSharp.Formats.Jpeg/Components/Decoder/Bytes.cs

@@ -69,11 +69,11 @@ namespace ImageSharp.Formats.Jpg
         /// <param name="inputStream">Input stream</param>
         /// <param name="errorCode">Error code</param>
         /// <returns>The <see cref="byte"/></returns>
-        internal byte ReadByteStuffedByte(Stream inputStream, out JpegDecoderCore.ErrorCodes errorCode)
+        internal byte ReadByteStuffedByte(Stream inputStream, out DecoderErrorCode errorCode)
         {
             byte x;
 
-            errorCode = JpegDecoderCore.ErrorCodes.NoError;
+            errorCode = DecoderErrorCode.NoError;
 
             // Take the fast path if bytes.buf contains at least two bytes.
             if (this.I + 2 <= this.J)
@@ -88,7 +88,7 @@ namespace ImageSharp.Formats.Jpg
 
                 if (this.Buffer[this.I] != 0x00)
                 {
-                    errorCode = JpegDecoderCore.ErrorCodes.MissingFF00;
+                    errorCode = DecoderErrorCode.MissingFF00;
                     return 0;
 
                     // throw new MissingFF00Exception();
@@ -112,7 +112,7 @@ namespace ImageSharp.Formats.Jpg
             this.UnreadableBytes = 2;
             if (x != 0x00)
             {
-                errorCode = JpegDecoderCore.ErrorCodes.MissingFF00;
+                errorCode = DecoderErrorCode.MissingFF00;
                 return 0;
 
                 // throw new MissingFF00Exception();
@@ -167,7 +167,7 @@ namespace ImageSharp.Formats.Jpg
             int n = inputStream.Read(this.Buffer, this.J, this.Buffer.Length - this.J);
             if (n == 0)
             {
-                throw new JpegDecoderCore.EOFException();
+                throw new EOFException();
             }
 
             this.J += n;

src/ImageSharp.Formats.Jpeg/Components/Decoder/DecoderErrorCode.cs

@@ -0,0 +1,29 @@
+// <copyright file="DecoderErrorCode.cs" company="James Jackson-South">
+// Copyright (c) James Jackson-South and contributors.
+// Licensed under the Apache License, Version 2.0.
+// </copyright>
+
+namespace ImageSharp.Formats
+{
+    /// <summary>
+    /// Represents "recoverable" decoder errors.
+    /// </summary>
+    internal enum DecoderErrorCode
+    {
+        /// <summary>
+        /// NoError
+        /// </summary>
+        NoError,
+
+        /// <summary>
+        /// MissingFF00
+        /// </summary>
+        // ReSharper disable once InconsistentNaming
+        MissingFF00,
+
+        /// <summary>
+        /// End of stream reached unexpectedly
+        /// </summary>
+        UnexpectedEndOfFile
+    }
+}

src/ImageSharp.Formats.Jpeg/Components/Decoder/DecoderThrowHelper.cs

@@ -0,0 +1,49 @@
+// <copyright file="DecoderThrowHelper.cs" company="James Jackson-South">
+// Copyright (c) James Jackson-South and contributors.
+// Licensed under the Apache License, Version 2.0.
+// </copyright>
+
+namespace ImageSharp.Formats.Jpg
+{
+    using System;
+    using System.Runtime.CompilerServices;
+
+    /// <summary>
+    /// Encapsulates exception thrower methods for the Jpeg Encoder
+    /// </summary>
+    internal static class DecoderThrowHelper
+    {
+        /// <summary>
+        /// Throws an exception that belongs to the given <see cref="DecoderErrorCode"/>
+        /// </summary>
+        /// <param name="errorCode">The <see cref="DecoderErrorCode"/></param>
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        public static void ThrowExceptionForErrorCode(this DecoderErrorCode errorCode)
+        {
+            switch (errorCode)
+            {
+                case DecoderErrorCode.NoError:
+                    throw new ArgumentException("ThrowExceptionForErrorCode() called with NoError!", nameof(errorCode));
+                case DecoderErrorCode.MissingFF00:
+                    throw new MissingFF00Exception();
+                case DecoderErrorCode.UnexpectedEndOfFile:
+                    throw new EOFException();
+                default:
+                    throw new ArgumentOutOfRangeException(nameof(errorCode), errorCode, null);
+            }
+        }
+
+        /// <summary>
+        /// Throws an exception if the given <see cref="DecoderErrorCode"/> defines an error.
+        /// </summary>
+        /// <param name="errorCode">The <see cref="DecoderErrorCode"/></param>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static void EnsureNoError(this DecoderErrorCode errorCode)
+        {
+            if (errorCode != DecoderErrorCode.NoError)
+            {
+                ThrowExceptionForErrorCode(errorCode);
+            }
+        }
+    }
+}

src/ImageSharp.Formats.Jpeg/Components/Decoder/EOFException.cs

@@ -0,0 +1,21 @@
+// <copyright file="EOFException.cs" company="James Jackson-South">
+// Copyright (c) James Jackson-South and contributors.
+// Licensed under the Apache License, Version 2.0.
+// </copyright>
+
+namespace ImageSharp.Formats.Jpg
+{
+    using System;
+
+    /// <summary>
+    /// The EOF (End of File exception).
+    /// Thrown when the decoder encounters an EOF marker without a proceeding EOI (End Of Image) marker
+    /// </summary>
+    internal class EOFException : Exception
+    {
+        public EOFException()
+            : base("Reached end of stream before proceeding EOI marker!")
+        {
+        }
+    }
+}

src/ImageSharp.Formats.Jpeg/Components/Decoder/JpegScanDecoder.cs

@@ -127,9 +127,9 @@ namespace ImageSharp.Formats.Jpg
             {
                 for (int mx = 0; mx < decoder.MCUCountX; mx++)
                 {
-                    for (int i = 0; i < this.componentScanCount; i++)
+                    for (int scanIndex = 0; scanIndex < this.componentScanCount; scanIndex++)
                     {
-                        this.componentIndex = this.pointers.ComponentScan[i].ComponentIndex;
+                        this.componentIndex = this.pointers.ComponentScan[scanIndex].ComponentIndex;
                         this.hi = decoder.ComponentArray[this.componentIndex].HorizontalFactor;
                         int vi = decoder.ComponentArray[this.componentIndex].VerticalFactor;
 
@@ -190,7 +190,7 @@ namespace ImageSharp.Formats.Jpg
                                 this.data.Block.Clear();
                             }
 
-                            this.ProcessBlockImpl(decoder, i);
+                            this.ProcessBlockImpl(decoder, scanIndex);
                         }
 
                         // for j
@@ -204,6 +204,7 @@ namespace ImageSharp.Formats.Jpg
                         // 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.
                         decoder.ReadFull(decoder.Temp, 0, 2);
+
                         if (decoder.Temp[0] != 0xff || decoder.Temp[1] != expectedRst)
                         {
                             throw new ImageFormatException("Bad RST marker");
@@ -306,12 +307,12 @@ namespace ImageSharp.Formats.Jpg
         /// Process the current block at (<see cref="bx"/>, <see cref="by"/>)
         /// </summary>
         /// <param name="decoder">The decoder</param>
-        /// <param name="i">The index of the scan</param>
+        /// <param name="scanIndex">The index of the scan</param>
-        private void ProcessBlockImpl(JpegDecoderCore decoder, int i)
+        private void ProcessBlockImpl(JpegDecoderCore decoder, int scanIndex)
         {
             var b = this.pointers.Block;
 
-            int huffmannIdx = (AcTableIndex * HuffmanTree.ThRowSize) + this.pointers.ComponentScan[i].AcTableSelector;
+            int huffmannIdx = (AcTableIndex * HuffmanTree.ThRowSize) + this.pointers.ComponentScan[scanIndex].AcTableSelector;
             if (this.ah != 0)
             {
                 this.Refine(decoder, ref decoder.HuffmanTrees[huffmannIdx], 1 << this.al);
@@ -319,6 +320,7 @@ namespace ImageSharp.Formats.Jpg
             else
             {
                 int zig = this.zigStart;
+                DecoderErrorCode errorCode;
                 if (zig == 0)
                 {
                     zig++;
@@ -326,13 +328,15 @@ namespace ImageSharp.Formats.Jpg
                     // Decode the DC coefficient, as specified in section F.2.2.1.
                     byte value =
                         decoder.DecodeHuffman(
-                            ref decoder.HuffmanTrees[(DcTableIndex * HuffmanTree.ThRowSize) + this.pointers.ComponentScan[i].DcTableSelector]);
+                            ref decoder.HuffmanTrees[(DcTableIndex * HuffmanTree.ThRowSize) + this.pointers.ComponentScan[scanIndex].DcTableSelector]);
                     if (value > 16)
                     {
                         throw new ImageFormatException("Excessive DC component");
                     }
 
                     int deltaDC = decoder.Bits.ReceiveExtend(value, decoder);
+                    // errorCode.EnsureNoError();
+
                     this.pointers.Dc[this.componentIndex] += deltaDC;
 
                     // b[0] = dc[compIndex] << al;
@@ -360,6 +364,7 @@ namespace ImageSharp.Formats.Jpg
                             }
 
                             int ac = decoder.Bits.ReceiveExtend(val1, decoder);
+                            // errorCode.EnsureNoError();
 
                             // b[Unzig[zig]] = ac << al;
                             Block8x8F.SetScalarAt(b, this.pointers.Unzig[zig], ac << this.al);

src/ImageSharp.Formats.Jpeg/Components/Decoder/MissingFF00Exception.cs

@@ -0,0 +1,17 @@
+// <copyright file="MissingFF00Exception.cs" company="James Jackson-South">
+// Copyright (c) James Jackson-South and contributors.
+// Licensed under the Apache License, Version 2.0.
+// </copyright>
+
+namespace ImageSharp.Formats.Jpg
+{
+    using System;
+
+    /// <summary>
+    /// The missing ff00 exception.
+    /// </summary>
+    // ReSharper disable once InconsistentNaming
+    internal class MissingFF00Exception : Exception
+    {
+    }
+}

src/ImageSharp.Formats.Jpeg/JpegDecoderCore.cs

@@ -93,24 +93,6 @@ namespace ImageSharp.Formats
             this.Bytes = Bytes.Create();
         }
 
-        /// <summary>
-        /// ReadByteStuffedByte was throwing exceptions on normal execution path (very inefficent)
-        /// It's better tho have an error code for this!
-        /// </summary>
-        internal enum ErrorCodes
-        {
-            /// <summary>
-            /// NoError
-            /// </summary>
-            NoError,
-
-            /// <summary>
-            /// MissingFF00
-            /// </summary>
-            // ReSharper disable once InconsistentNaming
-            MissingFF00
-        }
-
         /// <summary>
         /// Gets the component array
         /// </summary>
@@ -437,7 +419,8 @@ namespace ImageSharp.Formats
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public byte ReadByte()
         {
-            return this.Bytes.ReadByte(this.InputStream);
+            byte result = this.Bytes.ReadByte(this.InputStream);
+            return result;
         }
 
         /// <summary>
@@ -448,11 +431,8 @@ namespace ImageSharp.Formats
         {
             if (this.Bits.UnreadBits == 0)
             {
-                ErrorCodes errorCode = this.Bits.EnsureNBits(1, this);
+                DecoderErrorCode errorCode = this.Bits.EnsureNBits(1, this);
-                if (errorCode != ErrorCodes.NoError)
+                errorCode.EnsureNoError();
-                {
-                    throw new MissingFF00Exception();
-                }
             }
 
             bool ret = (this.Bits.Accumulator & this.Bits.Mask) != 0;
@@ -467,7 +447,22 @@ namespace ImageSharp.Formats
         /// <param name="data">The data to write to.</param>
         /// <param name="offset">The offset in the source buffer</param>
         /// <param name="length">The number of bytes to read</param>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public void ReadFull(byte[] data, int offset, int length)
+        {
+            DecoderErrorCode errorCode = this.ReadFullUnsafe(data, offset, length);
+            errorCode.EnsureNoError();
+        }
+
+        /// <summary>
+        /// Reads exactly length bytes into data. It does not care about byte stuffing.
+        /// Does not throw on errors, returns <see cref="JpegDecoderCore"/> instead!
+        /// </summary>
+        /// <param name="data">The data to write to.</param>
+        /// <param name="offset">The offset in the source buffer</param>
+        /// <param name="length">The number of bytes to read</param>
+        /// <returns>The <see cref="DecoderErrorCode"/></returns>
+        public DecoderErrorCode ReadFullUnsafe(byte[] data, int offset, int length)
         {
             // Unread the overshot bytes, if any.
             if (this.Bytes.UnreadableBytes != 0)
@@ -498,7 +493,9 @@ namespace ImageSharp.Formats
                     this.Bytes.Fill(this.InputStream);
                 }
             }
-        }
+
+            return DecoderErrorCode.NoError;
+         }
 
         /// <summary>
         /// Decodes the given number of bits
@@ -509,11 +506,8 @@ namespace ImageSharp.Formats
         {
             if (this.Bits.UnreadBits < count)
             {
-                ErrorCodes errorCode = this.Bits.EnsureNBits(count, this);
+                DecoderErrorCode errorCode = this.Bits.EnsureNBits(count, this);
-                if (errorCode != ErrorCodes.NoError)
+                errorCode.EnsureNoError();
-                {
-                    throw new MissingFF00Exception();
-                }
             }
 
             uint ret = this.Bits.Accumulator >> (this.Bits.UnreadBits - count);
@@ -538,9 +532,9 @@ namespace ImageSharp.Formats
 
             if (this.Bits.UnreadBits < 8)
             {
-                ErrorCodes errorCode = this.Bits.EnsureNBits(8, this);
+                DecoderErrorCode errorCode = this.Bits.EnsureNBits(8, this);
 
-                if (errorCode == ErrorCodes.NoError)
+                if (errorCode == DecoderErrorCode.NoError)
                 {
                     ushort v = huffmanTree.Lut[(this.Bits.Accumulator >> (this.Bits.UnreadBits - HuffmanTree.LutSize)) & 0xFF];
 
@@ -552,6 +546,10 @@ namespace ImageSharp.Formats
                         return (byte)(v >> 8);
                     }
                 }
+                else if (errorCode == DecoderErrorCode.UnexpectedEndOfFile)
+                {
+                    errorCode.ThrowExceptionForErrorCode();
+                }
                 else
                 {
                     this.UnreadByteStuffedByte();
@@ -563,11 +561,8 @@ namespace ImageSharp.Formats
             {
                 if (this.Bits.UnreadBits == 0)
                 {
-                    ErrorCodes errorCode = this.Bits.EnsureNBits(1, this);
+                    DecoderErrorCode errorCode = this.Bits.EnsureNBits(1, this);
-                    if (errorCode != ErrorCodes.NoError)
+                    errorCode.EnsureNoError();
-                    {
-                        throw new MissingFF00Exception();
-                    }
                 }
 
                 if ((this.Bits.Accumulator & this.Bits.Mask) != 0)
@@ -1440,7 +1435,20 @@ namespace ImageSharp.Formats
         /// Skips the next n bytes.
         /// </summary>
         /// <param name="count">The number of bytes to ignore.</param>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private void Skip(int count)
+        {
+            DecoderErrorCode errorCode = this.SkipUnsafe(count);
+            errorCode.EnsureNoError();
+        }
+
+        /// <summary>
+        /// Skips the next n bytes.
+        /// Does not throw, returns <see cref="DecoderErrorCode"/> instead!
+        /// </summary>
+        /// <param name="count">The number of bytes to ignore.</param>
+        /// <returns>The <see cref="DecoderErrorCode"/></returns>
+        private DecoderErrorCode SkipUnsafe(int count)
         {
             // Unread the overshot bytes, if any.
             if (this.Bytes.UnreadableBytes != 0)
@@ -1470,6 +1478,8 @@ namespace ImageSharp.Formats
 
                 this.Bytes.Fill(this.InputStream);
             }
+
+            return DecoderErrorCode.NoError;
         }
 
         /// <summary>
@@ -1490,21 +1500,5 @@ namespace ImageSharp.Formats
                 this.Bits.Mask >>= 8;
             }
         }
-
-        /// <summary>
-        /// The EOF (End of File exception).
-        /// Thrown when the decoder encounters an EOF marker without a proceeding EOI (End Of Image) marker
-        /// </summary>
-        internal class EOFException : Exception
-        {
-        }
-
-        /// <summary>
-        /// The missing ff00 exception.
-        /// </summary>
-        // ReSharper disable once InconsistentNaming
-        internal class MissingFF00Exception : Exception
-        {
-        }
     }
-}
+}

tests/ImageSharp.Tests/Formats/Jpg/BadEofJpegTests.cs

@@ -36,7 +36,7 @@ namespace ImageSharp.Tests
             provider.Utility.SaveTestOutputFile(image, "bmp");
         }
 
-        // [Theory] // TODO: #18
+        [Theory] // TODO: #18
         [WithFile(TestImages.Jpeg.Progressive.Bad.BadEOF, PixelTypes.Color)]
         public void LoadProgressiveImage<TColor>(TestImageProvider<TColor> provider)
             where TColor : struct, IPackedPixel, IEquatable<TColor>