From 555a0d7b67c952e2884b33b685a3f6f21745e618 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 10 Mar 2022 15:25:08 +0100 Subject: [PATCH 1/3] Fix app1 parsing --- .../Formats/Jpeg/JpegDecoderCore.cs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 023928f37d..ef4e3ffac2 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -677,7 +677,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg } /// - /// Processes the App1 marker retrieving any stored metadata + /// Processes the App1 marker retrieving any stored metadata. /// /// The input stream. /// The remaining bytes in the segment block. @@ -687,7 +687,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg const int XmpMarkerLength = 29; if (remaining < ExifMarkerLength || this.IgnoreMetadata) { - // Skip the application header length + // Skip the application header length. stream.Skip(remaining); return; } @@ -697,12 +697,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg JpegThrowHelper.ThrowInvalidImageContentException("Bad App1 Marker length."); } - // XMP marker is the longest, so read at least that many bytes into temp. + // XMP marker is the longer then the EXIF marker, so first try read the EXIF marker bytes. stream.Read(this.temp, 0, ExifMarkerLength); + remaining -= ExifMarkerLength; if (ProfileResolver.IsProfile(this.temp, ProfileResolver.ExifMarker)) { - remaining -= ExifMarkerLength; this.hasExif = true; byte[] profile = new byte[remaining]; stream.Read(profile, 0, remaining); @@ -713,7 +713,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg } else { - // If the EXIF information exceeds 64K, it will be split over multiple APP1 markers + // If the EXIF information exceeds 64K, it will be split over multiple APP1 markers. this.ExtendProfile(ref this.exifData, profile); } @@ -722,9 +722,10 @@ namespace SixLabors.ImageSharp.Formats.Jpeg if (ProfileResolver.IsProfile(this.temp, ProfileResolver.XmpMarker.Slice(0, ExifMarkerLength))) { - stream.Read(this.temp, 0, XmpMarkerLength - ExifMarkerLength); - remaining -= XmpMarkerLength; - if (ProfileResolver.IsProfile(this.temp, ProfileResolver.XmpMarker.Slice(ExifMarkerLength))) + int remainingXmpMarkerBytes = XmpMarkerLength - ExifMarkerLength; + stream.Read(this.temp, ExifMarkerLength, remainingXmpMarkerBytes); + remaining -= remainingXmpMarkerBytes; + if (ProfileResolver.IsProfile(this.temp, ProfileResolver.XmpMarker)) { this.hasXmp = true; byte[] profile = new byte[remaining]; @@ -736,7 +737,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg } else { - // If the XMP information exceeds 64K, it will be split over multiple APP1 markers + // If the XMP information exceeds 64K, it will be split over multiple APP1 markers. this.ExtendProfile(ref this.xmpData, profile); } From 29aa049b714d7c7fe0df90736b0c3e80d4d8c25a Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 10 Mar 2022 15:31:28 +0100 Subject: [PATCH 2/3] Add test for #2057 --- .../Formats/Jpg/JpegDecoderTests.Metadata.cs | 1 - .../Formats/Jpg/JpegDecoderTests.cs | 17 +++++++++++++++-- tests/ImageSharp.Tests/TestImages.cs | 1 + .../Input/Jpg/issues/Issue2057-App1Parsing.jpg | 3 +++ 4 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 tests/Images/Input/Jpg/issues/Issue2057-App1Parsing.jpg diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs index 840cc9f685..ffd58e5b73 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs @@ -4,7 +4,6 @@ using System; using System.IO; using System.Runtime.CompilerServices; -using System.Text; using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Jpeg; using SixLabors.ImageSharp.Metadata; diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 7a24469597..4409f91a02 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -21,7 +21,7 @@ using Xunit.Abstractions; namespace SixLabors.ImageSharp.Tests.Formats.Jpg { // TODO: Scatter test cases into multiple test classes - [Trait("Format", "Jpg")] + [Trait("Format", "Jpg")] public partial class JpegDecoderTests { public const PixelTypes CommonNonDefaultPixelTypes = PixelTypes.Rgba32 | PixelTypes.Argb32 | PixelTypes.Bgr24 | PixelTypes.RgbaVector; @@ -65,7 +65,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg private ITestOutputHelper Output { get; } - private static JpegDecoder JpegDecoder => new JpegDecoder(); + private static JpegDecoder JpegDecoder => new(); [Fact] public void ParseStream_BasicPropertiesAreCorrect() @@ -213,6 +213,19 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg } } + // https://github.com/SixLabors/ImageSharp/issues/2057 + [Theory] + [WithFile(TestImages.Jpeg.Issues.Issue2057App1Parsing, PixelTypes.Rgba32)] + public void Issue2057_DecodeWorks(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using (Image image = provider.GetImage(JpegDecoder)) + { + image.DebugSave(provider); + image.CompareToOriginal(provider); + } + } + // DEBUG ONLY! // The PDF.js output should be saved by "tests\ImageSharp.Tests\Formats\Jpg\pdfjs\jpeg-converter.htm" // into "\tests\Images\ActualOutput\JpegDecoderTests\" diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 5ed0a12f7d..2886b5106d 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -262,6 +262,7 @@ namespace SixLabors.ImageSharp.Tests public const string MalformedUnsupportedComponentCount = "Jpg/issues/issue-1900-malformed-unsupported-255-components.jpg"; public const string MultipleApp01932 = "Jpg/issues/issue-1932-app0-resolution.jpg"; public const string InvalidIptcTag = "Jpg/issues/Issue1942InvalidIptcTag.jpg"; + public const string Issue2057App1Parsing = "Jpg/issues/Issue2057-App1Parsing.jpg"; public static class Fuzz { diff --git a/tests/Images/Input/Jpg/issues/Issue2057-App1Parsing.jpg b/tests/Images/Input/Jpg/issues/Issue2057-App1Parsing.jpg new file mode 100644 index 0000000000..d1ffe4ac91 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/Issue2057-App1Parsing.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:7048dee15946bf981e5b0d2481ffcb8a64684fddca07172275b13a05f01b6b63 +size 1631109 From 1d2d717a50909135ef44e0427dbb30f81fc8424c Mon Sep 17 00:00:00 2001 From: Ynse Hoornenborg Date: Fri, 11 Mar 2022 12:24:01 +0100 Subject: [PATCH 3/3] Fix and test --- .../Metadata/Profiles/Exif/Values/ExifValue.cs | 2 +- .../Formats/Jpg/JpegDecoderTests.Metadata.cs | 13 +++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + .../Input/Jpg/issues/issue-2056-exif-null-array.jpg | 3 +++ 4 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 tests/Images/Input/Jpg/issues/issue-2056-exif-null-array.jpg diff --git a/src/ImageSharp/Metadata/Profiles/Exif/Values/ExifValue.cs b/src/ImageSharp/Metadata/Profiles/Exif/Values/ExifValue.cs index 3143339198..82bd6ad2ec 100644 --- a/src/ImageSharp/Metadata/Profiles/Exif/Values/ExifValue.cs +++ b/src/ImageSharp/Metadata/Profiles/Exif/Values/ExifValue.cs @@ -29,7 +29,7 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Exif { // All array types are value types so Clone() is sufficient here. var array = (Array)other.GetValue(); - this.TrySetValue(array.Clone()); + this.TrySetValue(array?.Clone()); } } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs index 840cc9f685..cd55c98ea9 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs @@ -302,6 +302,19 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg Assert.Null(ex); } + [Theory] + [WithFile(TestImages.Jpeg.Issues.ExifNullArrayTag, PixelTypes.Rgba32)] + public void Clone_WithNullRationalArrayTag_DoesNotThrowException(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + Exception ex = Record.Exception(() => + { + using Image image = provider.GetImage(JpegDecoder); + var clone = image.Metadata.ExifProfile.DeepClone(); + }); + Assert.Null(ex); + } + [Fact] public void EncodedStringTags_WriteAndRead() { diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index aa4314b8e0..39fede9148 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -262,6 +262,7 @@ namespace SixLabors.ImageSharp.Tests public const string MalformedUnsupportedComponentCount = "Jpg/issues/issue-1900-malformed-unsupported-255-components.jpg"; public const string MultipleApp01932 = "Jpg/issues/issue-1932-app0-resolution.jpg"; public const string InvalidIptcTag = "Jpg/issues/Issue1942InvalidIptcTag.jpg"; + public const string ExifNullArrayTag = "Jpg/issues/issue-2056-exif-null-array.jpg"; public static class Fuzz { diff --git a/tests/Images/Input/Jpg/issues/issue-2056-exif-null-array.jpg b/tests/Images/Input/Jpg/issues/issue-2056-exif-null-array.jpg new file mode 100644 index 0000000000..9b5bc8303d --- /dev/null +++ b/tests/Images/Input/Jpg/issues/issue-2056-exif-null-array.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:4c52500be37a8ea1ee1caeb78c79e44b02e10912df4f6db65313c6745573c8ee +size 250451