Fix an issue when reading images in multithreading and where the stream get disposed too soon.

Author: onizet

src/Html2OpenXml/IO/ImagePrefetcher.cs

@@ -52,7 +52,7 @@ sealed class ImagePrefetcher<T> : IImageLoader
     private readonly T hostingPart;
     private readonly IWebRequest resourceLoader;
     private readonly HtmlImageInfoCollection prefetchedImages;
-    private readonly object lockObject = new object();
+    private readonly object lockObject = new();
     private readonly ImageProcessingMode processingMode;
 
 
@@ -68,7 +68,7 @@ public ImagePrefetcher(T hostingPart, IWebRequest resourceLoader, ImageProcessin
         this.hostingPart = hostingPart;
         this.resourceLoader = resourceLoader;
         this.processingMode = processingMode;
-        this.prefetchedImages = new HtmlImageInfoCollection();
+        this.prefetchedImages = [];
     }
 
     //____________________________________________________________________
@@ -133,65 +133,38 @@ public ImagePrefetcher(T hostingPart, IWebRequest resourceLoader, ImageProcessin
     /// </summary>
     private async Task<HtmlImageInfo?> DownloadRemoteImage(string src, CancellationToken cancellationToken)
     {
-        Uri imageUri = new Uri(src, UriKind.RelativeOrAbsolute);
+        Uri imageUri = new(src, UriKind.RelativeOrAbsolute);
         if (imageUri.IsAbsoluteUri && !resourceLoader.SupportsProtocol(imageUri.Scheme))
             return null;
 
-        Resource? response;
-
-        response = await resourceLoader.FetchAsync(imageUri, cancellationToken).ConfigureAwait(false);
+        using var response = await resourceLoader.FetchAsync(imageUri, cancellationToken).ConfigureAwait(false);
         if (response?.Content == null)
             return null;
 
-        using (response)
+        // Copy the stream into a MemoryStream to avoid ObjectDisposedException
+        using var buffer = new MemoryStream();
+        response.Content.CopyTo(buffer);
+        buffer.Seek(0, SeekOrigin.Begin);
+
+        // For requested url with no filename, we need to read the media mime type if provided
+        response.Headers.TryGetValue("Content-Type", out var mime);
+        if (!TryInspectMimeType(mime, out PartTypeInfo type)
+            && !TryGuessTypeFromUri(imageUri, out type)
+            && !TryGuessTypeFromStream(buffer, out type)
+            )
         {
-            // For requested url with no filename, we need to read the media mime type if provided
-            response.Headers.TryGetValue("Content-Type", out var mime);
-            if (!TryInspectMimeType(mime, out PartTypeInfo type)
-                && !TryGuessTypeFromUri(imageUri, out type)
-                && !TryGuessTypeFromStream(response.Content, out type))
-            {
-                return null;
-            }
-
-            // Generate a unique GUID-based relationship ID for the image part
-            string relationshipId = "img_" + Guid.NewGuid().ToString("N");
-            
-            // Synchronize access to AddImagePart to prevent concurrent modifications to the OpenXml document
-            ImagePart ipart;
-            lock (lockObject)
-            {
-                ipart = hostingPart.AddImagePart(type, relationshipId);
-            }
-            
-            Size originalSize;
-            using (var outputStream = ipart.GetStream(FileMode.Create))
-            {
-                response.Content.CopyTo(outputStream);
-
-                outputStream.Seek(0L, SeekOrigin.Begin);
-                originalSize = GetImageSize(outputStream);
-            }
-
-            string partId;
-            lock (lockObject)
-            {
-                partId = hostingPart.GetIdOfPart(ipart);
-            }
-
-            return new HtmlImageInfo(src, partId) {
-                TypeInfo = type,
-                Size = originalSize
-            };
+            return null;
         }
+
+        return SaveImageAssert(src, type, buffer.CopyTo);
     }
 
     /// <summary>
     /// Create an external relationship to an image without downloading it.
     /// </summary>
     private HtmlImageInfo? CreateExternalImageLink(string src)
     {
-        Uri imageUri = new Uri(src, UriKind.RelativeOrAbsolute);
+        Uri imageUri = new(src, UriKind.RelativeOrAbsolute);
 
         // Resolve relative URIs if possible (only for DefaultWebRequest which has BaseImageUrl)
         if (!imageUri.IsAbsoluteUri && resourceLoader is DefaultWebRequest defaultWebRequest
@@ -220,8 +193,7 @@ public ImagePrefetcher(T hostingPart, IWebRequest resourceLoader, ImageProcessin
 
         // Return image info with external flag set
         // Note: Size will be empty as we don't download the image
-        return new HtmlImageInfo(src, relationshipId)
-        {
+        return new HtmlImageInfo(src, relationshipId) {
             IsExternal = true,
             Size = Size.Empty,
             TypeInfo = ImagePartType.Png // Default type, actual type doesn't matter for external links
@@ -235,39 +207,37 @@ public ImagePrefetcher(T hostingPart, IWebRequest resourceLoader, ImageProcessin
     {
         if (DataUri.TryCreate(src, out var dataUri))
         {
-            Size originalSize;
             knownContentType.TryGetValue(dataUri!.Mime, out PartTypeInfo type);
-            // Generate a unique GUID-based relationship ID for the image part
-            string relationshipId = "img_" + Guid.NewGuid().ToString("N");
-            
-            // Synchronize access to AddImagePart to prevent concurrent modifications to the OpenXml document
-            ImagePart ipart;
-            lock (lockObject)
-            {
-                ipart = hostingPart.AddImagePart(type, relationshipId);
-            }
-            
-            using (var outputStream = ipart.GetStream(FileMode.Create))
-            {
-                outputStream.Write(dataUri.Data, 0, dataUri.Data.Length);
 
-                outputStream.Seek(0L, SeekOrigin.Begin);
-                originalSize = GetImageSize(outputStream);
-            }
+            return SaveImageAssert(src, type, stream => stream.Write(dataUri.Data, 0, dataUri.Data.Length));
+        }
 
-            string partId;
-            lock (lockObject)
-            {
-                partId = hostingPart.GetIdOfPart(ipart);
-            }
+        return null;
+    }
 
-            return new HtmlImageInfo(src, partId) {
-                TypeInfo = type,
-                Size = originalSize
-            };
+    private HtmlImageInfo SaveImageAssert(string src, PartTypeInfo type, Action<Stream> writeImage)
+    {
+        ImagePart ipart;
+        string relationshipId = "img_" + Guid.NewGuid().ToString("N");
+        lock (lockObject)
+        {
+            ipart = hostingPart.AddImagePart(type, relationshipId);
         }
 
-        return null;
+        Size originalSize;
+        using (var outputStream = ipart.GetStream(FileMode.Create))
+        {
+            writeImage(outputStream);
+            outputStream.Seek(0L, SeekOrigin.Begin);
+            originalSize = GetImageSize(outputStream);
+        }
+
+        string partId = hostingPart.GetIdOfPart(ipart);
+        return new HtmlImageInfo(src, partId)
+        {
+            TypeInfo = type,
+            Size = originalSize
+        };
     }
 
     //____________________________________________________________________

test/HtmlToOpenXml.Tests/ImgTests.cs

@@ -422,6 +422,37 @@ await converter.ParseBody(
             Assert.That(mainPart.ImageParts.Count(), Is.EqualTo(1));
         }
 
+        [Test(Description = "Prevent concurrent access to the OpenXml mainPart")]
+        public async Task MultipleImgInTag_ReturnsTwoDistinctImage()
+        {
+            var webRequest = new Mock<IO.IWebRequest>();
+            webRequest.Setup(x => x.SupportsProtocol(It.IsAny<string>())).Returns(true);
+            webRequest.Setup(x => x.FetchAsync(It.IsAny<Uri>(), It.IsAny<CancellationToken>()))
+                .Returns(Task.FromResult<IO.Resource?>(new() {
+                    Content = new MemoryStream(Convert.FromBase64String(@"/9j/4AAQSkZJRgABAQAAAQABAAD/4gKgSUNDX1BST0ZJTEUAAQEAAAKQbGNtcwQwAABtbnRyUkdCIFhZWiAH4QAHAAEAAAABAAZhY3NwQVBQTAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA9tYAAQAAAADTLWxjbXMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAtkZXNjAAABCAAAADhjcHJ0AAABQAAAAE53dHB0AAABkAAAABRjaGFkAAABpAAAACxyWFlaAAAB0AAAABRiWFlaAAAB5AAAABRnWFlaAAAB+AAAABRyVFJDAAACDAAAACBnVFJDAAACLAAAACBiVFJDAAACTAAAACBjaHJtAAACbAAAACRtbHVjAAAAAAAAAAEAAAAMZW5VUwAAABwAAAAcAHMAUgBHAEIAIABiAHUAaQBsAHQALQBpAG4AAG1sdWMAAAAAAAAAAQAAAAxlblVTAAAAMgAAABwATgBvACAAYwBvAHAAeQByAGkAZwBoAHQALAAgAHUAcwBlACAAZgByAGUAZQBsAHkAAAAAWFlaIAAAAAAAAPbWAAEAAAAA0y1zZjMyAAAAAAABDEoAAAXj///zKgAAB5sAAP2H///7ov///aMAAAPYAADAlFhZWiAAAAAAAABvlAAAOO4AAAOQWFlaIAAAAAAAACSdAAAPgwAAtr5YWVogAAAAAAAAYqUAALeQAAAY3nBhcmEAAAAAAAMAAAACZmYAAPKnAAANWQAAE9AAAApbcGFyYQAAAAAAAwAAAAJmZgAA8qcAAA1ZAAAT0AAACltwYXJhAAAAAAADAAAAAmZmAADypwAADVkAABPQAAAKW2Nocm0AAAAAAAMAAAAAo9cAAFR7AABMzQAAmZoAACZmAAAPXP/bAEMABQMEBAQDBQQEBAUFBQYHDAgHBwcHDwsLCQwRDxISEQ8RERMWHBcTFBoVEREYIRgaHR0fHx8TFyIkIh4kHB4fHv/bAEMBBQUFBwYHDggIDh4UERQeHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHv/CABEIAX4BfgMBIgACEQEDEQH/xAAcAAEBAAIDAQEAAAAAAAAAAAAAAQcIBAUGAgP/xAAUAQEAAAAAAAAAAAAAAAAAAAAA/9oADAMBAAIQAxAAAAHMoAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADxmKzYPj6q8c22/fUTuzaFh3KZzwAAAAAAAAAAAAAAAAAAflhJjUlAlIonZ9ZTZH1epuyB6IAAAAAAAAAAAAAAAADxvstezwv1BYFlhYFIX1PlRt19+O9iAAAAAAAAAAAAAAAAfOpu0+px9AlCUIsFCKMrZmwFns+gAAAAAAAAAAAAAAAfhqXt3rAdDYKgsCoKQqDI2dcUZYKAAAAAAAAAAAAAAABiPLnENTna9UShKEUATk8fJ5kn0nH5AAAAAAAAAAAAAAAAAB5jXbbDzxrHfWeSFQWBeTkg6LOTtSgAAAAAAAAAAAAAAAAAA/Pyvrhhvp89/Jgv0OU/o873fIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAH//EACYQAAEEAQQBBAMBAAAAAAAAAAQBAgMFBgARMFASFCAxkBMVIiP/2gAIAQEAAQUC+xZXI3XqhtMmif1lvkINep+TWpOpZppdbaT+VDtrERazMF3EKgLh6Z7msbkeSSkr7684qvnoLiG1g6XMrlSJuEQiYQijso7ILo8ssf19Xx4xYOr7Ni+TeizYv1N1x7axUz1dV0Ll2QuVZy+TAZtk6E534wm/HJhDlQ5PjoJm+cPirV5MIZ/u346HIhlEuuTCh1YF0Wfg7t4xYXEEVECRRdEVBGTBbASVx3FilarUgZ4M6O/qYrQU0WcMjgoKhxDgoPBOlt6wWyhtqA0F3tGHmJfT4+jHCCozqHsa5LCjCL0TibNPxYnePF36ExoVijVzY2xxNYnV+Ka8U+xj/8QAFBEBAAAAAAAAAAAAAAAAAAAAkP/aAAgBAwEBPwEQP//EABQRAQAAAAAAAAAAAAAAAAAAAJD/2gAIAQIBAT8BED//xAA7EAABAgIGBAoJBQEAAAAAAAABAgMAEQQSITFBUCIwUXEFEBMjMlJhYpHBIDNCU3KQobHhFDSBgpLR/9oACAEBAAY/AvmLaRA3x+4Z/wBiNFxB3HLC3W5d7qN4bzBDbgoyNjYt8THOvOOnvKnF30iYmD2RzNMdl1VGY8IqcIsWe8aH3EB2jupcQcU5OVKIAFpMGjcHrU2z7TgvXu2ajlqK6UHEYK3xZoPJ6beTK4PoypMoscI9s7N2qQ+wsoWk2Ql5Nirlp2HJDyapPu6DfZtPFdqk28y7oL8jE8jU0OhRxUG+861lSjNQFVW8ZFOHnyfWLUrxOtpDPereI/GRPr6raj9NXjxvDuD75EtHWEoqm8WcV3oXem+v4RkdKawr1huNutC/erreQyNrhBA6Og55axthF6zKEpSJJQJDI1sOiaFiRhdFdwtSqXSG3V/qnEmusaPYmJZJUVouJ9WuV34g0ekIqrH13akUikIk17KZdP8AEVjfk1R9EzgrEQSlJfa6yRb/ACPSqMNKWezCA5SpOrwTgP8AsTN+UWiJrYTPaLD4xzTzqd9sWUlEvgjTpR/qiBWbW8e+fKAkJSlIwAlFgyy6LvmMf//EACkQAAIBAQcEAQUBAAAAAAAAAAABESExQVFhcZGhUIGx8OEQQMHR8ZD/2gAIAQEAAT8h/wA710aRVOqD0j8nAesp0ufpLhst7S5eQ8aTLVnWnNQO5zpvy2RwbP0T8iHAjOKvu+ptBFqSdopqvw+xYb1Zw8Hk+jpKtIcJK+pYmhQeYnLyQlFElt8FlqJImzwX/BfVF1CzGsqpYLf+Jo0NjWPWdVmsU+jNgsSzo26HL0Ikj2GU9RGa5Ix8EZ8M9sIzQlrsSBzPD7p4p4FtH1tb9aX6dEbWBzP0Vy0JRSB5CvqGpu4KwWXfRqbuCq/RTDyVHC2oosVXwPgShehva3BqXob+OxGuxuQ89jcinwVIyexuJR/Dc3KrUy0AOdroN97ehIc1xkgExGa+DQ0Njvwd+D2z6aMpA/o6QVF4DF3QaI/gGYlNXYvBGFexGTIz4Y8yJvWwlqR7BrJF8kRaIvkXcUOYcmqDTLoK2ENNbuoGzVVbUsaoyrdTRNivqIk7HZsRlwZEUEqWIrgivqHuFT9jEhV0FjQpp2p5DzBGRDuIIuggh4EQRAkQQ8CMUMUqs7FH7BWdCdpu7m633cdy8g9sKE3/AE9sL5JpaXECCKVHFive1SPsOmlC+zf2Cjb9eQrJ6UbZa/jIIzZKIvngjEiHayUQrZVMiNSFbJZ+ErVcbV+NRKk+0X2FDU91TfB4tgMk5eiXNr08S+3wb/TRLZFt3gri/e5ZUlJWIe6Rb1ZmvT0FwvRlEVsqPxT9QiZLKYtWe6lCh2NPv8kMrd5I9n5IfrOKJtTbhd2QTGHe+eLgQpYSSp0eBIHzev8AyVTw9pfwxHuxsnw2PPvfmY3M4EbIR6akxEeR0uExttTYSbmwklYv9F//2gAMAwEAAgADAAAAEPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPOEPPPPPPPPPPPPPPPPPPKOEGPPPPPPPPPPPPPPPPPPPCAAAEPPPPPPPPPPPPPPPPPPLIAAAAEHPPPPPPPPPPPPPPPPFAAABAABPPPPPPPPPPPPPPPPBAAEAAABPPPPPPPPOPPPPPPPLCAAAAFPPPPPPPPPPHPPPPPPPHCAAAJPPPPPPPPPPPPPPPPPPPDLDPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPOPPPPPPPPPPPPPPPPPPPPPPPDPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPKPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPP/EABQRAQAAAAAAAAAAAAAAAAAAAJD/2gAIAQMBAT8QED//xAAUEQEAAAAAAAAAAAAAAAAAAACQ/9oACAECAQE/EBA//8QAKxAAAQIDBgUFAQEAAAAAAAAAAQARECExIEFQcZHwUWGB0fEwkKGx4UDB/9oACAEBAAE/EPcVFib9eIQprKgdJjoC+FDEzxCNghCi8ZAABmUKmHgw60qRAr29L1UAZaD5iCF/QZQDg4BhRWEVRQI8k9OZbJzKRuQgAobAUgFw5oQgArA6aeDWDdEmcxPW4CbDAFGouiAfaeRCGDirM/QEazHUgX4HNgh9n/WQDCADYQEjkjIaEN2JTzYnEVTVIwMiE1EnaBJQaAGgTQA0EMjIeBEXbOkYDVgVA4HKdAFxiAX/AEiDbApZAPsLiGAhzNCQEngp5EA2FZhBM1oEBT+F5gmnkwEAoLjfAY7sDmWdgA1RA2daHCFshEhnZDYEKQa73fHQjAGReR6KBAGQe7wu1KjgXLO/9RHRgPhEMAprWEOeN9Eh3QmfZr+QAU9cvFp3kiSDtswrwDYAzFsAAADSaU9+AIAZNsEGZWJHT3AhVaoEDcEKfMQAxAEM6J0yBXCYMQsCSdEkkDgACCSP9K2BPlADCLqEAB0wAs1A7DogDMHcCCA6egIcoAggieI4BucuMo4wQrApAUZBhiCGoCraShe4xMP/2Q==")),
+                    StatusCode = System.Net.HttpStatusCode.OK
+                }));
+            converter = new HtmlConverter(mainPart, webRequest.Object);
+
+            Assert.DoesNotThrowAsync(async () => {
+                await converter.ParseBody(@"
+                    <table>
+                    <tr>
+                        <td>
+                        <img src='https://domain/a.png'>
+                        <img src='https://domain/b.png'>
+                        </td>
+                    </tr>
+                    </table>");
+            }, "Should not raise ObjectDisposedException");
+
+            webRequest.Verify(s => s.FetchAsync(
+                It.IsAny<Uri>(),
+                It.IsAny<CancellationToken>()),
+                Times.Exactly(2));
+            Assert.That(mainPart.ImageParts.Count(), Is.EqualTo(2));
+        }
+
         private static (Drawing, ImagePart) AssertIsImg(OpenXmlPartContainer container, OpenXmlElement paragraph)
         {
             var run = paragraph.GetFirstChild<Run>();