From 3c2885e382e9c5057d4e059ca6da4bdadb01f8a8 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 23 Jan 2023 23:13:53 +1000 Subject: [PATCH 1/3] Allow zero DPI and only throw at EOF when not enough data --- .../Jpeg/Components/Decoder/JFifMarker.cs | 22 ++++--------------- .../Components/Decoder/SpectralConverter.cs | 6 +++++ .../Decoder/SpectralConverter{TPixel}.cs | 6 +++++ .../Formats/Jpeg/JpegDecoderCore.cs | 12 ++++++++++ .../Formats/Jpg/JpegDecoderTests.cs | 11 ++++++++++ .../Formats/Jpg/SpectralJpegTests.cs | 2 ++ tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Jpg/issues/issue-2315.jpg | 3 +++ 8 files changed, 45 insertions(+), 18 deletions(-) create mode 100644 tests/Images/Input/Jpg/issues/issue-2315.jpg diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JFifMarker.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JFifMarker.cs index 42dcf7200a..e13b26f9a9 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JFifMarker.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JFifMarker.cs @@ -26,16 +26,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder; /// The vertical pixel density. 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; } /// - /// Gets the horizontal pixel density. Must not be zero. + /// Gets the horizontal pixel density. /// public short XDensity { get; } /// - /// Gets the vertical pixel density. Must not be zero. + /// Gets the vertical pixel density. /// 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; diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter.cs index 0eb484b94a..51d9bfbced 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter.cs @@ -121,4 +121,10 @@ public static Size CalculateResultingImageSize(Size size, Size? targetSize, out return size; } + + /// + /// Gets a value indicating whether the converter has a pixel buffer. + /// + /// if the converter has a pixel buffer; otherwise, . + public abstract bool HasPixelBuffer(); } diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs index 4bb58d8a12..d6250127f5 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs @@ -85,6 +85,12 @@ public SpectralConverter(Configuration configuration, Size? targetSize = null) /// public Configuration Configuration { get; } + /// + /// Gets a value indicating whether the converter has a pixel buffer. + /// + /// if the converter has a pixel buffer; otherwise, . + public override bool HasPixelBuffer() => this.pixelBuffer is not null; + /// /// Gets converted pixel buffer. /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 511ec1ba1d..51de9ffbd1 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/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); } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 1d5a7e0ff8..425d12497c 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -300,4 +300,15 @@ public void Issue2136_DecodeWorks(TestImageProvider 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(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(JpegDecoder.Instance); + image.DebugSave(provider); + image.CompareToOriginal(provider); + } } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/SpectralJpegTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/SpectralJpegTests.cs index 58347a0724..805ee586a8 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/SpectralJpegTests.cs +++ b/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; diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 29223b1fe0..22a48ebee4 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/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 { diff --git a/tests/Images/Input/Jpg/issues/issue-2315.jpg b/tests/Images/Input/Jpg/issues/issue-2315.jpg new file mode 100644 index 0000000000..fe3001eddc --- /dev/null +++ b/tests/Images/Input/Jpg/issues/issue-2315.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:f129b057efb499d492e9afcffdd98de62aac1e04b97a09a75b4799ba498cd3c1 +size 319056 From c44cd6c244f4671fdb8b4c7fd426afc865e55084 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 23 Jan 2023 23:26:15 +1000 Subject: [PATCH 2/3] Update DecodeJpegParseStreamOnly.cs --- .../Codecs/Jpeg/DecodeJpegParseStreamOnly.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DecodeJpegParseStreamOnly.cs b/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DecodeJpegParseStreamOnly.cs index aa88242ce4..b5a7245292 100644 --- a/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DecodeJpegParseStreamOnly.cs +++ b/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) { } From c224dece1765026ca0da5086af88e71ed1fb30fc Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 24 Jan 2023 08:57:39 +1000 Subject: [PATCH 3/3] Update JFifMarkerTests.cs --- tests/ImageSharp.Tests/Formats/Jpg/JFifMarkerTests.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JFifMarkerTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JFifMarkerTests.cs index 3890c20e93..3b7e20eb4e 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JFifMarkerTests.cs +++ b/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() {