Merge pull request #2869 from IldarKhayrutdinov/subsub-ifd

Nested sub ifds parsing fix

src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs

@@ -7,6 +7,7 @@ using System.Collections.ObjectModel;
 using System.Diagnostics;
 using System.Globalization;
 using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
 using System.Text;
 using SixLabors.ImageSharp.Memory;
 
@@ -187,11 +188,21 @@ internal abstract class BaseExifReader
 
     protected void ReadSubIfd(List<IExifValue> values)
     {
-        if (this.subIfds is not null)
+        if (this.subIfds != null)
         {
-            foreach (ulong subIfdOffset in this.subIfds)
+            const int maxSubIfds = 8;
+            const int maxNestingLevel = 8;
+            Span<ulong> buf = stackalloc ulong[maxSubIfds];
+            for (int i = 0; i < maxNestingLevel && this.subIfds.Count > 0; i++)
             {
-                this.ReadValues(values, (uint)subIfdOffset);
+                int sz = Math.Min(this.subIfds.Count, maxSubIfds);
+                CollectionsMarshal.AsSpan(this.subIfds)[..sz].CopyTo(buf);
+
+                this.subIfds.Clear();
+                foreach (ulong subIfdOffset in buf[..sz])
+                {
+                    this.ReadValues(values, (uint)subIfdOffset);
+                }
             }
         }
     }
@@ -447,6 +458,7 @@ internal abstract class BaseExifReader
             ExifTagValue.TileByteCounts => new ExifLong8Array(ExifTagValue.TileByteCounts),
             _ => ExifValues.Create(tag) ?? ExifValues.Create(tag, dataType, numberOfComponents),
         };
+
         if (exifValue is null)
         {
             this.AddInvalidTag(new UnkownExifTag(tag));
@@ -481,8 +493,9 @@ internal abstract class BaseExifReader
 
         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;

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

@@ -439,9 +439,8 @@ public partial class JpegDecoderTests
         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 +467,45 @@ public partial class JpegDecoderTests
         image.Save(ms, new JpegEncoder());
     }
 
+    // https://github.com/SixLabors/ImageSharp/issues/2857
+    [Theory]
+    [WithFile(TestImages.Jpeg.Issues.Issue2857, PixelTypes.Rgb24)]
+    public void Issue2857_SubSubIfds<TPixel>(TestImageProvider<TPixel> provider)
+        where TPixel : unmanaged, IPixel<TPixel>
+    {
+        using Image<TPixel> 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(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());
+    }
+
     private static void VerifyEncodedStrings(ExifProfile exif)
     {
         Assert.NotNull(exif);

tests/ImageSharp.Tests/TestImages.cs

@@ -325,6 +325,7 @@ public static class TestImages
             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
             {

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