Fix EXIF UserComment decoding across current and legacy encodings

src/PicView.Core/Exif/ExifReader.cs

@@ -7,6 +7,8 @@ namespace PicView.Core.Exif;
 
 public static class ExifReader
 {
+    private static readonly UTF8Encoding StrictUtf8 = new(false, true);
+
     public static DateTime? GetDateTaken(IExifProfile profile)
     {
         var getDateTaken =
@@ -381,26 +383,126 @@ public static string GetSubject(IExifProfile? profile)
     /// <returns>A string containing the user comment. Returns an empty string if no comment is found or in case of an error.</returns>
     public static string GetUserComment(IExifProfile? profile)
     {
-        var commentBytes = profile?.GetValue(ExifTag.UserComment)?.Value;
-        if (commentBytes is null || commentBytes.Length <= 8)
+        if (profile is null)
         {
             return string.Empty;
         }
 
         try
         {
-            var decodedComment = Encoding.ASCII.GetString(commentBytes);
-            if (string.IsNullOrWhiteSpace(decodedComment))
+            // ImageMagick normalizes the profile byte order when a tag is first read.
+            // Preserve the original TIFF byte order before accessing UserComment.
+            var isBigEndian = IsBigEndian(profile);
+            var commentBytes = profile.GetValue(ExifTag.UserComment)?.Value;
+            if (commentBytes is null || commentBytes.Length == 0)
             {
                 return string.Empty;
             }
 
-            var result = decodedComment.StartsWith("UNICODE") ? decodedComment.Replace("UNICODE", "") : decodedComment;
-            return result.StartsWith("ASCII") ? string.Empty : result;
+            var value = commentBytes.AsSpan();
+            string result;
+            if (value.StartsWith("UNICODE\0"u8))
+            {
+                var exifVersion = GetExifVersion(profile);
+                var usesUtf8 = string.CompareOrdinal(exifVersion, "0300") >= 0;
+                result = DecodeUnicodeComment(value[8..], usesUtf8, isBigEndian);
+            }
+            else if (value.StartsWith("ASCII\0\0\0"u8))
+            {
+                result = Encoding.ASCII.GetString(value[8..]).TrimEnd('\0');
+            }
+            else if (value.StartsWith("JIS\0\0\0\0\0"u8))
+            {
+                // ISO-2022-JP requires a code-page decoder that PicView does not currently include.
+                result = string.Empty;
+            }
+            else if (value.Length >= 8 && value[..8].IndexOfAnyExcept((byte)0) < 0)
+            {
+                result = DecodeUtf8OrFallback(value[8..], Encoding.ASCII);
+            }
+            else
+            {
+                // PicView previously wrote comments without the EXIF character-code prefix.
+                result = Encoding.ASCII.GetString(value).TrimEnd('\0');
+            }
+
+            return string.IsNullOrWhiteSpace(result) ? string.Empty : result;
         }
         catch (Exception)
         {
             return string.Empty;
         }
     }
-}
+
+    private static string DecodeUnicodeComment(ReadOnlySpan<byte> comment, bool usesUtf8, bool isBigEndian)
+    {
+        if (comment.Length >= 2 && comment[0] == 0xfe && comment[1] == 0xff)
+        {
+            return Encoding.BigEndianUnicode.GetString(comment[2..]).TrimEnd('\0');
+        }
+
+        if (comment.Length >= 2 && comment[0] == 0xff && comment[1] == 0xfe)
+        {
+            return Encoding.Unicode.GetString(comment[2..]).TrimEnd('\0');
+        }
+
+        if (usesUtf8)
+        {
+            try
+            {
+                return StrictUtf8.GetString(comment).TrimEnd('\0');
+            }
+            catch (DecoderFallbackException)
+            {
+                var fallback = isBigEndian ? Encoding.BigEndianUnicode : Encoding.Unicode;
+                return fallback.GetString(comment).TrimEnd('\0');
+            }
+        }
+
+        var bigEndianPairs = 0;
+        var littleEndianPairs = 0;
+        for (var i = 0; i + 1 < comment.Length; i += 2)
+        {
+            if (comment[i] == 0 && comment[i + 1] != 0)
+            {
+                bigEndianPairs++;
+            }
+            else if (comment[i] != 0 && comment[i + 1] == 0)
+            {
+                littleEndianPairs++;
+            }
+        }
+
+        if (bigEndianPairs > littleEndianPairs)
+        {
+            return Encoding.BigEndianUnicode.GetString(comment).TrimEnd('\0');
+        }
+
+        if (littleEndianPairs > bigEndianPairs)
+        {
+            return Encoding.Unicode.GetString(comment).TrimEnd('\0');
+        }
+
+        var encoding = isBigEndian ? Encoding.BigEndianUnicode : Encoding.Unicode;
+        return encoding.GetString(comment).TrimEnd('\0');
+    }
+
+    private static string DecodeUtf8OrFallback(ReadOnlySpan<byte> comment, Encoding fallback)
+    {
+        try
+        {
+            return StrictUtf8.GetString(comment).TrimEnd('\0');
+        }
+        catch (DecoderFallbackException)
+        {
+            return fallback.GetString(comment).TrimEnd('\0');
+        }
+    }
+
+    private static bool IsBigEndian(IExifProfile profile)
+    {
+        var data = profile.ToByteArray();
+        return (data.Length >= 2 && data[0] == (byte)'M' && data[1] == (byte)'M') ||
+               (data.Length >= 8 && data[6] == (byte)'M' && data[7] == (byte)'M');
+    }
+}

src/PicView.Core/ViewModels/ExifViewModel.cs

@@ -618,6 +618,10 @@ public void UpdateExifValues(ImageModel model, MagickImage? magick = null)
                 }
             }
 
+            // Read UserComment before any other EXIF tag. Magick.NET normalizes the
+            // profile byte order on first access, which legacy Unicode decoding needs.
+            Comment.Value = ExifReader.GetUserComment(profile);
+
             if (profile != null)
             {
                 DpiY.Value = profile.GetValue(ExifTag.YResolution)?.Value.ToDouble() ?? magick.Density.X;
@@ -760,7 +764,6 @@ public void UpdateExifValues(ImageModel model, MagickImage? magick = null)
             ExifVersion.Value = ExifReader.GetExifVersion(profile);
             LensModel.Value = profile?.GetValue(ExifTag.LensModel)?.Value ?? string.Empty;
             LensMaker.Value = profile?.GetValue(ExifTag.LensMake)?.Value ?? string.Empty;
-            Comment.Value = ExifReader.GetUserComment(profile);
         }
         catch (Exception e)
         {
@@ -852,4 +855,4 @@ private async Task RemoveImageMetaData(FileInfo fileInfo)
         ExifVersion.Value = string.Empty;
         DateTaken.Value = null;
     }
-}
+}

src/PicView.Tests/Exif/ExifReaderTests.cs

@@ -0,0 +1,108 @@
+using System.Text;
+using ImageMagick;
+using PicView.Core.Exif;
+
+namespace PicView.Tests.Exif;
+
+public class ExifReaderTests
+{
+    private const string UnicodeComment = "Unicode comment: 中文";
+
+    [Fact]
+    public void GetUserComment_CurrentUtf8Comment_ReturnsDecodedText() =>
+        AssertComment(UnicodeComment, WithCharacterCode("UNICODE\0"u8, Encoding.UTF8.GetBytes(UnicodeComment)),
+            "0300");
+
+    [Fact]
+    public void GetUserComment_CurrentUtf8CommentWithNullTerminator_ReturnsDecodedText() =>
+        AssertComment(UnicodeComment,
+            WithCharacterCode("UNICODE\0"u8, Encoding.UTF8.GetBytes(UnicodeComment + '\0')), "0300");
+
+    [Theory]
+    [InlineData("A")]
+    [InlineData("AB")]
+    public void GetUserComment_CurrentUtf8CommentWithNullTerminatorAndEitherParity_ReturnsDecodedText(
+        string expected) =>
+        AssertComment(expected, WithCharacterCode("UNICODE\0"u8, Encoding.UTF8.GetBytes(expected + '\0')), "0300");
+
+    [Fact]
+    public void GetUserComment_CurrentUtf8CommentWithEmbeddedNulls_ReturnsDecodedText() =>
+        AssertComment("A\0B\0C", WithCharacterCode("UNICODE\0"u8, Encoding.UTF8.GetBytes("A\0B\0C")), "0300");
+
+    [Fact]
+    public void GetUserComment_LegacyLittleEndianUnicodeComment_ReturnsDecodedText() =>
+        AssertComment(UnicodeComment,
+            WithCharacterCode("UNICODE\0"u8, Encoding.Unicode.GetBytes(UnicodeComment + '\0')), "0232");
+
+    [Fact]
+    public void GetUserComment_LegacyLittleEndianUnicodeWithoutNullPattern_ReturnsDecodedText() =>
+        AssertComment("中", WithCharacterCode("UNICODE\0"u8, Encoding.Unicode.GetBytes("中")), "0232");
+
+    [Fact]
+    public void GetUserComment_LegacyLittleEndianBytesThatAreValidUtf8_ReturnsDecodedText() =>
+        AssertComment("āĂ", WithCharacterCode("UNICODE\0"u8, Encoding.Unicode.GetBytes("āĂ")), "0232");
+
+    [Fact]
+    public void GetUserComment_LegacyBigEndianUnicodeBom_ReturnsDecodedText() =>
+        AssertComment(UnicodeComment, WithCharacterCode("UNICODE\0"u8,
+            Encoding.BigEndianUnicode.GetPreamble()
+                .Concat(Encoding.BigEndianUnicode.GetBytes(UnicodeComment + '\0'))
+                .ToArray()), "0232");
+
+    [Fact]
+    public void GetUserComment_LegacyBigEndianUnicodeWithoutBom_ReturnsDecodedText() =>
+        AssertComment("Big endian comment", WithCharacterCode("UNICODE\0"u8,
+            Encoding.BigEndianUnicode.GetBytes("Big endian comment\0")), "0232");
+
+    [Fact]
+    public void GetUserComment_LegacyBigEndianUnicodeWithoutBomOrNullPattern_ReturnsDecodedText()
+    {
+        var profile = new ExifProfile(Convert.FromHexString(
+            "4578696600004D4D002A00000008000187690004000000010000001A000000000002900000070000000430323332928600070000000C0000003800000000554E49434F4445004E2D6587"));
+
+        var actual = ExifReader.GetUserComment(profile);
+
+        Assert.Equal("中文", actual);
+    }
+
+    [Fact]
+    public void GetUserComment_AsciiComment_ReturnsTextWithoutCharacterCode() =>
+        AssertComment("ASCII comment",
+            WithCharacterCode("ASCII\0\0\0"u8, Encoding.ASCII.GetBytes("ASCII comment\0")));
+
+    [Fact]
+    public void GetUserComment_UndefinedUtf8Comment_ReturnsDecodedText() =>
+        AssertComment(UnicodeComment, WithCharacterCode(new byte[8], Encoding.UTF8.GetBytes(UnicodeComment)));
+
+    [Fact]
+    public void GetUserComment_UnsupportedJisComment_ReturnsEmpty() =>
+        AssertComment(string.Empty, WithCharacterCode("JIS\0\0\0\0\0"u8, [0x1b, 0x24, 0x42, 0x24, 0x22]));
+
+    [Theory]
+    [InlineData("Legacy comment without prefix")]
+    [InlineData("Short")]
+    public void GetUserComment_LegacyPrefixlessComment_PreservesEntireText(string expected) =>
+        AssertComment(expected, Encoding.ASCII.GetBytes(expected));
+
+    private static void AssertComment(string expected, byte[] value, string? exifVersion = null)
+    {
+        var profile = new ExifProfile();
+        profile.SetValue(ExifTag.UserComment, value);
+        if (exifVersion is not null)
+        {
+            profile.SetValue(ExifTag.ExifVersion, Encoding.ASCII.GetBytes(exifVersion));
+        }
+
+        var actual = ExifReader.GetUserComment(profile);
+
+        Assert.Equal(expected, actual);
+    }
+
+    private static byte[] WithCharacterCode(ReadOnlySpan<byte> characterCode, byte[] comment)
+    {
+        var value = new byte[characterCode.Length + comment.Length];
+        characterCode.CopyTo(value);
+        comment.CopyTo(value, characterCode.Length);
+        return value;
+    }
+}

src/PicView.Tests/Exif/ExifViewModelTests.cs

@@ -0,0 +1,58 @@
+using ImageMagick;
+using PicView.Core.Localization;
+using PicView.Core.Models;
+using PicView.Core.ViewModels;
+
+namespace PicView.Tests.Exif;
+
+public class ExifViewModelTests
+{
+    [Fact]
+    public async Task UpdateExifValues_LegacyBigEndianComment_DecodesBeforeOtherExifTags()
+    {
+        var path = Path.Combine(Path.GetTempPath(), $"picview-exif-{Guid.NewGuid():N}.jpg");
+        try
+        {
+            await TranslationManager.LoadLanguage("en");
+            using (var image = new MagickImage(MagickColors.White, 1, 1))
+            {
+                image.Format = MagickFormat.Jpeg;
+                File.WriteAllBytes(path, AddExifProfile(image.ToByteArray(), BigEndianExif));
+            }
+
+            var model = new ImageModel
+            {
+                FileInfo = new FileInfo(path),
+                PixelWidth = 1,
+                PixelHeight = 1
+            };
+            using var viewModel = new ExifViewModel();
+            var imageWithExif = new MagickImage(path);
+
+            viewModel.UpdateExifValues(model, imageWithExif);
+
+            Assert.Equal("中文", viewModel.Comment.Value);
+        }
+        finally
+        {
+            File.Delete(path);
+        }
+    }
+
+    private static readonly byte[] BigEndianExif = Convert.FromHexString(
+        "4578696600004D4D002A00000008000187690004000000010000001A000000000002900000070000000430323332928600070000000C0000003800000000554E49434F4445004E2D6587");
+
+    private static byte[] AddExifProfile(byte[] jpeg, byte[] exif)
+    {
+        var segmentLength = exif.Length + 2;
+        var result = new byte[jpeg.Length + exif.Length + 4];
+        jpeg.AsSpan(0, 2).CopyTo(result);
+        result[2] = 0xff;
+        result[3] = 0xe1;
+        result[4] = (byte)(segmentLength >> 8);
+        result[5] = (byte)segmentLength;
+        exif.CopyTo(result, 6);
+        jpeg.AsSpan(2).CopyTo(result.AsSpan(6 + exif.Length));
+        return result;
+    }
+}