Merge pull request #2944 from SixLabors/js/v3-issue-2906

V3 Use EXIF byte order for EXIF encoded strings.

Author: JimBobSquarePants

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

@@ -50,37 +50,66 @@ private static Encoding JIS0208Encoding
         _ => UndefinedCodeBytes
     };
 
-    public static Encoding GetEncoding(CharacterCode code) => code switch
+    public static Encoding GetEncoding(CharacterCode code, ByteOrder order) => code switch
     {
         CharacterCode.ASCII => Encoding.ASCII,
         CharacterCode.JIS => JIS0208Encoding,
-        CharacterCode.Unicode => Encoding.Unicode,
+        CharacterCode.Unicode => order is ByteOrder.BigEndian ? Encoding.BigEndianUnicode : Encoding.Unicode,
         CharacterCode.Undefined => Encoding.UTF8,
         _ => Encoding.UTF8
     };
 
-    public static bool TryParse(ReadOnlySpan<byte> buffer, out EncodedString encodedString)
+    public static bool TryParse(ReadOnlySpan<byte> buffer, ByteOrder order, out EncodedString encodedString)
     {
         if (TryDetect(buffer, out CharacterCode code))
         {
-            string text = GetEncoding(code).GetString(buffer[CharacterCodeBytesLength..]);
-            encodedString = new EncodedString(code, text);
-            return true;
+            ReadOnlySpan<byte> textBuffer = buffer[CharacterCodeBytesLength..];
+            if (code == CharacterCode.Unicode && textBuffer.Length >= 2)
+            {
+                // Check BOM
+                if (textBuffer[0] == 0xFF && textBuffer[1] == 0xFE)
+                {
+                    // Little-endian BOM
+                    string text = Encoding.Unicode.GetString(textBuffer[2..]);
+                    encodedString = new EncodedString(code, text);
+                    return true;
+                }
+                else if (textBuffer[0] == 0xFE && textBuffer[1] == 0xFF)
+                {
+                    // Big-endian BOM
+                    string text = Encoding.BigEndianUnicode.GetString(textBuffer[2..]);
+                    encodedString = new EncodedString(code, text);
+                    return true;
+                }
+                else
+                {
+                    // No BOM, use EXIF byte order
+                    string text = GetEncoding(code, order).GetString(textBuffer);
+                    encodedString = new EncodedString(code, text);
+                    return true;
+                }
+            }
+            else
+            {
+                string text = GetEncoding(code, order).GetString(textBuffer);
+                encodedString = new EncodedString(code, text);
+                return true;
+            }
         }
 
         encodedString = default;
         return false;
     }
 
     public static uint GetDataLength(EncodedString encodedString) =>
-        (uint)GetEncoding(encodedString.Code).GetByteCount(encodedString.Text) + CharacterCodeBytesLength;
+        (uint)GetEncoding(encodedString.Code, ByteOrder.LittleEndian).GetByteCount(encodedString.Text) + CharacterCodeBytesLength;
 
     public static int Write(EncodedString encodedString, Span<byte> destination)
     {
         GetCodeBytes(encodedString.Code).CopyTo(destination);
 
         string text = encodedString.Text;
-        int count = Write(GetEncoding(encodedString.Code), text, destination[CharacterCodeBytesLength..]);
+        int count = Write(GetEncoding(encodedString.Code, ByteOrder.LittleEndian), text, destination[CharacterCodeBytesLength..]);
 
         return CharacterCodeBytesLength + count;
     }

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

@@ -90,8 +90,8 @@ internal abstract class BaseExifReader
     private readonly MemoryAllocator? allocator;
     private readonly Stream data;
     private List<ExifTag>? invalidTags;
-
     private List<ulong>? subIfds;
+    private bool isBigEndian;
 
     protected BaseExifReader(Stream stream, MemoryAllocator? allocator)
     {
@@ -116,7 +116,17 @@ protected BaseExifReader(Stream stream, MemoryAllocator? allocator)
     /// </summary>
     public uint ThumbnailOffset { get; protected set; }
 
-    public bool IsBigEndian { get; protected set; }
+    public bool IsBigEndian
+    {
+        get => this.isBigEndian;
+        protected set
+        {
+            this.isBigEndian = value;
+            this.ByteOrder = value ? ByteOrder.BigEndian : ByteOrder.LittleEndian;
+        }
+    }
+
+    protected ByteOrder ByteOrder { get; private set; }
 
     public List<(ulong Offset, ExifDataType DataType, ulong NumberOfComponents, ExifValue Exif)> BigValues { get; } = new();
 
@@ -485,7 +495,14 @@ private void ReadValue64(List<IExifValue> values, Span<byte> offsetBuffer)
 
     private void Add(IList<IExifValue> values, IExifValue exif, object? value)
     {
-        if (!exif.TrySetValue(value))
+        if (exif is ExifEncodedString encodedString)
+        {
+            if (!encodedString.TrySetValue(value, this.ByteOrder))
+            {
+                return;
+            }
+        }
+        else if (!exif.TrySetValue(value))
         {
             return;
         }

src/ImageSharp/Metadata/Profiles/Exif/Tags/ExifTag.cs

@@ -23,14 +23,14 @@ public abstract partial class ExifTag : IEquatable<ExifTag>
     /// </summary>
     /// <param name="left">The first <see cref="ExifTag"/> to compare.</param>
     /// <param name="right"> The second <see cref="ExifTag"/> to compare.</param>
-    public static bool operator ==(ExifTag left, ExifTag right) => Equals(left, right);
+    public static bool operator ==(ExifTag? left, ExifTag? right) => left?.Equals(right) == true;
 
     /// <summary>
     /// Determines whether the specified <see cref="ExifTag"/> instances are not considered equal.
     /// </summary>
     /// <param name="left">The first <see cref="ExifTag"/> to compare.</param>
     /// <param name="right"> The second <see cref="ExifTag"/> to compare.</param>
-    public static bool operator !=(ExifTag left, ExifTag right) => !Equals(left, right);
+    public static bool operator !=(ExifTag? left, ExifTag? right) => !(left == right);
 
     /// <inheritdoc/>
     public override bool Equals(object? obj)

src/ImageSharp/Metadata/Profiles/Exif/Values/ExifEncodedString.cs

@@ -24,7 +24,7 @@ private ExifEncodedString(ExifEncodedString value)
 
     protected override string StringValue => this.Value.Text;
 
-    public override bool TrySetValue(object? value)
+    public bool TrySetValue(object? value, ByteOrder order)
     {
         if (base.TrySetValue(value))
         {
@@ -38,7 +38,7 @@ public override bool TrySetValue(object? value)
         }
         else if (value is byte[] buffer)
         {
-            if (ExifEncodedStringHelpers.TryParse(buffer, out EncodedString encodedString))
+            if (ExifEncodedStringHelpers.TryParse(buffer, order, out EncodedString encodedString))
             {
                 this.Value = encodedString;
                 return true;
@@ -48,5 +48,8 @@ public override bool TrySetValue(object? value)
         return false;
     }
 
+    public override bool TrySetValue(object? value)
+        => this.TrySetValue(value, ByteOrder.LittleEndian);
+
     public override IExifValue DeepClone() => new ExifEncodedString(this);
 }

tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs

@@ -5,6 +5,7 @@
 using SixLabors.ImageSharp.Formats;
 using SixLabors.ImageSharp.Formats.Webp;
 using SixLabors.ImageSharp.Metadata;
+using SixLabors.ImageSharp.Metadata.Profiles.Exif;
 using SixLabors.ImageSharp.PixelFormats;
 using SixLabors.ImageSharp.Tests.TestUtilities;
 using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison;
@@ -559,4 +560,22 @@ public void WebpDecoder_CanDecode_Issue2925<TPixel>(TestImageProvider<TPixel> pr
         image.DebugSave(provider);
         image.CompareToOriginal(provider, ReferenceDecoder);
     }
+
+    [Theory]
+    [WithFile(Lossy.Issue2906, PixelTypes.Rgba32)]
+    public void WebpDecoder_CanDecode_Issue2906<TPixel>(TestImageProvider<TPixel> provider)
+        where TPixel : unmanaged, IPixel<TPixel>
+    {
+        using Image<TPixel> image = provider.GetImage(WebpDecoder.Instance);
+
+        ExifProfile exifProfile = image.Metadata.ExifProfile;
+        IExifValue<EncodedString> comment = exifProfile.GetValue(ExifTag.UserComment);
+
+        Assert.NotNull(comment);
+        Assert.Equal(EncodedString.CharacterCode.Unicode, comment.Value.Code);
+        Assert.StartsWith("1girl, pariya, ", comment.Value.Text);
+
+        image.DebugSave(provider);
+        image.CompareToOriginal(provider, ReferenceDecoder);
+    }
 }

tests/ImageSharp.Tests/TestImages.cs

@@ -860,6 +860,7 @@ public static class Lossy
             public const string Issue2801 = "Webp/issues/Issue2801.webp";
             public const string Issue2866 = "Webp/issues/Issue2866.webp";
             public const string Issue2925 = "Webp/issues/Issue2925.webp";
+            public const string Issue2906 = "Webp/issues/Issue2906.webp";
         }
     }
 

tests/Images/Input/Webp/issues/Issue2906.webp

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:56fe6a91feb9545c0a15966e0f6bc560890b193073c96ae9e39bf387c7e0cbca
+size 157092