Enhance XmpProfile to add XDocument normalization

src/ImageSharp/Metadata/Profiles/XMP/XmpProfile.cs

@@ -1,8 +1,8 @@
 // Copyright (c) Six Labors.
 // Licensed under the Six Labors Split License.
 
-using System.Diagnostics;
 using System.Text;
+using System.Xml;
 using System.Xml.Linq;
 
 namespace SixLabors.ImageSharp.Metadata.Profiles.Xmp;
@@ -25,18 +25,17 @@ public sealed class XmpProfile : IDeepCloneable<XmpProfile>
     /// Initializes a new instance of the <see cref="XmpProfile"/> class.
     /// </summary>
     /// <param name="data">The UTF8 encoded byte array to read the XMP profile from.</param>
-    public XmpProfile(byte[]? data) => this.Data = data;
+    public XmpProfile(byte[]? data) => this.Data = NormalizeDataIfNeeded(data);
 
     /// <summary>
-    /// Initializes a new instance of the <see cref="XmpProfile"/> class
-    /// by making a copy from another XMP profile.
+    /// Initializes a new instance of the <see cref="XmpProfile"/> class from an XML document.
+    /// The document is serialized as UTF-8 without BOM.
     /// </summary>
-    /// <param name="other">The other XMP profile, from which the clone should be made from.</param>
-    private XmpProfile(XmpProfile other)
+    /// <param name="document">The XMP XML document.</param>
+    public XmpProfile(XDocument document)
     {
-        Guard.NotNull(other, nameof(other));
-
-        this.Data = other.Data;
+        Guard.NotNull(document, nameof(document));
+        this.Data = SerializeDocument(document);
     }
 
     /// <summary>
@@ -45,30 +44,28 @@ public sealed class XmpProfile : IDeepCloneable<XmpProfile>
     internal byte[]? Data { get; private set; }
 
     /// <summary>
-    /// Gets the raw XML document containing the XMP profile.
+    /// Convert the content of this <see cref="XmpProfile"/> into an <see cref="XDocument"/>.
     /// </summary>
     /// <returns>The <see cref="XDocument"/></returns>
-    public XDocument? GetDocument()
+    public XDocument? ToXDocument()
     {
-        byte[]? byteArray = this.Data;
-        if (byteArray is null)
+        byte[]? data = this.Data;
+        if (data is null || data.Length == 0)
         {
             return null;
         }
 
-        // Strip leading whitespace, as the XmlReader doesn't like them.
-        int count = byteArray.Length;
-        for (int i = count - 1; i > 0; i--)
+        using MemoryStream stream = new(data, writable: false);
+
+        XmlReaderSettings settings = new()
         {
-            if (byteArray[i] is 0 or 0x0f)
-            {
-                count--;
-            }
-        }
+            DtdProcessing = DtdProcessing.Ignore,
+            XmlResolver = null,
+            CloseInput = false
+        };
 
-        using MemoryStream stream = new(byteArray, 0, count);
-        using StreamReader reader = new(stream, Encoding.UTF8);
-        return XDocument.Load(reader);
+        using XmlReader reader = XmlReader.Create(stream, settings);
+        return XDocument.Load(reader, LoadOptions.PreserveWhitespace);
     }
 
     /// <summary>
@@ -84,5 +81,76 @@ public sealed class XmpProfile : IDeepCloneable<XmpProfile>
     }
 
     /// <inheritdoc/>
-    public XmpProfile DeepClone() => new(this);
+    public XmpProfile DeepClone()
+    {
+        Guard.NotNull(this.Data);
+
+        byte[] clone = new byte[this.Data.Length];
+        this.Data.AsSpan().CopyTo(clone);
+        return new XmpProfile(clone);
+    }
+
+    private static byte[] SerializeDocument(XDocument document)
+    {
+        using MemoryStream ms = new();
+
+        XmlWriterSettings writerSettings = new()
+        {
+            Encoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false), // no BOM
+            OmitXmlDeclaration = true, // generally safer for XMP consumers
+            Indent = false,
+            NewLineHandling = NewLineHandling.None
+        };
+
+        using (XmlWriter xw = XmlWriter.Create(ms, writerSettings))
+        {
+            document.Save(xw);
+        }
+
+        return ms.ToArray();
+    }
+
+    private static byte[]? NormalizeDataIfNeeded(byte[]? data)
+    {
+        if (data is null || data.Length == 0)
+        {
+            return data;
+        }
+
+        // Allocation-free fast path for the normal case.
+        bool hasBom = data.Length >= 3 && data[0] == 0xEF && data[1] == 0xBB && data[2] == 0xBF;
+        bool hasTrailingPad = data[^1] is 0 or 0x0F;
+
+        if (!hasBom && !hasTrailingPad)
+        {
+            return data;
+        }
+
+        int start = hasBom ? 3 : 0;
+        int end = data.Length;
+
+        if (hasTrailingPad)
+        {
+            while (end > start)
+            {
+                byte b = data[end - 1];
+                if (b is not 0 and not 0x0F)
+                {
+                    break;
+                }
+
+                end--;
+            }
+        }
+
+        int length = end - start;
+        if (length <= 0)
+        {
+            return [];
+        }
+
+        byte[] normalized = new byte[length];
+        Buffer.BlockCopy(data, start, normalized, 0, length);
+        return normalized;
+    }
 }

tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs

@@ -157,7 +157,7 @@ public class TiffMetadataTests
         {
             Assert.NotNull(rootFrameMetaData.XmpProfile);
             Assert.NotNull(rootFrameMetaData.ExifProfile);
-            Assert.Equal(2599, rootFrameMetaData.XmpProfile.Data.Length);
+            Assert.Equal(2596, rootFrameMetaData.XmpProfile.Data.Length); // padding bytes are trimmed
             Assert.Equal(25, rootFrameMetaData.ExifProfile.Values.Count);
         }
     }
@@ -186,7 +186,7 @@ public class TiffMetadataTests
         Assert.Equal(32, rootFrame.Width);
         Assert.Equal(32, rootFrame.Height);
         Assert.NotNull(rootFrame.Metadata.XmpProfile);
-        Assert.Equal(2599, rootFrame.Metadata.XmpProfile.Data.Length);
+        Assert.Equal(2596, rootFrame.Metadata.XmpProfile.Data.Length); // padding bytes are trimmed
 
         ExifProfile exifProfile = rootFrame.Metadata.ExifProfile;
         TiffFrameMetadata tiffFrameMetadata = rootFrame.Metadata.GetTiffMetadata();

tests/ImageSharp.Tests/Metadata/ImageFrameMetadataTests.cs

@@ -74,7 +74,7 @@ public class ImageFrameMetadataTests
         Assert.False(metaData.ExifProfile.Equals(clone.ExifProfile));
         Assert.True(metaData.ExifProfile.Values.Count == clone.ExifProfile.Values.Count);
         Assert.False(ReferenceEquals(metaData.XmpProfile, clone.XmpProfile));
-        Assert.True(metaData.XmpProfile.Data.Equals(clone.XmpProfile.Data));
+        Assert.False(ReferenceEquals(metaData.XmpProfile.Data, clone.XmpProfile.Data));
         Assert.False(metaData.GetGifMetadata().Equals(clone.GetGifMetadata()));
         Assert.False(metaData.IccProfile.Equals(clone.IccProfile));
         Assert.False(metaData.IptcProfile.Equals(clone.IptcProfile));

tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs

@@ -78,6 +78,34 @@ public class XmpProfileTests
         }
     }
 
+    [Fact]
+    public void XmlProfile_CtorFromXDocument_Works()
+    {
+        // arrange
+        XDocument document = CreateMinimalXDocument();
+
+        // act
+        XmpProfile profile = new(document);
+
+        // assert
+        XmpProfileContainsExpectedValues(profile);
+    }
+
+    [Fact]
+    public void XmpProfile_ToXDocument_ReturnsValidDocument()
+    {
+        // arrange
+        XmpProfile profile = CreateMinimalXmlProfile();
+
+        // act
+        XDocument document = profile.ToXDocument();
+
+        // assert
+        Assert.NotNull(document);
+        Assert.Equal("xmpmeta", document.Root.Name.LocalName);
+        Assert.Equal("adobe:ns:meta/", document.Root.Name.NamespaceName);
+    }
+
     [Fact]
     public void XmpProfile_ToFromByteArray_ReturnsClone()
     {
@@ -97,11 +125,11 @@ public class XmpProfileTests
     {
         // arrange
         XmpProfile profile = CreateMinimalXmlProfile();
-        byte[] original = profile.ToByteArray();
+        byte[] original = profile.Data;
 
         // act
         XmpProfile clone = profile.DeepClone();
-        byte[] actual = clone.ToByteArray();
+        byte[] actual = clone.Data;
 
         // assert
         Assert.False(ReferenceEquals(original, actual));
@@ -218,7 +246,7 @@ public class XmpProfileTests
     private static void XmpProfileContainsExpectedValues(XmpProfile xmp)
     {
         Assert.NotNull(xmp);
-        XDocument document = xmp.GetDocument();
+        XDocument document = xmp.ToXDocument();
         Assert.NotNull(document);
         Assert.Equal("xmpmeta", document.Root.Name.LocalName);
         Assert.Equal("adobe:ns:meta/", document.Root.Name.NamespaceName);
@@ -232,6 +260,8 @@ public class XmpProfileTests
         return profile;
     }
 
+    private static XDocument CreateMinimalXDocument() => CreateMinimalXmlProfile().ToXDocument();
+
     private static Image<Rgba32> WriteAndRead(Image<Rgba32> image, IImageEncoder encoder)
     {
         using (MemoryStream memStream = new())