diff --git a/src/BloomExe/ImageProcessing/TagLibCultureFix.cs b/src/BloomExe/ImageProcessing/TagLibCultureFix.cs new file mode 100644 index 000000000000..ccfe8c91ef4a --- /dev/null +++ b/src/BloomExe/ImageProcessing/TagLibCultureFix.cs @@ -0,0 +1,61 @@ +using System.IO; + +namespace Bloom.ImageProcessing +{ + /// + /// 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 LastIndexOf("."). 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 taglib/, 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. + /// + public static class TagLibCultureFix + { + private static bool _registered; + + /// + /// Registers the resolver with TagLib#. Call it once at startup, before anything touches + /// image metadata; calling it again does nothing. + /// + public static void Register() + { + if (_registered) + return; + _registered = true; + TagLib.File.AddFileTypeResolver(ResolveWithOrdinalExtension); + } + + /// + /// 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. + /// + 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); + } + } +} diff --git a/src/BloomExe/Program.cs b/src/BloomExe/Program.cs index 37ea1b20ee92..cbfeb51185be 100644 --- a/src/BloomExe/Program.cs +++ b/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. diff --git a/src/BloomTests/ImageProcessing/TagLibCultureFixTests.cs b/src/BloomTests/ImageProcessing/TagLibCultureFixTests.cs new file mode 100644 index 000000000000..2564ef340348 --- /dev/null +++ b/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 +{ + /// + /// 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. + /// + [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(); + } + + /// + /// Makes a small image file of the given type ("png" or "jpg") and returns its path. + /// + 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; + } + + /// + /// 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. + /// + 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)); + } + } + } +} diff --git a/src/BloomTests/TestTagLibCultureFix.cs b/src/BloomTests/TestTagLibCultureFix.cs new file mode 100644 index 000000000000..1fcaa9e39b54 --- /dev/null +++ b/src/BloomTests/TestTagLibCultureFix.cs @@ -0,0 +1,25 @@ +using Bloom.ImageProcessing; +using NUnit.Framework; + +namespace BloomTests +{ + /// + /// 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. + /// + /// + /// Like SetupFixture, this is a [SetUpFixture] in the BloomTests namespace, so it applies + /// to that namespace and everything under it. + /// + [SetUpFixture] + public class TestTagLibCultureFix + { + /// NUnit calls this once, before any test runs. + [OneTimeSetUp] + public void RegisterTagLibCultureFix() + { + TagLibCultureFix.Register(); + } + } +}