Merge pull request #2324 from SixLabors/js/2315

Allow zero DPI and only throw at EOF when not enough data

Author: JimBobSquarePants

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

@@ -26,16 +26,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder;
     /// <param name="yDensity">The vertical pixel density.</param>
     private JFifMarker(byte majorVersion, byte minorVersion, byte densityUnits, short xDensity, short yDensity)
     {
-        if (xDensity <= 0)
-        {
-            JpegThrowHelper.ThrowInvalidImageContentException($"X-Density {xDensity} must be greater than 0.");
-        }
-
-        if (yDensity <= 0)
-        {
-            JpegThrowHelper.ThrowInvalidImageContentException($"Y-Density {yDensity} must be greater than 0.");
-        }
-
         this.MajorVersion = majorVersion;
         this.MinorVersion = minorVersion;
 
@@ -64,12 +54,12 @@ private JFifMarker(byte majorVersion, byte minorVersion, byte densityUnits, shor
     public PixelResolutionUnit DensityUnits { get; }
 
     /// <summary>
-    /// Gets the horizontal pixel density. Must not be zero.
+    /// Gets the horizontal pixel density.
     /// </summary>
     public short XDensity { get; }
 
     /// <summary>
-    /// Gets the vertical pixel density. Must not be zero.
+    /// Gets the vertical pixel density.
     /// </summary>
     public short YDensity { get; }
 
@@ -88,12 +78,8 @@ public static bool TryParse(byte[] bytes, out JFifMarker marker)
             byte densityUnits = bytes[7];
             short xDensity = (short)((bytes[8] << 8) | bytes[9]);
             short yDensity = (short)((bytes[10] << 8) | bytes[11]);
-
-            if (xDensity > 0 && yDensity > 0)
-            {
-                marker = new JFifMarker(majorVersion, minorVersion, densityUnits, xDensity, yDensity);
-                return true;
-            }
+            marker = new JFifMarker(majorVersion, minorVersion, densityUnits, xDensity, yDensity);
+            return true;
         }
 
         marker = default;

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

@@ -121,4 +121,10 @@ public static Size CalculateResultingImageSize(Size size, Size? targetSize, out
 
         return size;
     }
+
+    /// <summary>
+    /// Gets a value indicating whether the converter has a pixel buffer.
+    /// </summary>
+    /// <returns><see langword="true"/> if the converter has a pixel buffer; otherwise, <see langword="false"/>.</returns>
+    public abstract bool HasPixelBuffer();
 }

src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs

@@ -85,6 +85,12 @@ public SpectralConverter(Configuration configuration, Size? targetSize = null)
     /// </summary>
     public Configuration Configuration { get; }
 
+    /// <summary>
+    /// Gets a value indicating whether the converter has a pixel buffer.
+    /// </summary>
+    /// <returns><see langword="true"/> if the converter has a pixel buffer; otherwise, <see langword="false"/>.</returns>
+    public override bool HasPixelBuffer() => this.pixelBuffer is not null;
+
     /// <summary>
     /// Gets converted pixel buffer.
     /// </summary>

src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs

@@ -356,6 +356,18 @@ internal void ParseStream(BufferedReadStream stream, SpectralConverter spectralC
                 // to uint to avoid sign extension.
                 if (stream.RemainingBytes < (uint)markerContentByteSize)
                 {
+                    if (metadataOnly && this.Metadata != null && this.Frame != null)
+                    {
+                        // We have enough data to decode the image, so we can stop parsing.
+                        return;
+                    }
+
+                    if (this.Metadata != null && this.Frame != null && spectralConverter.HasPixelBuffer())
+                    {
+                        // We have enough data to decode the image, so we can stop parsing.
+                        return;
+                    }
+
                     JpegThrowHelper.ThrowNotEnoughBytesForMarker(fileMarker.Marker);
                 }
 

tests/ImageSharp.Benchmarks/Codecs/Jpeg/DecodeJpegParseStreamOnly.cs

@@ -54,6 +54,8 @@ public override void ConvertStrideBaseline()
         {
         }
 
+        public override bool HasPixelBuffer() => throw new NotImplementedException();
+
         public override void InjectFrameData(JpegFrame frame, IRawJpegData jpegData)
         {
         }

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

@@ -46,15 +46,6 @@ public void MarkerIgnoresIncorrectValue()
         Assert.Equal(default, marker);
     }
 
-    [Fact]
-    public void MarkerIgnoresCorrectHeaderButInvalidDensities()
-    {
-        bool isJFif = JFifMarker.TryParse(this.bytes3, out JFifMarker marker);
-
-        Assert.False(isJFif);
-        Assert.Equal(default, marker);
-    }
-
     [Fact]
     public void MarkerEqualityIsCorrect()
     {

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

@@ -300,4 +300,15 @@ public void Issue2136_DecodeWorks<TPixel>(TestImageProvider<TPixel> provider)
         image.DebugSave(provider);
         image.CompareToOriginal(provider);
     }
+
+    // https://github.com/SixLabors/ImageSharp/issues/2315
+    [Theory]
+    [WithFile(TestImages.Jpeg.Issues.Issue2315_NotEnoughBytes, PixelTypes.Rgba32)]
+    public void Issue2315_DecodeWorks<TPixel>(TestImageProvider<TPixel> provider)
+        where TPixel : unmanaged, IPixel<TPixel>
+    {
+        using Image<TPixel> image = provider.GetImage(JpegDecoder.Instance);
+        image.DebugSave(provider);
+        image.CompareToOriginal(provider);
+    }
 }

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

@@ -187,6 +187,8 @@ public override void ConvertStrideBaseline()
             }
         }
 
+        public override bool HasPixelBuffer() => throw new NotImplementedException();
+
         public override void InjectFrameData(JpegFrame frame, IRawJpegData jpegData)
         {
             this.frame = frame;

tests/ImageSharp.Tests/TestImages.cs

@@ -281,6 +281,7 @@ public static class Issues
             public const string ValidExifArgumentNullExceptionOnEncode = "Jpg/issues/Issue2087-exif-null-reference-on-encode.jpg";
             public const string Issue2133_DeduceColorSpace = "Jpg/issues/Issue2133.jpg";
             public const string Issue2136_ScanMarkerExtraneousBytes = "Jpg/issues/Issue2136-scan-segment-extraneous-bytes.jpg";
+            public const string Issue2315_NotEnoughBytes = "Jpg/issues/issue-2315.jpg";
 
             public static class Fuzz
             {