From bc35ee6b37ee638c5ff491efff59b0625a19ca14 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 22 Jan 2026 15:32:06 +1000 Subject: [PATCH] Enhance XmpProfile to add XDocument normalization --- .../Metadata/Profiles/XMP/XmpProfile.cs | 118 ++++++++++++++---- .../Formats/Tiff/TiffMetadataTests.cs | 4 +- .../Metadata/ImageFrameMetadataTests.cs | 2 +- .../Metadata/Profiles/XMP/XmpProfileTests.cs | 36 +++++- 4 files changed, 129 insertions(+), 31 deletions(-) diff --git a/src/ImageSharp/Metadata/Profiles/XMP/XmpProfile.cs b/src/ImageSharp/Metadata/Profiles/XMP/XmpProfile.cs index 77ff35df0..de34a1eaa 100644 --- a/src/ImageSharp/Metadata/Profiles/XMP/XmpProfile.cs +++ b/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 /// Initializes a new instance of the class. /// /// The UTF8 encoded byte array to read the XMP profile from. - public XmpProfile(byte[]? data) => this.Data = data; + public XmpProfile(byte[]? data) => this.Data = NormalizeDataIfNeeded(data); /// - /// Initializes a new instance of the class - /// by making a copy from another XMP profile. + /// Initializes a new instance of the class from an XML document. + /// The document is serialized as UTF-8 without BOM. /// - /// The other XMP profile, from which the clone should be made from. - private XmpProfile(XmpProfile other) + /// The XMP XML document. + public XmpProfile(XDocument document) { - Guard.NotNull(other, nameof(other)); - - this.Data = other.Data; + Guard.NotNull(document, nameof(document)); + this.Data = SerializeDocument(document); } /// @@ -45,30 +44,28 @@ public sealed class XmpProfile : IDeepCloneable internal byte[]? Data { get; private set; } /// - /// Gets the raw XML document containing the XMP profile. + /// Convert the content of this into an . /// /// The - 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); } /// @@ -84,5 +81,76 @@ public sealed class XmpProfile : IDeepCloneable } /// - 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; + } } diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs index 20b3ec2bb..f92d8ed59 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffMetadataTests.cs +++ b/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(); diff --git a/tests/ImageSharp.Tests/Metadata/ImageFrameMetadataTests.cs b/tests/ImageSharp.Tests/Metadata/ImageFrameMetadataTests.cs index cee37ca56..e50d0b617 100644 --- a/tests/ImageSharp.Tests/Metadata/ImageFrameMetadataTests.cs +++ b/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)); diff --git a/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs b/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs index e121d24f9..8e8f89e6f 100644 --- a/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs +++ b/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 WriteAndRead(Image image, IImageEncoder encoder) { using (MemoryStream memStream = new())