From 9152ea8f1c2f9fcf0ca596d00deac6a3ad50c6b8 Mon Sep 17 00:00:00 2001
From: CleanCodeDeveloper
 <16760760+CleanCodeDeveloper@users.noreply.github.com>
Date: Fri, 10 Dec 2021 14:54:05 +0100
Subject: [PATCH] [Image Resizer]Try to recover metadata even when the metadata
 data structure is invalid (#14914)

* metadata.Clone() fails also for situations where we still can recover metadata

metadata.Clone() is also an expensive operation (deep copy) and it is not necessary anymore as we build up the metadata object from scratch anyway

* If an exception is throw here something is seriously wrong with the metadata structure

We take all metadata we have read so far an write it to the resized image

* add log statement

* Adjust test written for #2447 as we are able to copy the metadata now

* Improve documentation
---
 .../tests/Models/ResizeOperationTests.cs      |  4 +--
 .../ui/Extensions/BitmapMetadataExtension.cs  | 35 ++++++++++++++++---
 .../imageresizer/ui/Models/ResizeOperation.cs |  7 ++--
 3 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/src/modules/imageresizer/tests/Models/ResizeOperationTests.cs b/src/modules/imageresizer/tests/Models/ResizeOperationTests.cs
index ea22ccd05c..b0d843e361 100644
--- a/src/modules/imageresizer/tests/Models/ResizeOperationTests.cs
+++ b/src/modules/imageresizer/tests/Models/ResizeOperationTests.cs
@@ -33,7 +33,7 @@ namespace ImageResizer.Models
         }
 
         [TestMethod]
-        public void ExecuteCopiesFrameMetadataExceptWhenMetadataCannotBeCloned()
+        public void ExecuteCopiesFrameMetadataEvenWhenMetadataCannotBeCloned()
         {
             var operation = new ResizeOperation("TestMetadataIssue2447.jpg", _directory, Settings());
 
@@ -41,7 +41,7 @@ namespace ImageResizer.Models
 
             AssertEx.Image(
                 _directory.File(),
-                image => Assert.IsNull(((BitmapMetadata)image.Frames[0].Metadata).CameraModel));
+                image => Assert.IsNotNull(((BitmapMetadata)image.Frames[0].Metadata).CameraModel));
         }
 
         [TestMethod]
diff --git a/src/modules/imageresizer/ui/Extensions/BitmapMetadataExtension.cs b/src/modules/imageresizer/ui/Extensions/BitmapMetadataExtension.cs
index d6885c75ee..6bf4b4ace9 100644
--- a/src/modules/imageresizer/ui/Extensions/BitmapMetadataExtension.cs
+++ b/src/modules/imageresizer/ui/Extensions/BitmapMetadataExtension.cs
@@ -105,14 +105,31 @@ namespace ImageResizer.Extensions
         }
 
         /// <summary>
-        /// Gets all metadata
-        /// Iterates recursively through all metadata
+        /// Gets all metadata.
+        /// Iterates recursively through metadata and adds valid items to a list while skipping invalid data items.
         /// </summary>
+        /// <remarks>
+        /// Invalid data items are items which throw an exception when reading the data with metadata.GetQuery(...).
+        /// Sometimes Metadata collections are improper closed and cause an exception on IEnumerator.MoveNext(). In this case, we return all data items which were successfully collected so far.
+        /// </remarks>
+        /// <returns>
+        /// metadata path and metadata value of all successfully read data items.
+        /// </returns>
         public static List<(string metadataPath, object value)> GetListOfMetadata(this BitmapMetadata metadata)
         {
             var listOfAllMetadata = new List<(string metadataPath, object value)>();
 
-            GetMetadataRecursively(metadata, string.Empty);
+            try
+            {
+                GetMetadataRecursively(metadata, string.Empty);
+            }
+#pragma warning disable CA1031 // Do not catch general exception types
+            catch (Exception ex)
+#pragma warning restore CA1031 // Do not catch general exception types
+            {
+                Debug.WriteLine($"Exception while trying to iterate recursively over metadata. We were able to read {listOfAllMetadata.Count} metadata entries.");
+                Debug.WriteLine(ex);
+            }
 
             return listOfAllMetadata;
 
@@ -202,7 +219,17 @@ namespace ImageResizer.Extensions
         {
             var listOfAllMetadata = new List<(string metadataPath, object value)>();
 
-            GetMetadataRecursively(metadata, string.Empty);
+            try
+            {
+                GetMetadataRecursively(metadata, string.Empty);
+            }
+#pragma warning disable CA1031 // Do not catch general exception types
+            catch (Exception ex)
+#pragma warning restore CA1031 // Do not catch general exception types
+            {
+                Debug.WriteLine($"Exception while trying to iterate recursively over metadata. We were able to read {listOfAllMetadata.Count} metadata entries.");
+                Debug.WriteLine(ex);
+            }
 
             return listOfAllMetadata;
 
diff --git a/src/modules/imageresizer/ui/Models/ResizeOperation.cs b/src/modules/imageresizer/ui/Models/ResizeOperation.cs
index cf12b033f3..daf5b3137b 100644
--- a/src/modules/imageresizer/ui/Models/ResizeOperation.cs
+++ b/src/modules/imageresizer/ui/Models/ResizeOperation.cs
@@ -83,17 +83,14 @@ namespace ImageResizer.Models
                     {
                         try
                         {
-                            // Detect whether metadata can copied successfully
-                            var modifiableMetadata = metadata.Clone();
-
 #if DEBUG
                             Debug.WriteLine($"### Processing metadata of file {_file}");
-                            modifiableMetadata.PrintsAllMetadataToDebugOutput();
+                            metadata.PrintsAllMetadataToDebugOutput();
 #endif
 
                             // read all metadata and build up metadata object from the scratch. Discard invalid (unreadable/unwritable) metadata.
                             var newMetadata = new BitmapMetadata(metadata.Format);
-                            var listOfMetadata = modifiableMetadata.GetListOfMetadata();
+                            var listOfMetadata = metadata.GetListOfMetadata();
                             foreach (var (metadataPath, value) in listOfMetadata)
                             {
                                 if (value is BitmapMetadata bitmapMetadata)