From 20597ac07e0cc5c181c64c24d9fa69e483865cf9 Mon Sep 17 00:00:00 2001 From: aladin <6892108+aladin7@users.noreply.github.com> Date: Tue, 21 Jul 2026 14:03:39 +0300 Subject: [PATCH 1/2] Fix EXIF UserComment decoding --- src/PicView.Core/Exif/ExifReader.cs | 116 +++++++++++++++++-- src/PicView.Core/ViewModels/ExifViewModel.cs | 7 +- src/PicView.Tests/Exif/ExifReaderTests.cs | 108 +++++++++++++++++ src/PicView.Tests/Exif/ExifViewModelTests.cs | 58 ++++++++++ 4 files changed, 280 insertions(+), 9 deletions(-) create mode 100644 src/PicView.Tests/Exif/ExifReaderTests.cs create mode 100644 src/PicView.Tests/Exif/ExifViewModelTests.cs diff --git a/src/PicView.Core/Exif/ExifReader.cs b/src/PicView.Core/Exif/ExifReader.cs index 4edf21251..e7537eab4 100644 --- a/src/PicView.Core/Exif/ExifReader.cs +++ b/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) /// A string containing the user comment. Returns an empty string if no comment is found or in case of an error. 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; } } -} \ No newline at end of file + + private static string DecodeUnicodeComment(ReadOnlySpan 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 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'); + } +} diff --git a/src/PicView.Core/ViewModels/ExifViewModel.cs b/src/PicView.Core/ViewModels/ExifViewModel.cs index e0566077f..877b577ad 100644 --- a/src/PicView.Core/ViewModels/ExifViewModel.cs +++ b/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; } -} \ No newline at end of file +} diff --git a/src/PicView.Tests/Exif/ExifReaderTests.cs b/src/PicView.Tests/Exif/ExifReaderTests.cs new file mode 100644 index 000000000..c2f1b34d2 --- /dev/null +++ b/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 characterCode, byte[] comment) + { + var value = new byte[characterCode.Length + comment.Length]; + characterCode.CopyTo(value); + comment.CopyTo(value, characterCode.Length); + return value; + } +} diff --git a/src/PicView.Tests/Exif/ExifViewModelTests.cs b/src/PicView.Tests/Exif/ExifViewModelTests.cs new file mode 100644 index 000000000..7906e483d --- /dev/null +++ b/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; + } +} From b45cbda5023d93251c76508fe626e0b1285f5bde Mon Sep 17 00:00:00 2001 From: aladin <6892108+aladin7@users.noreply.github.com> Date: Wed, 22 Jul 2026 18:50:21 +0300 Subject: [PATCH 2/2] Handle mislabeled Unicode EXIF comments --- src/PicView.Core/Exif/ExifReader.cs | 33 ++++++++++++++++++++++- src/PicView.Tests/Exif/ExifReaderTests.cs | 10 +++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/PicView.Core/Exif/ExifReader.cs b/src/PicView.Core/Exif/ExifReader.cs index e7537eab4..f431a2b98 100644 --- a/src/PicView.Core/Exif/ExifReader.cs +++ b/src/PicView.Core/Exif/ExifReader.cs @@ -409,7 +409,7 @@ public static string GetUserComment(IExifProfile? profile) } else if (value.StartsWith("ASCII\0\0\0"u8)) { - result = Encoding.ASCII.GetString(value[8..]).TrimEnd('\0'); + result = DecodeAsciiComment(value[8..], isBigEndian); } else if (value.StartsWith("JIS\0\0\0\0\0"u8)) { @@ -434,6 +434,37 @@ public static string GetUserComment(IExifProfile? profile) } } + private static string DecodeAsciiComment(ReadOnlySpan comment, bool isBigEndian) + { + try + { + var utf8 = StrictUtf8.GetString(comment).TrimEnd('\0'); + if (!ContainsUnexpectedControlCharacters(utf8)) + { + return utf8; + } + } + catch (DecoderFallbackException) + { + // Some applications label Unicode comment bytes as ASCII. + } + + return DecodeUnicodeComment(comment, false, isBigEndian); + } + + private static bool ContainsUnexpectedControlCharacters(ReadOnlySpan text) + { + foreach (var character in text) + { + if (char.IsControl(character) && character is not ('\t' or '\r' or '\n')) + { + return true; + } + } + + return false; + } + private static string DecodeUnicodeComment(ReadOnlySpan comment, bool usesUtf8, bool isBigEndian) { if (comment.Length >= 2 && comment[0] == 0xfe && comment[1] == 0xff) diff --git a/src/PicView.Tests/Exif/ExifReaderTests.cs b/src/PicView.Tests/Exif/ExifReaderTests.cs index c2f1b34d2..eb53aed26 100644 --- a/src/PicView.Tests/Exif/ExifReaderTests.cs +++ b/src/PicView.Tests/Exif/ExifReaderTests.cs @@ -70,6 +70,16 @@ public void GetUserComment_AsciiComment_ReturnsTextWithoutCharacterCode() => AssertComment("ASCII comment", WithCharacterCode("ASCII\0\0\0"u8, Encoding.ASCII.GetBytes("ASCII comment\0"))); + [Fact] + public void GetUserComment_AsciiMarkedUtf8Comment_ReturnsDecodedText() => + AssertComment(UnicodeComment, + WithCharacterCode("ASCII\0\0\0"u8, Encoding.UTF8.GetBytes(UnicodeComment + '\0'))); + + [Fact] + public void GetUserComment_AsciiMarkedUtf16Comment_ReturnsDecodedText() => + AssertComment(UnicodeComment, + WithCharacterCode("ASCII\0\0\0"u8, Encoding.Unicode.GetBytes(UnicodeComment + '\0'))); + [Fact] public void GetUserComment_UndefinedUtf8Comment_ReturnsDecodedText() => AssertComment(UnicodeComment, WithCharacterCode(new byte[8], Encoding.UTF8.GetBytes(UnicodeComment)));