From d48a6dd5d1986b4d215ab6b9a92b6dfa79d9dd39 Mon Sep 17 00:00:00 2001 From: Ildar Khayrutdinov Date: Sun, 26 Jan 2025 20:08:26 +0300 Subject: [PATCH 1/4] fix for loading multiple sub ifds Fixes #2857 --- .../Metadata/Profiles/Exif/ExifReader.cs | 13 +++++- .../Formats/Jpg/JpegDecoderTests.Metadata.cs | 45 ++++++++++++++++++- tests/ImageSharp.Tests/TestImages.cs | 1 + .../Jpg/issues/issue-2857-subsub-ifds.jpg | 3 ++ 4 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 tests/Images/Input/Jpg/issues/issue-2857-subsub-ifds.jpg diff --git a/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs b/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs index a4fcd9275b..034ada0b8b 100644 --- a/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs +++ b/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs @@ -189,10 +189,19 @@ protected void ReadSubIfd(List values) { if (this.subIfds is not null) { - foreach (ulong subIfdOffset in this.subIfds) + do { - this.ReadValues(values, (uint)subIfdOffset); + int sz = this.subIfds.Count; + Span buf = sz <= 256 ? stackalloc ulong[sz] : new ulong[sz]; + + this.subIfds.CopyTo(buf); + this.subIfds.Clear(); + foreach (ulong subIfdOffset in buf) + { + this.ReadValues(values, (uint)subIfdOffset); + } } + while (this.subIfds.Count > 0); } } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs index a803372537..27fbaf4aee 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs @@ -10,6 +10,7 @@ using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Tests.TestUtilities; +using static SixLabors.ImageSharp.Metadata.Profiles.Exif.EncodedString; // ReSharper disable InconsistentNaming namespace SixLabors.ImageSharp.Tests.Formats.Jpg; @@ -439,9 +440,8 @@ public void JpegDecoder_DecodeMetadataComment(TestImageProvider Assert.Equal(expectedComment, metadata.Comments.ElementAtOrDefault(0).ToString()); image.DebugSave(provider); image.CompareToOriginal(provider); - } - + // https://github.com/SixLabors/ImageSharp/issues/2758 [Theory] [WithFile(TestImages.Jpeg.Issues.Issue2758, PixelTypes.L8)] @@ -468,6 +468,47 @@ public void Issue2758_DecodeWorks(TestImageProvider provider) image.Save(ms, new JpegEncoder()); } + // https://github.com/SixLabors/ImageSharp/issues/2857 + [Theory] + [WithFile(TestImages.Jpeg.Issues.Issue2857, PixelTypes.Rgb24)] + public void Issue2857_SubSubIfds(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(JpegDecoder.Instance); + + Assert.Equal(5616, image.Width); + Assert.Equal(3744, image.Height); + + JpegMetadata meta = image.Metadata.GetJpegMetadata(); + Assert.Equal(92, meta.LuminanceQuality); + Assert.Equal(93, meta.ChrominanceQuality); + + ExifProfile exifProfile = image.Metadata.ExifProfile; + Assert.NotNull(exifProfile); + + using MemoryStream ms = new(); + bool hasThumbnail = exifProfile.TryCreateThumbnail(out _); + Assert.False(hasThumbnail); + + Assert.Equal("BilderBox - Erwin Wodicka / wodicka@aon.at", exifProfile.GetValue(ExifTag.Copyright).Value); + Assert.Equal("Adobe Photoshop CS3 Windows", exifProfile.GetValue(ExifTag.Software).Value); + + Assert.Equal("Carers; seniors; caregiver; senior care; retirement home; hands; old; elderly; elderly caregiver; elder care; elderly care; geriatric care; nursing home; age; old age care; outpatient; needy; health care; home nurse; home care; sick; retirement; medical; mobile; the elderly; nursing department; nursing treatment; nursing; care services; nursing services; nursing care; nursing allowance; nursing homes; home nursing; care category; nursing class; care; nursing shortage; nursing patient care staff\0", exifProfile.GetValue(ExifTag.XPKeywords).Value); + + Assert.Equal( + new EncodedString(CharacterCode.ASCII, "StockSubmitter|Miscellaneous||Miscellaneous$|00|0000330000000110000000000000000|22$@NA_1005010.460@145$$@Miscellaneous.Miscellaneous$$@$@26$$@$@$@$@205$@$@$@$@$@$@$@$@$@43$@$@$@$$@Miscellaneous.Miscellaneous$$@90$$@22$@$@$@$@$@$@$|||"), + exifProfile.GetValue(ExifTag.UserComment).Value); + + image.Mutate(x => x.Crop(new(0, 0, 100, 100))); + + image.Save(ms, new JpegEncoder()); + + foreach (IExifValue val in image.Metadata.ExifProfile.Values) + { + this.Output.WriteLine($"{val.Tag}={val.GetValue()}"); + } + } + private static void VerifyEncodedStrings(ExifProfile exif) { Assert.NotNull(exif); diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 4130474b58..f4f00c8fcc 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -325,6 +325,7 @@ public static class Issues public const string Issue2067_CommentMarker = "Jpg/issues/issue-2067-comment.jpg"; public const string Issue2638 = "Jpg/issues/Issue2638.jpg"; public const string Issue2758 = "Jpg/issues/issue-2758.jpg"; + public const string Issue2857 = "Jpg/issues/issue-2857-subsub-ifds.jpg"; public static class Fuzz { diff --git a/tests/Images/Input/Jpg/issues/issue-2857-subsub-ifds.jpg b/tests/Images/Input/Jpg/issues/issue-2857-subsub-ifds.jpg new file mode 100644 index 0000000000..5e5288f22e --- /dev/null +++ b/tests/Images/Input/Jpg/issues/issue-2857-subsub-ifds.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:ab45137ded01a42658aa94421165a358b184a536b6ab64427d8255e8d78c25b9 +size 2829977 From 8de08ba72ac519d3174ca72df69a81ee2bd5cb0d Mon Sep 17 00:00:00 2001 From: Ildar Khayrutdinov Date: Sun, 26 Jan 2025 21:57:55 +0300 Subject: [PATCH 2/4] fix for duplicates skipping --- src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs | 5 +++-- .../Formats/Jpg/JpegDecoderTests.Metadata.cs | 11 ++++------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs b/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs index 034ada0b8b..e9683d615c 100644 --- a/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs +++ b/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs @@ -490,8 +490,9 @@ private void Add(IList values, ExifValue exif, object? value) foreach (IExifValue val in values) { - // Sometimes duplicates appear, can compare val.Tag == exif.Tag - if (val == exif) + // to skip duplicates must be used Equals method, + // == operator not defined for ExifValue and IExifValue + if (exif.Equals(val)) { Debug.WriteLine($"Duplicate Exif tag: tag={exif.Tag}, dataType={exif.DataType}"); return; diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs index 27fbaf4aee..20a9ad3877 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs @@ -10,7 +10,6 @@ using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Tests.TestUtilities; -using static SixLabors.ImageSharp.Metadata.Profiles.Exif.EncodedString; // ReSharper disable InconsistentNaming namespace SixLabors.ImageSharp.Tests.Formats.Jpg; @@ -496,17 +495,15 @@ public void Issue2857_SubSubIfds(TestImageProvider provider) Assert.Equal("Carers; seniors; caregiver; senior care; retirement home; hands; old; elderly; elderly caregiver; elder care; elderly care; geriatric care; nursing home; age; old age care; outpatient; needy; health care; home nurse; home care; sick; retirement; medical; mobile; the elderly; nursing department; nursing treatment; nursing; care services; nursing services; nursing care; nursing allowance; nursing homes; home nursing; care category; nursing class; care; nursing shortage; nursing patient care staff\0", exifProfile.GetValue(ExifTag.XPKeywords).Value); Assert.Equal( - new EncodedString(CharacterCode.ASCII, "StockSubmitter|Miscellaneous||Miscellaneous$|00|0000330000000110000000000000000|22$@NA_1005010.460@145$$@Miscellaneous.Miscellaneous$$@$@26$$@$@$@$@205$@$@$@$@$@$@$@$@$@43$@$@$@$$@Miscellaneous.Miscellaneous$$@90$$@22$@$@$@$@$@$@$|||"), + new EncodedString(EncodedString.CharacterCode.ASCII, "StockSubmitter|Miscellaneous||Miscellaneous$|00|0000330000000110000000000000000|22$@NA_1005010.460@145$$@Miscellaneous.Miscellaneous$$@$@26$$@$@$@$@205$@$@$@$@$@$@$@$@$@43$@$@$@$$@Miscellaneous.Miscellaneous$$@90$$@22$@$@$@$@$@$@$|||"), exifProfile.GetValue(ExifTag.UserComment).Value); + // the profile contains 4 duplicated UserComment + Assert.Equal(1, exifProfile.Values.Count(t => t.Tag == ExifTag.UserComment)); + image.Mutate(x => x.Crop(new(0, 0, 100, 100))); image.Save(ms, new JpegEncoder()); - - foreach (IExifValue val in image.Metadata.ExifProfile.Values) - { - this.Output.WriteLine($"{val.Tag}={val.GetValue()}"); - } } private static void VerifyEncodedStrings(ExifProfile exif) From 40b7be804173c56ba8eb5ea2f3b3c305caea3df2 Mon Sep 17 00:00:00 2001 From: Ildar Khayrutdinov Date: Mon, 27 Jan 2025 08:09:59 +0300 Subject: [PATCH 3/4] fix allocation (CA2014) --- src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs b/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs index e9683d615c..1c8cf227c0 100644 --- a/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs +++ b/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs @@ -191,10 +191,7 @@ protected void ReadSubIfd(List values) { do { - int sz = this.subIfds.Count; - Span buf = sz <= 256 ? stackalloc ulong[sz] : new ulong[sz]; - - this.subIfds.CopyTo(buf); + ulong[] buf = [.. this.subIfds]; this.subIfds.Clear(); foreach (ulong subIfdOffset in buf) { @@ -456,6 +453,7 @@ private void ReadValue64(List values, Span offsetBuffer) ExifTagValue.TileByteCounts => new ExifLong8Array(ExifTagValue.TileByteCounts), _ => ExifValues.Create(tag) ?? ExifValues.Create(tag, dataType, numberOfComponents), }; + if (exifValue is null) { this.AddInvalidTag(new UnkownExifTag(tag)); From 14aeb3f7c78e2a981718de8c8448750eddfb5f50 Mon Sep 17 00:00:00 2001 From: Ildar Khayrutdinov Date: Sun, 13 Apr 2025 20:14:30 +0300 Subject: [PATCH 4/4] limit for iteration & bufSz --- .../Metadata/Profiles/Exif/ExifReader.cs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs b/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs index 1c8cf227c0..3af4eb3c39 100644 --- a/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs +++ b/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs @@ -7,6 +7,7 @@ using System.Diagnostics; using System.Globalization; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Text; using SixLabors.ImageSharp.Memory; @@ -187,18 +188,22 @@ protected void ReadValues(List values, uint offset) protected void ReadSubIfd(List values) { - if (this.subIfds is not null) + if (this.subIfds != null) { - do + const int maxSubIfds = 8; + const int maxNestingLevel = 8; + Span buf = stackalloc ulong[maxSubIfds]; + for (int i = 0; i < maxNestingLevel && this.subIfds.Count > 0; i++) { - ulong[] buf = [.. this.subIfds]; + int sz = Math.Min(this.subIfds.Count, maxSubIfds); + CollectionsMarshal.AsSpan(this.subIfds)[..sz].CopyTo(buf); + this.subIfds.Clear(); - foreach (ulong subIfdOffset in buf) + foreach (ulong subIfdOffset in buf[..sz]) { this.ReadValues(values, (uint)subIfdOffset); } } - while (this.subIfds.Count > 0); } }