Fix image metadata on Thai-locale computers (BL-16926)

src/BloomExe/ImageProcessing/TagLibCultureFix.cs

@@ -0,0 +1,61 @@
+using System.IO;
+
+namespace Bloom.ImageProcessing
+{
+    /// <summary>
+    /// Works around a TagLib# bug that makes every image metadata read or write fail on a computer
+    /// whose regional format is Thai (and probably Lao) (BL-16926).
+    ///
+    /// When TagLib.File.Create is not given a mime type, TagLib# works one out from the file name
+    /// with a culture-sensitive <c>LastIndexOf(".")</c>. Under Thai collation in ICU the period is
+    /// ignorable, so the search matches at the very end of the name, the extension comes out empty,
+    /// the mime type is <c>taglib/</c>, and TagLib# throws UnsupportedFormatException. libpalaso's
+    /// ClearShare metadata code and Bloom's own code both call TagLib.File.Create(path), so rather
+    /// than fixing each caller we register a FileTypeResolver, TagLib#'s own extension point, which
+    /// sees every Create in the process. It redoes the extension lookup ordinally and hands back to
+    /// TagLib# with the right mime type. Once a TagLib# release does an ordinal search itself
+    /// (https://github.com/mono/taglib-sharp/issues/380, fixed by PR 381), this can go.
+    /// </summary>
+    public static class TagLibCultureFix
+    {
+        private static bool _registered;
+
+        /// <summary>
+        /// Registers the resolver with TagLib#. Call it once at startup, before anything touches
+        /// image metadata; calling it again does nothing.
+        /// </summary>
+        public static void Register()
+        {
+            if (_registered)
+                return;
+            _registered = true;
+            TagLib.File.AddFileTypeResolver(ResolveWithOrdinalExtension);
+        }
+
+        /// <summary>
+        /// If TagLib# came up with a mime type it does not know but the file name has an extension
+        /// that it does know, create the file with the mime type for that extension. Otherwise
+        /// return null, which tells TagLib# to carry on as it would have.
+        /// </summary>
+        private static TagLib.File ResolveWithOrdinalExtension(
+            TagLib.File.IFileAbstraction abstraction,
+            string mimetype,
+            TagLib.ReadStyle style
+        )
+        {
+            if (TagLib.FileTypes.AvailableTypes.ContainsKey(mimetype))
+                return null;
+            // Path.GetExtension searches for the period ordinally, whatever the culture.
+            var extension = Path.GetExtension(abstraction.Name);
+            if (string.IsNullOrEmpty(extension))
+                return null;
+            var correctMimetype = "taglib/" + extension.Substring(1).ToLowerInvariant();
+            if (
+                correctMimetype == mimetype
+                || !TagLib.FileTypes.AvailableTypes.ContainsKey(correctMimetype)
+            )
+                return null;
+            return TagLib.File.Create(abstraction, correctMimetype, style);
+        }
+    }
+}

src/BloomExe/Program.cs

@@ -17,6 +17,7 @@
 using Bloom.Collection.BloomPack;
 using Bloom.CollectionChoosing;
 using Bloom.ErrorReporter;
+using Bloom.ImageProcessing;
 using Bloom.MiscUI;
 using Bloom.Properties;
 using Bloom.Registration;
@@ -130,6 +131,7 @@ static int Main(string[] args1)
             // final call to CleanupTempFolder. Also prevents our temp files competing with
             // other programs for 64K available default temp file names.
             TempFile.NamePrefix = "bloom";
+            TagLibCultureFix.Register();
             CheckForCorruptUserConfig();
             // Ensure that the registration information is loaded early before Team Collection
             // needs it.

src/BloomTests/ImageProcessing/TagLibCultureFixTests.cs

@@ -0,0 +1,106 @@
+using System;
+using System.Drawing;
+using System.Drawing.Imaging;
+using System.IO;
+using Bloom;
+using Bloom.ImageProcessing;
+using BloomTemp;
+using NUnit.Framework;
+using SIL.Code;
+using SIL.IO;
+using SIL.Windows.Forms.ImageToolbox;
+
+namespace BloomTests.ImageProcessing
+{
+    /// <summary>
+    /// BL-16926: on a Thai-locale computer TagLib# could not work out an image's file type from its
+    /// name, so every read or write of image metadata threw. See TagLibCultureFix.
+    /// </summary>
+    [TestFixture]
+    public class TagLibCultureFixTests
+    {
+        private TemporaryFolder _folder;
+
+        [SetUp]
+        public void Setup()
+        {
+            // The SetUpFixture registers this for the whole run; say so here too, so that these
+            // tests do not depend on it.
+            TagLibCultureFix.Register();
+            _folder = new TemporaryFolder("TagLibCultureFixTests");
+        }
+
+        [TearDown]
+        public void TearDown()
+        {
+            _folder.Dispose();
+        }
+
+        /// <summary>
+        /// Makes a small image file of the given type ("png" or "jpg") and returns its path.
+        /// </summary>
+        private string MakeImage(string extension)
+        {
+            var path = Path.Combine(_folder.FolderPath, "original." + extension);
+            var format = extension == "png" ? ImageFormat.Png : ImageFormat.Jpeg;
+            using (var bitmap = new Bitmap(10, 10))
+                RobustImageIO.SaveImage(bitmap, path, format);
+            return path;
+        }
+
+        /// <summary>
+        /// Without this, the tests below would pass for nothing on a runtime whose Thai collation
+        /// does not ignore the period, since TagLib# would then find the extension unaided.
+        /// </summary>
+        private static void AssertThaiCollationIgnoresThePeriod()
+        {
+            Assert.That(
+                "original.png".LastIndexOf("."),
+                Is.Not.EqualTo(8),
+                "This runtime's th-TH collation finds the period, so it cannot reproduce BL-16926"
+            );
+        }
+
+        [TestCase("png")]
+        [TestCase("jpg")]
+        [SetCulture("th-TH")]
+        public void ImageMetadata_UnderThaiCulture_CanBeSavedAndReadBack(string extension)
+        {
+            AssertThaiCollationIgnoresThePeriod();
+            var imagePath = MakeImage(extension);
+            Assert.That(RobustFileIO.MetadataFromFile(imagePath).Creator, Is.Null.Or.Empty);
+
+            // This is libpalaso's route into TagLib#, as used whenever a picture's credits change.
+            using (var image = PalasoImage.FromFileRobustly(imagePath))
+            {
+                image.Metadata.Creator = "joe";
+                image.Metadata.CopyrightNotice = "Copyright 1999 by me";
+                RetryUtility.Retry(() => image.SaveUpdatedMetadataIfItMakesSense());
+            }
+
+            var metadata = RobustFileIO.MetadataFromFile(imagePath);
+            Assert.That(metadata.Creator, Is.EqualTo("joe"));
+            Assert.That(metadata.CopyrightNotice, Is.EqualTo("Copyright 1999 by me"));
+        }
+
+        [TestCase("png", typeof(TagLib.Png.File), "taglib/png")]
+        [TestCase("jpg", typeof(TagLib.Jpeg.File), "taglib/jpg")]
+        [SetCulture("th-TH")]
+        public void CreateTaglibFile_UnderThaiCulture_RecognizesTheFileType(
+            string extension,
+            Type expectedType,
+            string expectedMimeType
+        )
+        {
+            AssertThaiCollationIgnoresThePeriod();
+            var imagePath = MakeImage(extension);
+
+            // This is Bloom's own route into TagLib#.
+            using (var file = RobustFileIO.CreateTaglibFile(imagePath))
+            {
+                Assert.That(file, Is.InstanceOf(expectedType));
+                Assert.That(file.MimeType, Is.EqualTo(expectedMimeType));
+            }
+        }
+    }
+}

src/BloomTests/TestTagLibCultureFix.cs

@@ -0,0 +1,25 @@
+using Bloom.ImageProcessing;
+using NUnit.Framework;
+
+namespace BloomTests
+{
+    /// <summary>
+    /// Registers Bloom's TagLib# workaround for Thai-style collation (see TagLibCultureFix) before
+    /// any test runs, just as Program.Main does before Bloom does anything else, so that tests
+    /// touching image metadata behave as Bloom does when the culture sweep runs them under th-TH.
+    /// </summary>
+    /// <remarks>
+    /// Like SetupFixture, this is a [SetUpFixture] in the BloomTests namespace, so it applies
+    /// to that namespace and everything under it.
+    /// </remarks>
+    [SetUpFixture]
+    public class TestTagLibCultureFix
+    {
+        /// <summary>NUnit calls this once, before any test runs.</summary>
+        [OneTimeSetUp]
+        public void RegisterTagLibCultureFix()
+        {
+            TagLibCultureFix.Register();
+        }
+    }
+}