diff --git a/.gitignore b/.gitignore index 86c063c..cedb7b8 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,6 @@ runtime.*.liblouis/runtimes/ # Rider / ReSharper per-user settings *.DotSettings.user + +# Worktrees created by spawned Claude Code sessions +.claude/worktrees/ diff --git a/LibLouis.NET.Test/AssemblyInfo.cs b/LibLouis.NET.Test/AssemblyInfo.cs new file mode 100644 index 0000000..dffc93b --- /dev/null +++ b/LibLouis.NET.Test/AssemblyInfo.cs @@ -0,0 +1,7 @@ +using Xunit; + +// liblouis keeps global state (compiled table cache, log callback, data path) and is explicitly +// not thread safe - LibLouis serialises its own calls behind a lock for exactly that reason. +// xunit parallelises across test classes by default, which lets unsynchronised native calls race +// and produce spurious "could not be compiled" failures. Run the whole assembly serially. +[assembly: CollectionBehavior(DisableTestParallelization = true)] diff --git a/LibLouis.NET.Test/CollectingLogger.cs b/LibLouis.NET.Test/CollectingLogger.cs new file mode 100644 index 0000000..9049579 --- /dev/null +++ b/LibLouis.NET.Test/CollectingLogger.cs @@ -0,0 +1,30 @@ +using System; +using System.Collections.Generic; + +using Microsoft.Extensions.Logging; + +namespace LibLouis.NET.Test; + +/// +/// Captures everything liblouis logs, so tests can assert on what the native side reported. +/// +internal sealed class CollectingLogger : ILogger +{ + public List Messages { get; } = []; + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + // LogLevel is qualified throughout: in this namespace the unqualified name binds to + // LibLouis.NET.LogLevel, the native enum, not the Microsoft.Extensions.Logging one. + public bool IsEnabled(Microsoft.Extensions.Logging.LogLevel logLevel) => true; + + public void Log( + Microsoft.Extensions.Logging.LogLevel logLevel, + EventId eventId, + TState state, + Exception? exception, + Func formatter) + { + Messages.Add(formatter(state, exception)); + } +} diff --git a/LibLouis.NET.Test/HyphenateTests.cs b/LibLouis.NET.Test/HyphenateTests.cs new file mode 100644 index 0000000..2da22eb --- /dev/null +++ b/LibLouis.NET.Test/HyphenateTests.cs @@ -0,0 +1,70 @@ +using System; +using System.IO; +using System.Linq; +using System.Text.RegularExpressions; + +using Xunit; + +namespace LibLouis.NET.Test; + +/// +/// lou_hyphenate takes a caller-allocated char *hyphens buffer and writes inlen + 1 bytes +/// into it: '0' or '1' per character, plus a terminator (lou_translateString.c:4080). +/// +public class HyphenateTests +{ + private const string Word = "bogstaver"; + + private static readonly string[] Tables = ["da-dk-braillo.dis", "da-dk-g26.ctb"]; + + private static string[] TablePaths() => + [.. Tables.Select(t => Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "nota-tables", t))]; + + [Fact] + public void Hyphenate_ReturnsOneHyphenationFlagPerCharacter() + { + string hyphens = LibLouis.Instance.Hyphenate(TablePaths(), Word, TranslationMode.Regular); + + Assert.Equal(Word.Length, hyphens.Length); + Assert.Matches("^[012]+$", hyphens); + + // "bog-sta-ver": the table has to find at least one break, otherwise this test is not + // exercising hyphenation at all. + Assert.Contains('1', hyphens); + } + + /// + /// The result must describe the word that was passed in, not a NUL terminator the wrapper + /// added. Unlike lou_translateString, lou_hyphenate does not clamp inlen at the first NUL: + /// it memcpy's exactly inlen characters, so an inflated inlen hyphenates the terminator too. + /// + [Fact] + public void Hyphenate_DoesNotIncludeTheNulTerminator() + { + foreach (string word in new[] { "a", "bo", "bogstaver", "hyphenation" }) + { + string hyphens = LibLouis.Instance.Hyphenate(TablePaths(), word, TranslationMode.Regular); + + Assert.Equal(word.Length, hyphens.Length); + } + } + + /// + /// inlen is a widechar count. On a UCS-4 build a non-BMP character is one widechar but two + /// chars, so passing string.Length claims the buffer is longer than it is - and lou_hyphenate + /// memcpy's exactly inlen widechars out of it, with no terminator to stop at. + /// + [Theory] + [InlineData("bogstaver\U0001D11E")] // one flag too many + [InlineData("bogstaver\U0001D11E\U0001D11E")] // and reads past the input buffer + public void Hyphenate_ReturnsOneFlagPerWidecharNotPerCodeUnit(string word) + { + int expected = SafeNativeMethods.lou_charSize() == 4 + ? word.EnumerateRunes().Count() + : word.Length; + + string hyphens = LibLouis.Instance.Hyphenate(TablePaths(), word, TranslationMode.Regular); + + Assert.Equal(expected, hyphens.Length); + } +} diff --git a/LibLouis.NET.Test/IndexTablesTests.cs b/LibLouis.NET.Test/IndexTablesTests.cs new file mode 100644 index 0000000..7bfac4e --- /dev/null +++ b/LibLouis.NET.Test/IndexTablesTests.cs @@ -0,0 +1,58 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; + +using Microsoft.Extensions.Logging; + +using Xunit; + +namespace LibLouis.NET.Test; + +/// +/// lou_indexTables walks its argument until it hits a NULL pointer +/// (for (table = tables; *table; table++), metadata.c:905). A managed string[] marshals to +/// exactly Length pointers with no terminator, so liblouis reads past the end of the array. +/// +public class IndexTablesTests +{ + private static readonly string[] Tables = ["da-dk-g26.ctb", "da-dk-g16-markers.ctb"]; + + private static string[] TablePaths() => + [.. Tables.Select(t => Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "nota-tables", t))]; + + /// + /// liblouis logs one "Analyzing table <name>" line per array entry it walks, so the number + /// of those lines is a direct measure of how far it read. + /// + /// + /// Without the terminator this does not fail, it hangs: liblouis reads the managed memory + /// following the array as a char* and _lou_logMessage formats it with %s until it runs out of + /// readable memory. A regression here shows up as a test run that never finishes. + /// + [Fact] + public void IndexTables_DoesNotReadPastTheEndOfTheArray() + { + string[] paths = TablePaths(); + + CollectingLogger logger = new(); + ILogger previous = LibLouis.Instance.Logger; + LibLouis.Instance.Logger = logger; + + try + { + LibLouis.Instance.IndexTables(paths); + } + finally + { + LibLouis.Instance.Logger = previous; + } + + List analyzed = [.. logger.Messages + .Where(m => m.StartsWith("Analyzing table ", StringComparison.Ordinal)) + .Select(m => m["Analyzing table ".Length..])]; + + Assert.Equal(paths, analyzed); + } + +} diff --git a/LibLouis.NET.Test/InputLengthTests.cs b/LibLouis.NET.Test/InputLengthTests.cs new file mode 100644 index 0000000..605a37e --- /dev/null +++ b/LibLouis.NET.Test/InputLengthTests.cs @@ -0,0 +1,82 @@ +using System; +using System.IO; +using System.Linq; + +using Xunit; + +namespace LibLouis.NET.Test; + +/// +/// inlen is a widechar count that excludes the NUL terminator, matching the header and what +/// upstream callers pass. These tests pin down the two properties that depend on it: +/// +/// * The terminator is not translated as if it were text. The buffer stays NUL terminated +/// (PrepareUCSInputBuffer's job) and lou_translateString clamps at the first NUL +/// (while (k < *inlen && inbufx[k]) k++;, lou_translateString.c:1191), so an +/// embedded NUL still ends the input. +/// * Nothing is written past the position arrays the argument checks demand. liblouis +/// overwrites *inlen with the number of characters actually consumed +/// (lou_translateString.c:1354) before computing outputPos. +/// +/// The wrapper previously passed input.Length + 1 here. That was safe - the clamp at :1191 and +/// the overwrite at :1354 between them made the extra count unreachable - but it left +/// correctness resting on two undocumented internals instead of the documented contract. +/// +public class InputLengthTests +{ + private const string Input = "Første linje. Anden linje, med kursiveret tekst. Tredje linje."; + + private static readonly string[] Tables = ["da-dk-braillo.dis", "da-dk-g26.ctb"]; + + private static string[] TablePaths() => + [.. Tables.Select(t => Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "nota-tables", t))]; + + /// + /// Translate() only requires outputPosition to hold input.Length entries, so liblouis must + /// not write beyond that. The array is deliberately oversized and sentinel filled, so an + /// out-of-bounds write would be observable here instead of corrupting the heap. + /// + [Fact] + public void Translate_DoesNotWriteOutputPositionsPastInputLength() + { + // Not -1: liblouis pre-fills outputPos with -1 for the characters it owns, so -1 could + // not tell an untouched entry apart from one liblouis had written. + const int sentinel = int.MinValue; + const int slack = 8; + + int outputLength = Input.Length * 4; + + int[] outputPosition = new int[Input.Length + slack]; + Array.Fill(outputPosition, sentinel); + + LibLouis.Instance.Translate( + TablePaths(), + Input, + outputLength, + null, + null, + outputPosition, + new int[outputLength], + 0, + TranslationMode.Regular); + + int firstUntouched = Array.FindIndex(outputPosition, p => p == sentinel); + + Assert.Equal(Input.Length, firstUntouched); + } + + /// + /// The NUL terminator is not translated as if it were input text. + /// + [Fact] + public void Translate_DoesNotTranslateTheNulTerminator() + { + const string input = "abc"; + + string translated = LibLouis.Instance.Translate( + TablePaths(), input, input.Length * 4, null, null, TranslationMode.Regular); + + Assert.DoesNotContain('\0', translated); + Assert.Equal(input, translated); + } +} diff --git a/LibLouis.NET.Test/LibLouis.NET.Test.csproj b/LibLouis.NET.Test/LibLouis.NET.Test.csproj index 8dee374..40fc277 100644 --- a/LibLouis.NET.Test/LibLouis.NET.Test.csproj +++ b/LibLouis.NET.Test/LibLouis.NET.Test.csproj @@ -5,6 +5,8 @@ enable false true + + true @@ -27,7 +29,31 @@ - + + + PreserveNewest + + + + PreserveNewest diff --git a/LibLouis.NET.Test/LogCallbackTests.cs b/LibLouis.NET.Test/LogCallbackTests.cs new file mode 100644 index 0000000..507c7bc --- /dev/null +++ b/LibLouis.NET.Test/LogCallbackTests.cs @@ -0,0 +1,70 @@ +using System; + +using Microsoft.Extensions.Logging; + +using Xunit; + +namespace LibLouis.NET.Test; + +/// +/// liblouis keeps the function pointer it is handed by lou_registerLogCallback and calls it for +/// the rest of the process's life. The managed delegate behind that pointer therefore has to stay +/// alive for just as long: the marshalling stub only keeps it alive for the duration of the +/// registration call itself. +/// +public class LogCallbackTests +{ + /// + /// Forces collections between registering the callback and provoking a native log message. + /// If nothing roots the delegate, the pointer liblouis holds is dangling by then. + /// + [Fact] + public void Logger_StillReceivesMessagesAfterGarbageCollection() + { + CollectingLogger logger = new(); + LibLouis.Instance.Logger = logger; + + for (int i = 0; i < 3; i++) + { + GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, blocking: true, compacting: true); + GC.WaitForPendingFinalizers(); + } + + // Any failing call makes liblouis log; a table that cannot be compiled is the simplest. + Assert.Throws( + () => LibLouis.Instance.Translate( + ["no-such-table-at-all.ctb"], "x", 8, null, null, TranslationMode.Regular)); + + Assert.NotEmpty(logger.Messages); + } + + /// + /// The callback runs on a native stack. An exception thrown out of it cannot be handled by + /// liblouis and tears the process down, so an unmapped level must not throw. + /// + [Fact] + public void LogCallback_SurvivesALevelItDoesNotKnow() + { + CollectingLogger logger = new(); + LibLouis.Instance.Logger = logger; + + // 12345 is not one of the logLevels values liblouis defines. + NativeMethods.LoggingCallback callback = GetRegisteredCallback(); + + callback((LogLevel)12345, "message at an unknown level"); + } + + /// + /// Reaches the delegate the wrapper registered, so the test calls exactly what liblouis calls. + /// + private static NativeMethods.LoggingCallback GetRegisteredCallback() + { + object? field = typeof(LibLouis) + .GetField("_logCallback", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance) + ?.GetValue(LibLouis.Instance); + + Assert.NotNull(field); + + return (NativeMethods.LoggingCallback)field; + } +} diff --git a/LibLouis.NET.Test/NativeLockTests.cs b/LibLouis.NET.Test/NativeLockTests.cs new file mode 100644 index 0000000..48ce8b8 --- /dev/null +++ b/LibLouis.NET.Test/NativeLockTests.cs @@ -0,0 +1,196 @@ +using System; +using System.Collections.Concurrent; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Threading; +using System.Threading.Tasks; + +using Xunit; + +namespace LibLouis.NET.Test; + +/// +/// liblouis is not thread safe and its state is process-global, so every native call in the +/// assembly has to serialise on one lock - including the ones that do not obviously touch shared +/// state. +/// +/// +/// Holding the lock is not directly observable: lou_version returns a static string and the +/// Logging setters are single pointer-sized writes, so an unsynchronised build does not reliably +/// misbehave. These tests therefore guard the two things that are observable - that no native +/// entry point was left outside the lock, and that adding the lock did not introduce a deadlock +/// or change behaviour. +/// +public class NativeLockTests +{ + private static readonly string[] Tables = ["da-dk-braillo.dis", "da-dk-g26.ctb"]; + + private static string[] TablePaths() => + [.. Tables.Select(t => Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "nota-tables", t))]; + + [Fact] + public void VersionIsReported() + { + Assert.False(string.IsNullOrWhiteSpace(LibLouis.Instance.Version)); + } + + [Fact] + public void LogLevelRoundTrips() + { + LogLevel previous = Logging.LogLevel; + + try + { + Logging.LogLevel = LogLevel.Warning; + Assert.Equal(LogLevel.Warning, Logging.LogLevel); + } + finally + { + Logging.LogLevel = previous; + } + } + + /// + /// The lock is shared between LibLouis and the static Logging helper, and Monitor is + /// reentrant, so hammering all three from several threads must neither deadlock nor produce a + /// wrong translation. + /// + [Fact] + public async Task ConcurrentUseDoesNotDeadlockOrCorrupt() + { + const string input = "Første linje"; + const string expected = "@fze linje"; + + LogLevel previous = Logging.LogLevel; + + using CancellationTokenSource cts = new(TimeSpan.FromSeconds(5)); + + ConcurrentBag failures = []; + + try + { + Task[] workers = + [ + .. Enumerable.Range(0, 4).Select(_ => Task.Run(() => + { + while (!cts.IsCancellationRequested) + { + string result = LibLouis.Instance.Translate( + TablePaths(), input, 64, null, null, TranslationMode.Regular); + + if (result != expected) + { + failures.Add($"translation returned '{result}'"); + return; + } + } + })), + Task.Run(() => + { + while (!cts.IsCancellationRequested) + { + _ = LibLouis.Instance.Version; + Logging.LogLevel = LogLevel.Error; + } + }), + ]; + + Task all = Task.WhenAll(workers); + + Assert.Same( + all, + await Task.WhenAny(all, Task.Delay(TimeSpan.FromSeconds(30)))); + + await all; + } + finally + { + Logging.LogLevel = previous; + } + + Assert.Empty(failures); + } + + /// + /// Catches a native call added later without the lock. Deliberately source-based: there is no + /// runtime signal for "this P/Invoke ran unsynchronised". + /// + /// + /// A call that genuinely does not need the lock has to say so, by carrying an "unlocked:" + /// comment giving the reason. That keeps the exemptions few and explains each one, instead of + /// letting the test quietly special-case whole methods. + /// + [Theory] + [InlineData("LibLouis.cs")] + [InlineData("Logging.cs")] + public void EveryNativeCallSiteIsLockedOrJustified(string fileName) + { + string[] lines = ReadLibrarySource(fileName).Split('\n'); + + int depth = 0; + int lockDepth = -1; + bool pendingLock = false; + + for (int i = 0; i < lines.Length; i++) + { + string line = lines[i].Trim(); + + // The body starts at the brace on the next line, so record the depth once we are + // actually inside it rather than on the "lock (" line itself. + if (line.StartsWith("lock (", StringComparison.Ordinal)) + { + pendingLock = true; + } + + bool isNativeCall = line.Contains("NativeMethods.", StringComparison.Ordinal) + && !line.StartsWith("//", StringComparison.Ordinal) + && !line.StartsWith("///", StringComparison.Ordinal) + && !line.Contains("NativeMethods.LoggingCallback", StringComparison.Ordinal); + + if (isNativeCall && lockDepth < 0) + { + bool justified = lines + .Take(i) + .Reverse() + .TakeWhile(l => l.Trim().StartsWith("//", StringComparison.Ordinal)) + .Any(l => l.Contains("unlocked:", StringComparison.Ordinal)); + + Assert.True(justified, $"{fileName}: native call outside a lock: {line}"); + } + + int updated = depth + lines[i].Count(c => c == '{') - lines[i].Count(c => c == '}'); + + if (pendingLock && updated > depth) + { + lockDepth = updated; + pendingLock = false; + } + + depth = updated; + + if (lockDepth >= 0 && depth < lockDepth) + { + lockDepth = -1; + } + } + } + + private static string ReadLibrarySource(string fileName) + { + DirectoryInfo? directory = new(AppContext.BaseDirectory); + + while (directory is not null && !File.Exists(Path.Combine(directory.FullName, "LibLouis.NET.sln"))) + { + directory = directory.Parent; + } + + Assert.NotNull(directory); + + string path = Path.Combine(directory.FullName, "LibLouis.NET", fileName); + + Assert.True(File.Exists(path), $"could not locate {path}"); + + return File.ReadAllText(path); + } +} diff --git a/LibLouis.NET.Test/NativeMethodsTests.cs b/LibLouis.NET.Test/NativeMethodsTests.cs index a203486..0bb2032 100644 --- a/LibLouis.NET.Test/NativeMethodsTests.cs +++ b/LibLouis.NET.Test/NativeMethodsTests.cs @@ -23,7 +23,7 @@ public void SingleMode() Array.Fill(modes, TypeForm.ForeignLanguage); string resultString = LibLouis.Instance.Translate( - tables.Select(t => Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "tables", t)), + tables.Select(t => Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "nota-tables", t)), input, outputLength, modes, @@ -48,7 +48,7 @@ public void NestedMode_EmphasisInForeignLanguage() modes[6] = TypeForm.ForeignLanguage | TypeForm.Emphasis; string resultString = LibLouis.Instance.Translate( - tables.Select(t => Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "tables", t)), + tables.Select(t => Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "nota-tables", t)), input, outputLength, modes, @@ -89,7 +89,7 @@ public void NestedMode_ForeignLanguageInEmphasis() Assert.Equal(inputLength, modes.Length); string resultString = LibLouis.Instance.Translate( - tables.Select(t => Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "tables", t)), + tables.Select(t => Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "nota-tables", t)), input, outputLength, modes, @@ -105,7 +105,7 @@ public void TestPositionResults() const string input = "Første linje. Anden linje, med kursiveret tekst. Tredje linje."; const string expected = "@fze linje. @anç linje, m kursi#rò ükz. @tàdje linje."; - string[] tables = ["tables/da-dk-braillo.dis", "tables/da-dk-g26.ctb"]; + string[] tables = ["nota-tables/da-dk-braillo.dis", "nota-tables/da-dk-g26.ctb"]; int outputLength = input.Length * 4; int cursorPosition = 0; @@ -116,7 +116,10 @@ public void TestPositionResults() Assert.Equal(expected, translated.Output); - Assert.Equal(inputPosition, translated.InputPosition); + // The returned arrays are sized to the strings they index rather than to the scratch + // buffers passed in, so InputPosition covers the output and no slicing is needed to use + // it. For this BMP input the values are unchanged from what liblouis wrote. + Assert.Equal(inputPosition[..translated.Output.Length], translated.InputPosition); Assert.Equal(outputPosition, translated.OutputPosition); Assert.Equal('A', input[inputPosition[12]]); diff --git a/LibLouis.NET.Test/NonBmpTests.cs b/LibLouis.NET.Test/NonBmpTests.cs new file mode 100644 index 0000000..18a5999 --- /dev/null +++ b/LibLouis.NET.Test/NonBmpTests.cs @@ -0,0 +1,70 @@ +using System; +using System.IO; +using System.Linq; + +using Xunit; + +namespace LibLouis.NET.Test; + +/// +/// On a UCS-4 build a liblouis widechar holds a whole Unicode character, so a non-BMP character +/// occupies one widechar but two chars of a .NET string. Passing string.Length as a widechar +/// count therefore overstates the length of the buffer. +/// +/// lou_translateString survives that, because it clamps at the NUL terminator. lou_dotsToChar and +/// lou_charToDots do not clamp: they read and write exactly the count they are given +/// (lou_translateString.c:4142), so a count in the wrong unit reads past the input buffer. +/// +public class NonBmpTests +{ + /// U+1D11E MUSICAL SYMBOL G CLEF - one character, two UTF-16 code units. + private const string NonBmp = "\U0001D11E\U0001D11E"; + + private static readonly string[] Tables = ["da-dk-braillo.dis", "da-dk-g26.ctb"]; + + private static string[] TablePaths() => + [.. Tables.Select(t => Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "nota-tables", t))]; + + /// + /// How many widechars liblouis sees for : whole characters on a UCS-4 + /// build, UTF-16 code units on a UCS-2 one. + /// + private static int ExpectedCells(string value) + { + return SafeNativeMethods.lou_charSize() == 4 + ? value.EnumerateRunes().Count() + : value.Length; + } + + [Fact] + public void CharactersToDots_ProducesOneCellPerWidecharNotPerCodeUnit() + { + string dots = LibLouis.Instance.CharactersToDots(TablePaths(), NonBmp); + + Assert.Equal(ExpectedCells(NonBmp), dots.Length); + } + + [Fact] + public void DotsToCharacters_ProducesOneCharacterPerCell() + { + string dots = LibLouis.Instance.CharactersToDots(TablePaths(), NonBmp); + + string roundTripped = LibLouis.Instance.DotsToCharacters(TablePaths(), dots); + + Assert.Equal(dots.Length, roundTripped.Length); + } + + /// + /// BMP text must keep behaving exactly as before: there string.Length and the widechar count + /// agree, so this guards the common case against the fix. + /// + [Fact] + public void CharactersToDots_IsUnchangedForBmpText() + { + const string input = "abc"; + + string dots = LibLouis.Instance.CharactersToDots(TablePaths(), input); + + Assert.Equal(input.Length, dots.Length); + } +} diff --git a/LibLouis.NET.Test/NotaTablesTests.cs b/LibLouis.NET.Test/NotaTablesTests.cs new file mode 100644 index 0000000..dc9d247 --- /dev/null +++ b/LibLouis.NET.Test/NotaTablesTests.cs @@ -0,0 +1,98 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text.RegularExpressions; + +using Xunit; + +namespace LibLouis.NET.Test; + +/// +/// Guards the nota-tables/ directory, which the tests use in place of the upstream set. +/// +public class NotaTablesTests +{ + private static readonly string NotaTableDirectory = + Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "nota-tables"); + + private static readonly string UpstreamTableDirectory = + Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "tables"); + + /// + /// liblouis resolves an include relative to the directory of the table doing the including, so + /// every table nota-tables/ pulls in has to be there too. Most are Nota's own; the handful of + /// general upstream tables they need are copied in by an explicit list in the csproj, and an + /// explicit list is exactly the kind of thing that goes stale the moment a table gains an + /// include. + /// + [Fact] + public void NotaTablesAreSelfContained() + { + var present = TableFilesIn(NotaTableDirectory).ToHashSet(StringComparer.Ordinal); + var missing = new SortedSet(StringComparer.Ordinal); + + foreach (string file in present) + { + foreach (string included in IncludesOf(Path.Combine(NotaTableDirectory, file))) + { + if (!present.Contains(included)) + { + missing.Add($"{included} (included by {file})"); + } + } + } + + Assert.True( + missing.Count == 0, + "nota-tables/ is missing tables it includes, so those cannot be compiled from that " + + "directory alone. Add them to the upstream copy list in the csproj:\n " + + string.Join("\n ", missing)); + } + + /// + /// The separate directory exists so that nothing of Nota's shadows the upstream set. Sharing a + /// file name across the two directories is expected and fine - twenty two of Nota's tables are + /// forks of an upstream table of the same name. What must never happen is Nota's tables being + /// copied into tables/ as well, which is what the old single directory did. + /// + /// The tables that exist nowhere upstream are the reliable canary: if one of them turns up in + /// tables/, the copy is misconfigured, whatever the file names say. + /// + [Fact] + public void NotaTablesDoNotLeakIntoTheUpstreamDirectory() + { + Assert.True(Directory.Exists(UpstreamTableDirectory), "the upstream tables were not staged"); + + string[] notaOnly = + [ + "da-dk-g16-markers.ctb", + "da-dk-g16_1993-markers.ctb", + "da-dk-braillo.dis", + "da-dk-g16-crossword.ctb", + "da-dk-g26l.ctb", + "da-dk-g26l-lit.ctb", + "da-dk-g28l.ctb", + "da-dk-g2_1993.dic", + ]; + + var upstream = TableFilesIn(UpstreamTableDirectory).ToHashSet(StringComparer.Ordinal); + var leaked = notaOnly.Where(upstream.Contains).OrderBy(f => f, StringComparer.Ordinal).ToList(); + + Assert.True( + leaked.Count == 0, + "tables/ should hold only the upstream set, but these tables of Nota's are in it, so " + + "the two are being copied to the same place again:\n " + string.Join("\n ", leaked)); + } + + private static IEnumerable TableFilesIn(string directory) => + Directory.EnumerateFiles(directory) + .Select(Path.GetFileName) + .Where(f => f is not null && !f.EndsWith(".md", StringComparison.OrdinalIgnoreCase))!; + + private static IEnumerable IncludesOf(string path) => + File.ReadLines(path) + .Select(line => Regex.Match(line, @"^\s*include\s+(\S+)")) + .Where(m => m.Success) + .Select(m => m.Groups[1].Value); +} diff --git a/LibLouis.NET.Test/OutputDotsTests.cs b/LibLouis.NET.Test/OutputDotsTests.cs new file mode 100644 index 0000000..6ce3373 --- /dev/null +++ b/LibLouis.NET.Test/OutputDotsTests.cs @@ -0,0 +1,101 @@ +using System; +using System.IO; +using System.Linq; + +using Xunit; + +namespace LibLouis.NET.Test; + +/// +/// On a successful forward translation liblouis reports, per output cell, whether the cell +/// contains dot 7 or dot 8 (lou_translateString.c:1330). It writes that into the typeform +/// buffer - which is why the buffer has to be output-sized, and why the caller's input-sized +/// array must not receive it. The wrapper surfaces the information on TranslatedString instead, +/// so callers get it without the write-past-the-end hazard. +/// +public class OutputDotsTests +{ + private static readonly string[] Tables = ["da-dk-braillo.dis", "da-dk-g08.ctb"]; + + private static string[] EightDotTables() => + [.. Tables.Select(t => Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "nota-tables", t))]; + + private static TranslatedString TranslateWithTypeForm(string input) + { + TypeForm[] typeform = new TypeForm[input.Length]; + + return LibLouis.Instance.Translate( + EightDotTables(), + input, + input.Length * 4, + typeform, + null, + new int[input.Length], + new int[input.Length * 4], + 0, + TranslationMode.Regular); + } + + /// + /// Danish 8-dot braille marks a capital with dot 7 on the letter's own cell, so casing gives + /// a per-cell pattern we can predict: the flag must differ between the capital and the small + /// letters. + /// + [Fact] + public void ReportsDot7OnCapitalCells() + { + TranslatedString result = TranslateWithTypeForm("Abc"); + + Assert.NotNull(result.OutputDots78); + Assert.Equal(result.Output.Length, result.OutputDots78.Length); + + Assert.True(result.OutputDots78[0], "capital A should carry dot 7 in an 8-dot table"); + Assert.All(result.OutputDots78.Skip(1), d => Assert.False(d, "small letters should not")); + } + + /// + /// liblouis only computes the information when a typeform buffer is supplied, so without one + /// the property must be null rather than a fabricated all-false array. + /// + [Fact] + public void IsNullWhenNoTypeFormWasPassed() + { + TranslatedString result = LibLouis.Instance.Translate( + EightDotTables(), + "Abc", + 16, + null, + null, + new int[3], + new int[16], + 0, + TranslationMode.Regular); + + Assert.Null(result.OutputDots78); + } + + /// + /// The safety half of the contract, restated from the caller's side: surfacing the output + /// information must not bring back the write-back into the caller's array. + /// + [Fact] + public void CallersArrayStaysUntouched() + { + TypeForm[] typeform = new TypeForm[3]; + Array.Fill(typeform, TypeForm.Italic); + + TranslatedString result = LibLouis.Instance.Translate( + EightDotTables(), + "Abc", + 16, + typeform, + null, + new int[3], + new int[16], + 0, + TranslationMode.Regular); + + Assert.NotNull(result.OutputDots78); + Assert.All(typeform, t => Assert.Equal(TypeForm.Italic, t)); + } +} diff --git a/LibLouis.NET.Test/PositionMappingTests.cs b/LibLouis.NET.Test/PositionMappingTests.cs new file mode 100644 index 0000000..9ec1d80 --- /dev/null +++ b/LibLouis.NET.Test/PositionMappingTests.cs @@ -0,0 +1,181 @@ +using System; +using System.IO; +using System.Linq; + +using Xunit; + +namespace LibLouis.NET.Test; + +/// +/// liblouis indexes its position arrays in widechars - whole Unicode characters on our UCS-4 +/// builds. .NET callers read them as indices into a string, which is UTF-16. The two agree for +/// BMP text and diverge on the first non-BMP character, so the wrapper translates them. +/// +/// +/// The invariant that matters is not "the numbers look right" but that every value is directly +/// usable as a string index: Output[result.InputPosition[k]] must address the character +/// liblouis meant, and must never land on the trailing half of a surrogate pair. +/// +public class PositionMappingTests +{ + private static readonly string[] Tables = ["da-dk-braillo.dis", "da-dk-g26.ctb"]; + + private static string[] TablePaths() => + [.. Tables.Select(t => Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "nota-tables", t))]; + + private static TranslatedString Translate(string input) + { + int outputLength = Math.Max(16, input.Length * 4); + + return LibLouis.Instance.Translate( + TablePaths(), + input, + outputLength, + null, + null, + new int[input.Length], + new int[outputLength], + 0, + TranslationMode.Regular); + } + + /// + /// Sized to the strings they index, so no slicing guesswork is needed. + /// + [Theory] + [InlineData("Første linje. Anden linje.")] + [InlineData("bogstaver")] + [InlineData("a\U0001D11Eb")] + [InlineData("\U0001D11E\U0001D11E")] + [InlineData("😀 hej")] + public void ArraysAreSizedToTheStringsTheyIndex(string input) + { + TranslatedString result = Translate(input); + + Assert.Equal(input.Length, result.OutputPosition.Length); + Assert.Equal(result.Output.Length, result.InputPosition.Length); + } + + /// + /// Every InputPosition value must be a usable index into the input string, and must address + /// the start of a character rather than the low half of a surrogate pair. + /// + [Theory] + [InlineData("Første linje. Anden linje.")] + [InlineData("a\U0001D11Eb")] + [InlineData("\U0001D11E\U0001D11E")] + [InlineData("😀 hej")] + public void InputPositionsAddressWholeCharactersOfTheInput(string input) + { + TranslatedString result = Translate(input); + + foreach (int position in result.InputPosition) + { + Assert.InRange(position, 0, input.Length - 1); + Assert.False( + char.IsLowSurrogate(input[position]), + $"position {position} lands on the trailing half of a surrogate pair"); + } + } + + /// + /// The same, in the other direction. + /// + [Theory] + [InlineData("Første linje. Anden linje.")] + [InlineData("a\U0001D11Eb")] + [InlineData("😀 hej")] + public void OutputPositionsAddressWholeCharactersOfTheOutput(string input) + { + TranslatedString result = Translate(input); + + foreach (int position in result.OutputPosition) + { + Assert.InRange(position, 0, result.Output.Length - 1); + Assert.False( + char.IsLowSurrogate(result.Output[position]), + $"position {position} lands on the trailing half of a surrogate pair"); + } + } + + /// + /// Both halves of a surrogate pair belong to the same character, so both must report the same + /// braille cell. + /// + [Fact] + public void SurrogatePairHalvesShareAnOutputPosition() + { + const string input = "a\U0001D11Eb"; + + TranslatedString result = Translate(input); + + // index 1 and 2 are the two halves of U+1D11E + Assert.Equal(result.OutputPosition[1], result.OutputPosition[2]); + + // and the surrounding BMP characters map elsewhere + Assert.NotEqual(result.OutputPosition[0], result.OutputPosition[1]); + } + + /// + /// BMP text must be completely unaffected: widechar and UTF-16 indices coincide there, so the + /// values have to match what liblouis wrote into the caller's scratch array. + /// + [Fact] + public void BmpTextIsUnchanged() + { + const string input = "Første linje. Anden linje, med kursiveret tekst. Tredje linje."; + + int outputLength = input.Length * 4; + + int[] scratchOutput = new int[input.Length]; + int[] scratchInput = new int[outputLength]; + + TranslatedString result = LibLouis.Instance.Translate( + TablePaths(), input, outputLength, null, null, scratchOutput, scratchInput, 0, TranslationMode.Regular); + + Assert.Equal(scratchOutput, result.OutputPosition); + Assert.Equal(scratchInput[..result.Output.Length], result.InputPosition); + } + + /// + /// The pattern that consumer code actually uses, which was correct only for BMP text. + /// + [Theory] + [InlineData("Første linje")] + [InlineData("a\U0001D11Eb")] + public void ConsumerSlicePatternStaysInBounds(string input) + { + TranslatedString result = Translate(input); + + int[] sliced = result.InputPosition[..result.Output.Length]; + + Assert.Equal(result.InputPosition.Length, sliced.Length); + Assert.All(sliced, p => Assert.InRange(p, 0, input.Length - 1)); + } + + /// + /// The cursor comes back as an index into the braille output, so it has to be translated too. + /// + [Fact] + public void CursorPositionIsAnIndexIntoTheOutput() + { + const string input = "a\U0001D11Ebc"; + + int outputLength = input.Length * 4; + + // Cursor on 'b', which sits after the surrogate pair. + TranslatedString result = LibLouis.Instance.Translate( + TablePaths(), + input, + outputLength, + null, + null, + new int[input.Length], + new int[outputLength], + input.IndexOf('b', StringComparison.Ordinal), + TranslationMode.Regular); + + Assert.InRange(result.CursorPosition, 0, result.Output.Length - 1); + Assert.False(char.IsLowSurrogate(result.Output[result.CursorPosition])); + } +} diff --git a/LibLouis.NET.Test/ReturnedStringOwnershipTests.cs b/LibLouis.NET.Test/ReturnedStringOwnershipTests.cs new file mode 100644 index 0000000..4c3a811 --- /dev/null +++ b/LibLouis.NET.Test/ReturnedStringOwnershipTests.cs @@ -0,0 +1,41 @@ +using System; +using System.IO; + +using Xunit; + +namespace LibLouis.NET.Test; + +/// +/// liblouis owns every string it returns, so the wrapper must not hand those pointers to the +/// marshaller's Free. +/// +/// * lou_setDataPath / lou_getDataPath return a pointer into a static char[MAXSTRING] inside +/// liblouis (compileTranslationTable.c:59-73). Passing that to free() is undefined behaviour +/// on every platform. +/// * lou_findTable returns malloc'd memory. Our Windows binaries are built with mingw-w64 and +/// allocate from msvcrt.dll, while .NET frees through ucrtbase.dll - different heaps, so +/// freeing it from managed code corrupts the heap there. +/// +public class ReturnedStringOwnershipTests +{ + /// + /// Setting the data path returns the static buffer, which the marshaller would then free. + /// + /// + /// The path is the test output directory rather than something arbitrary, because the data + /// path takes part in resolving relative table names and other tests rely on that. + /// + [Fact] + public void DataPath_RoundTripsWithoutFreeingLiblouisMemory() + { + string path = Path.TrimEndingDirectorySeparator(AppContext.BaseDirectory); + + LibLouis.Instance.DataPath = path; + + Assert.Equal(path, LibLouis.Instance.DataPath); + + // Reading it again returns the same static buffer; a stale free shows up here as a crash + // or as garbage. + Assert.Equal(path, LibLouis.Instance.DataPath); + } +} diff --git a/LibLouis.NET.Test/SafeNativeMethods.cs b/LibLouis.NET.Test/SafeNativeMethods.cs new file mode 100644 index 0000000..0bcf339 --- /dev/null +++ b/LibLouis.NET.Test/SafeNativeMethods.cs @@ -0,0 +1,41 @@ +using System.Runtime.InteropServices; +using System.Text; + +namespace LibLouis.NET.Test; + +/// +/// Raw P/Invoke used to characterise native behaviour without going through the wrapper. +/// +/// +/// Strings are passed as pre-encoded NUL terminated UTF-8 rather than as managed strings, so +/// there is no marshalling behaviour of our own between the test and liblouis. +/// +internal static class SafeNativeMethods +{ + /// + /// Bytes per liblouis widechar: 2 for a UCS-2 build, 4 for UCS-4. + /// + [DefaultDllImportSearchPaths(DllImportSearchPath.SafeDirectories)] + [DllImport("liblouis", EntryPoint = "lou_charSize")] + internal static extern int lou_charSize(); + + [DefaultDllImportSearchPaths(DllImportSearchPath.SafeDirectories)] + [DllImport("liblouis", EntryPoint = "lou_translateString")] + internal static extern int lou_translateString( + byte[] tableList, + byte[] inbuf, + ref int inlen, + byte[] outbuf, + ref int outlen, + ushort[]? typeform, + byte[]? spacing, + int mode); + + /// + /// Encodes a string the way liblouis expects a const char *. + /// + internal static byte[] Utf8(string value) + { + return Encoding.UTF8.GetBytes(value + "\0"); + } +} diff --git a/LibLouis.NET.Test/ShutdownTests.cs b/LibLouis.NET.Test/ShutdownTests.cs new file mode 100644 index 0000000..f7ccec0 --- /dev/null +++ b/LibLouis.NET.Test/ShutdownTests.cs @@ -0,0 +1,170 @@ +using System; +using System.IO; +using System.Linq; +using System.Reflection; + +using Xunit; + +namespace LibLouis.NET.Test; + +/// +/// Shutdown frees liblouis's translation and display table chains, which are global to the +/// process. It is deliberately not IDisposable: nothing here is owned by a single caller, so +/// there is no "done with it" moment to hang disposal off, and an accidental +/// using (LibLouis.Instance) would tear liblouis down for everything else in the process. +/// +/// +/// These tests set the flag directly instead of calling Shutdown. LibLouis is a process-wide +/// singleton and the suite runs serially in one process, so really shutting it down would fail +/// every test that ran afterwards. The flag is restored in a finally for the same reason. +/// End-to-end shutdown is exercised out of process. +/// +public class ShutdownTests +{ + private static readonly string[] Tables = ["da-dk-braillo.dis", "da-dk-g26.ctb"]; + + private static string[] TablePaths() => + [.. Tables.Select(t => Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "nota-tables", t))]; + + private static FieldInfo ShutDownField => + typeof(LibLouis).GetField("_shutDown", BindingFlags.NonPublic | BindingFlags.Static) + ?? throw new InvalidOperationException("_shutDown field not found"); + + /// + /// The shape change itself: an accidental using statement must not compile. + /// + [Fact] + public void LibLouisIsNotDisposable() + { + Assert.False(typeof(IDisposable).IsAssignableFrom(typeof(LibLouis))); + } + + /// + /// Shutdown is process-wide teardown, so it belongs on the type, not on an instance nobody + /// exclusively owns. + /// + [Fact] + public void ShutdownIsStatic() + { + MethodInfo? shutdown = typeof(LibLouis).GetMethod( + "Shutdown", BindingFlags.Public | BindingFlags.Static, Type.EmptyTypes); + + Assert.NotNull(shutdown); + Assert.Equal(typeof(void), shutdown.ReturnType); + } + + [Fact] + public void UsingTheInstanceAfterShutdownThrows() + { + WhileMarkedShutDown(() => + { + Assert.Throws( + () => LibLouis.Instance.Translate(TablePaths(), "abc", 16, null, null, TranslationMode.Regular)); + + Assert.Throws( + () => LibLouis.Instance.Translate( + TablePaths(), "abc", 16, null, null, new int[16], new int[16], 0, TranslationMode.Regular)); + + Assert.Throws( + () => LibLouis.Instance.BackTranslate(TablePaths(), "abc", 16, null, null, TranslationMode.Regular)); + + Assert.Throws( + () => LibLouis.Instance.CharactersToDots(TablePaths(), "abc")); + + Assert.Throws( + () => LibLouis.Instance.DotsToCharacters(TablePaths(), "abc")); + + Assert.Throws( + () => LibLouis.Instance.Hyphenate(TablePaths(), "bogstaver", TranslationMode.Regular)); + + Assert.Throws( + () => LibLouis.Instance.IndexTables(TablePaths())); + + Assert.Throws( + () => LibLouis.Instance.FindTable("type:literary")); + }); + } + + /// + /// The message has to say what happened: "cannot access a disposed object" would be a lie for + /// a type that is not disposable. + /// + [Fact] + public void TheFailureExplainsItself() + { + WhileMarkedShutDown(() => + { + InvalidOperationException e = Assert.Throws( + () => LibLouis.Instance.CharactersToDots(TablePaths(), "abc")); + + Assert.Contains("shut down", e.Message, StringComparison.OrdinalIgnoreCase); + }); + } + + /// + /// Diagnostics stay available: neither touches anything lou_free released. + /// + [Fact] + public void VersionAndLoggerSurviveShutdown() + { + WhileMarkedShutDown(() => + { + Assert.False(string.IsNullOrWhiteSpace(LibLouis.Instance.Version)); + + LibLouis.Instance.Logger = new CollectingLogger(); + }); + } + + /// + /// The instance works again once the flag is cleared, so the guard is the only thing stopping + /// it - the test is not just observing a broken singleton. + /// + [Fact] + public void TheGuardIsWhatBlocksUse() + { + WhileMarkedShutDown(() => + Assert.Throws( + () => LibLouis.Instance.CharactersToDots(TablePaths(), "abc"))); + + Assert.Equal(3, LibLouis.Instance.CharactersToDots(TablePaths(), "abc").Length); + } + + /// + /// lou_free is process-global, but a finalizer is per managed instance. In a collectible + /// AssemblyLoadContext that would free the tables of every other context still using them, + /// and it would do it on the finalizer thread, outside the lock. + /// + [Fact] + public void LibLouisHasNoFinalizer() + { + MethodInfo? finalizer = typeof(LibLouis) + .GetMethod("Finalize", BindingFlags.NonPublic | BindingFlags.Instance); + + Assert.Equal(typeof(object), finalizer?.DeclaringType); + } + + [Fact] + public void ShutDownFlagIsVolatile() + { + // Read outside the lock by the guards, written under it by Shutdown. + Assert.Contains( + ShutDownField.GetRequiredCustomModifiers(), + m => m == typeof(System.Runtime.CompilerServices.IsVolatile)); + } + + private static void WhileMarkedShutDown(Action body) + { + FieldInfo field = ShutDownField; + + field.SetValue(null, true); + + try + { + body(); + } + finally + { + field.SetValue(null, false); + } + } +} diff --git a/LibLouis.NET.Test/TypeFormBufferTests.cs b/LibLouis.NET.Test/TypeFormBufferTests.cs new file mode 100644 index 0000000..3d01cf2 --- /dev/null +++ b/LibLouis.NET.Test/TypeFormBufferTests.cs @@ -0,0 +1,129 @@ +using System; +using System.IO; +using System.Linq; +using System.Runtime.InteropServices; +using System.Text; + +using Xunit; + +namespace LibLouis.NET.Test; + +/// +/// liblouis writes the typeform array back for every *output* cell, not for every input +/// character. Passing a typeform array sized to the input therefore lets native code write +/// past the end of a managed array whenever the translation grows the text - which the +/// marker tables in this repository do routinely. +/// +public class TypeFormBufferTests +{ + private const string Input = "This is a test."; + + /// Translation of with the marker tables, 20 cells for 15 characters. + private const string ExpectedOutput = "`,@this is a test.`,"; + + private static readonly string[] Tables = ["da-dk-braillo.dis", "da-dk-g16-markers.ctb"]; + + private static string[] TablePaths() => + [.. Tables.Select(t => Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "nota-tables", t))]; + + /// + /// Documents the native contract that makes the overrun possible, independent of the wrapper: + /// lou_translateString writes one typeform entry per output cell. Uses a deliberately + /// oversized buffer so nothing is corrupted while we measure how far native code writes. + /// + [Fact] + public void Native_WritesOneTypeformEntryPerOutputCell() + { + int charSize = SafeNativeMethods.lou_charSize(); + Encoding encoder = charSize == 4 ? Encoding.UTF32 : Encoding.Unicode; + + int inputLength = Input.Length; + int outputLength = ExpectedOutput.Length; + + byte[] inputBuffer = encoder.GetBytes(Input + "\0"); + byte[] outputBuffer = new byte[(outputLength + 1) * charSize]; + + // Far larger than either length, so native writes stay in bounds and are observable. + // typeform is in/out: the first inputLength entries are real input (foreign language, + // matching the other tests), the rest are plain text. Neither value collides with the + // ASCII '0' / '8' that liblouis writes back, so any such entry marks a native write. + ushort[] typeform = new ushort[outputLength * 4]; + Array.Fill(typeform, (ushort)TypeForm.ForeignLanguage, 0, inputLength); + + int inLen = inputLength; + int outLen = outputLength; + + int result = SafeNativeMethods.lou_translateString( + SafeNativeMethods.Utf8(string.Join(',', TablePaths())), + inputBuffer, + ref inLen, + outputBuffer, + ref outLen, + typeform, + null, + 0); + + Assert.NotEqual(0, result); + Assert.Equal(ExpectedOutput, encoder.GetString(outputBuffer, 0, outLen * charSize)); + + // liblouis writes the ASCII characters '0' / '8' per output cell. + int lastWritten = Array.FindLastIndex(typeform, t => t == '0' || t == '8'); + + Assert.Equal(outLen - 1, lastWritten); + + // The point of the test: native wrote beyond the input length, so an input-sized + // managed array would have been overrun by exactly this many entries. + Assert.True( + lastWritten >= inputLength, + $"Expected native writes past input length {inputLength}, but last write was at {lastWritten}."); + } + + /// + /// The wrapper must not let native code write into - let alone past - the caller's typeform + /// array. The public contract sizes typeform to the input, so the wrapper owes the caller a + /// buffer big enough for the output. + /// + [Fact] + public void Translate_DoesNotWriteIntoCallersTypeformArray() + { + TypeForm[] typeform = new TypeForm[Input.Length]; + Array.Fill(typeform, TypeForm.ForeignLanguage); + + TypeForm[] untouched = (TypeForm[])typeform.Clone(); + + string output = LibLouis.Instance.Translate( + TablePaths(), Input, Input.Length * 2, typeform, null, TranslationMode.Regular); + + Assert.Equal(ExpectedOutput, output); + Assert.Equal(untouched, typeform); + } + + /// + /// The same overrun through the position-reporting overload. + /// + [Fact] + public void TranslateWithPositions_DoesNotWriteIntoCallersTypeformArray() + { + int outputLength = Input.Length * 2; + + TypeForm[] typeform = new TypeForm[Input.Length]; + Array.Fill(typeform, TypeForm.ForeignLanguage); + + TypeForm[] untouched = (TypeForm[])typeform.Clone(); + + TranslatedString translated = LibLouis.Instance.Translate( + TablePaths(), + Input, + outputLength, + typeform, + null, + new int[Input.Length], + new int[outputLength], + 0, + TranslationMode.Regular); + + Assert.Equal(ExpectedOutput, translated.Output); + Assert.Equal(untouched, typeform); + } + +} diff --git a/LibLouis.NET.Test/UTF8StringNoFreeMarshallerTests.cs b/LibLouis.NET.Test/UTF8StringNoFreeMarshallerTests.cs new file mode 100644 index 0000000..eea4522 --- /dev/null +++ b/LibLouis.NET.Test/UTF8StringNoFreeMarshallerTests.cs @@ -0,0 +1,66 @@ +using System; +using System.Runtime.InteropServices; +using System.Text; + +using Xunit; + +namespace LibLouis.NET.Test; + +/// +/// The marshaller used for strings liblouis owns. It only converts inbound: not freeing is +/// correct for memory liblouis allocated, and would be a leak for buffers we allocate ourselves, +/// so it is restricted to return values. +/// +public unsafe class UTF8StringNoFreeMarshallerTests +{ + [Theory] + [InlineData("")] + [InlineData("a")] + [InlineData("nota-tables/da-dk-g26.ctb")] + [InlineData("Første linje")] // multi-byte UTF-8 + [InlineData("\U0001D11E")] // non-BMP, surrogate pair on the managed side + public void ConvertToManaged_ReadsNulTerminatedUtf8(string value) + { + byte[] utf8 = Encoding.UTF8.GetBytes(value + "\0"); + + fixed (byte* unmanaged = utf8) + { + Assert.Equal(value, UTF8StringNoFreeMarshaller.ConvertToManaged(unmanaged)); + } + } + + /// + /// The string must stop at the terminator, not run on into whatever follows it. + /// + [Fact] + public void ConvertToManaged_StopsAtTheTerminator() + { + byte[] utf8 = Encoding.UTF8.GetBytes("abc\0trailing garbage"); + + fixed (byte* unmanaged = utf8) + { + Assert.Equal("abc", UTF8StringNoFreeMarshaller.ConvertToManaged(unmanaged)); + } + } + + [Fact] + public void ConvertToManaged_MapsNullPointerToNull() + { + Assert.Null(UTF8StringNoFreeMarshaller.ConvertToManaged(null)); + } + + /// + /// Free must leave the memory alone. If it released it, the allocator would abort on the + /// second release here. + /// + [Fact] + public void Free_DoesNotReleaseTheMemory() + { + byte* buffer = (byte*)NativeMemory.Alloc(4); + + UTF8StringNoFreeMarshaller.Free(buffer); + + // Ours to release, and still ours after Free. + NativeMemory.Free(buffer); + } +} diff --git a/LibLouis.NET.Test/nota-tables/README.md b/LibLouis.NET.Test/nota-tables/README.md new file mode 100644 index 0000000..da6339e --- /dev/null +++ b/LibLouis.NET.Test/nota-tables/README.md @@ -0,0 +1,31 @@ +# Nota's Danish tables, for the tests + +Kept in their own directory, and copied to `nota-tables/` in the output rather than `tables/`. + +`LibLouis.NET.Tables` copies the upstream table set into `tables/`. Five of the files here share a +name with an upstream table and differ from it, so a single shared directory would make which copy +a test gets depend on MSBuild item ordering. Separate directories mean every test states which set +it means, and the upstream braille specs can be checked against upstream tables without this set +shadowing them. + +## What these are + +A fork of an older upstream, not a patch on the current one. They add things upstream does not +have, such as the `foreign` emphasis class behind `TypeForm.ForeignLanguage`, and they are missing +things upstream has since fixed, such as the rules that remove the space between `§` and a following +number. Do not assume a file here matches the upstream file of the same name in either direction. + +Two of them, `da-dk-g16-markers.ctb` and `da-dk-braillo.dis`, exist nowhere upstream, which is why +the tests cannot simply use the upstream set. + +## These are not canonical + +The canonical Nota tables ship with the application. This is a copy, and a stale one: only seven of +the thirty files are reachable from any test. + +Reachable: `da-dk-g16-markers.ctb`, `da-dk-braillo.dis`, `da-dk-g08.ctb`, `da-dk-g26.ctb`, and the +three they include (`da-dk-6miscChars.cti`, `da-dk-octobraille.dis`, `da-dk-g2.dic`). + +The other twenty three are referenced by nothing. They are kept for now, but nothing keeps them in +step with the application's copy, so treat any of them as evidence of nothing. Prefer adding a test +that needs a file over adding a file that no test needs. diff --git a/LibLouis.NET.Test/tables/da-dk-6miscChars.cti b/LibLouis.NET.Test/nota-tables/da-dk-6miscChars.cti similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-6miscChars.cti rename to LibLouis.NET.Test/nota-tables/da-dk-6miscChars.cti diff --git a/LibLouis.NET.Test/tables/da-dk-6miscChars_1993.cti b/LibLouis.NET.Test/nota-tables/da-dk-6miscChars_1993.cti similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-6miscChars_1993.cti rename to LibLouis.NET.Test/nota-tables/da-dk-6miscChars_1993.cti diff --git a/LibLouis.NET.Test/tables/da-dk-8miscChars.cti b/LibLouis.NET.Test/nota-tables/da-dk-8miscChars.cti similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-8miscChars.cti rename to LibLouis.NET.Test/nota-tables/da-dk-8miscChars.cti diff --git a/LibLouis.NET.Test/tables/da-dk-8miscChars_1993.cti b/LibLouis.NET.Test/nota-tables/da-dk-8miscChars_1993.cti similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-8miscChars_1993.cti rename to LibLouis.NET.Test/nota-tables/da-dk-8miscChars_1993.cti diff --git a/LibLouis.NET.Test/tables/da-dk-braillo.dis b/LibLouis.NET.Test/nota-tables/da-dk-braillo.dis similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-braillo.dis rename to LibLouis.NET.Test/nota-tables/da-dk-braillo.dis diff --git a/LibLouis.NET.Test/tables/da-dk-g08.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g08.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g08.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g08.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g08_1993.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g08_1993.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g08_1993.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g08_1993.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g16-crossword.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g16-crossword.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g16-crossword.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g16-crossword.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g16-lit_1993.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g16-lit_1993.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g16-lit_1993.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g16-lit_1993.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g16-markers.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g16-markers.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g16-markers.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g16-markers.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g16.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g16.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g16.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g16.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g16_1993-markers.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g16_1993-markers.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g16_1993-markers.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g16_1993-markers.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g16_1993.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g16_1993.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g16_1993.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g16_1993.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g18.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g18.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g18.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g18.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g18_1993.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g18_1993.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g18_1993.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g18_1993.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g2.dic b/LibLouis.NET.Test/nota-tables/da-dk-g2.dic similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g2.dic rename to LibLouis.NET.Test/nota-tables/da-dk-g2.dic diff --git a/LibLouis.NET.Test/tables/da-dk-g26-lit_1993.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g26-lit_1993.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g26-lit_1993.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g26-lit_1993.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g26.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g26.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g26.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g26.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g26_1993.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g26_1993.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g26_1993.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g26_1993.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g26l-lit.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g26l-lit.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g26l-lit.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g26l-lit.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g26l-lit_1993.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g26l-lit_1993.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g26l-lit_1993.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g26l-lit_1993.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g26l.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g26l.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g26l.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g26l.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g26l_1993.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g26l_1993.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g26l_1993.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g26l_1993.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g28.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g28.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g28.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g28.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g28_1993.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g28_1993.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g28_1993.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g28_1993.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g28l.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g28l.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g28l.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g28l.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g28l_1993.ctb b/LibLouis.NET.Test/nota-tables/da-dk-g28l_1993.ctb similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g28l_1993.ctb rename to LibLouis.NET.Test/nota-tables/da-dk-g28l_1993.ctb diff --git a/LibLouis.NET.Test/tables/da-dk-g2_1993.dic b/LibLouis.NET.Test/nota-tables/da-dk-g2_1993.dic similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-g2_1993.dic rename to LibLouis.NET.Test/nota-tables/da-dk-g2_1993.dic diff --git a/LibLouis.NET.Test/tables/da-dk-octobraille.dis b/LibLouis.NET.Test/nota-tables/da-dk-octobraille.dis similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-octobraille.dis rename to LibLouis.NET.Test/nota-tables/da-dk-octobraille.dis diff --git a/LibLouis.NET.Test/tables/da-dk-octobraille_1993.dis b/LibLouis.NET.Test/nota-tables/da-dk-octobraille_1993.dis similarity index 100% rename from LibLouis.NET.Test/tables/da-dk-octobraille_1993.dis rename to LibLouis.NET.Test/nota-tables/da-dk-octobraille_1993.dis diff --git a/LibLouis.NET/LibLouis.cs b/LibLouis.NET/LibLouis.cs index 22bb72b..6d8e661 100644 --- a/LibLouis.NET/LibLouis.cs +++ b/LibLouis.NET/LibLouis.cs @@ -8,7 +8,7 @@ namespace LibLouis.NET; -public class LibLouis : IDisposable +public class LibLouis { /// /// LibLouis loglevels to ILogger logLevels table. @@ -27,7 +27,13 @@ public class LibLouis : IDisposable /// /// LibLouis is *NOT* thread safe, so we'll have to use a lock to avoid concurrrent access to native liblouis calls. /// - private readonly object _lock; + /// + /// Static, and shared with : the state it protects belongs to the native + /// library, not to this instance, so every native call in the assembly has to serialise on the + /// same object. Monitor is reentrant, so a logger that calls back in while liblouis is logging + /// does not deadlock. + /// + internal static readonly object NativeLock = new(); /// /// LibLouis can currently use either UCS-4 (1:1 mapping of UTF-32), or UCS-2 (WTF-16 without surrogate pairs), @@ -48,6 +54,17 @@ public class LibLouis : IDisposable private string _lastLogMessage = string.Empty; + /// + /// Roots the delegate behind the function pointer liblouis holds. + /// + /// + /// The interop stub only keeps the delegate alive for the duration of the registration call, + /// but liblouis keeps calling the pointer for the rest of the process's life. Without a + /// reference here the delegate is collected and the next native log message kills the process + /// with "A callback was made on a garbage collected delegate". + /// + private readonly NativeMethods.LoggingCallback _logCallback; + static LibLouis() { Instance = new LibLouis(); @@ -55,7 +72,8 @@ static LibLouis() private LibLouis() { - _lock = new object(); + // unlocked: the type initializer runs single threaded, and no other thread can hold a + // reference to the singleton until it has finished, so there is nothing to race with. CharacterSize = NativeMethods.lou_charSize(); LibLouisStringEncoder = CharacterSize switch { @@ -64,19 +82,27 @@ private LibLouis() _ => throw new NotImplementedException($"Liblouis is a character size of {CharacterSize}!?"), }; + _logCallback = LogCallback; + // Register managed log callback, so we can give reasonable exception messages. - NativeMethods.lou_registerLogCallback(LogCallback); + // unlocked: same reason - still inside the type initializer. + NativeMethods.lou_registerLogCallback(_logCallback); } - // https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/unmanaged - ~LibLouis() - { - // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method - Dispose(disposing: false); - } + // Deliberately no finalizer. lou_free tears down state that is global to the process, while a + // finalizer runs per managed instance: in a collectible AssemblyLoadContext it would free the + // tables of every other context still using liblouis, from the finalizer thread, outside the + // lock. Nothing here owns a handle that leaks if Shutdown is never called. private ILogger _logger = NullLogger.Instance; - private bool disposedValue; + + /// + /// Read by the guards without the lock, written by under it. + /// + /// + /// Static because what it tracks is the state of the native library, not of this instance. + /// + private static volatile bool _shutDown; /// /// ILogger instance LibLouis will log to. @@ -91,34 +117,64 @@ private void SetLogger(ILogger logger) { ArgumentNullException.ThrowIfNull(logger, nameof(logger)); - lock (_lock) + // Deliberately usable after disposal: neither call touches anything lou_free released, + // and being able to attach a logger while shutting down is worth more than the symmetry. + lock (NativeLock) { _logger = logger; NativeMethods.lou_setLogLevel(LogLevel.All); - NativeMethods.lou_registerLogCallback(LogCallback); + NativeMethods.lou_registerLogCallback(_logCallback); } } + /// + /// Called by liblouis, on a native stack. + /// + /// + /// Nothing may be thrown out of here. liblouis has no way to handle a managed exception, and + /// letting one unwind through its frames tears the process down. + /// private void LogCallback(LogLevel level, string message) { - Microsoft.Extensions.Logging.LogLevel l = LogLevels[level]; - _lastLogMessage = message; + try + { + _lastLogMessage = message; - if (_logger.IsEnabled(l)) + // liblouis is free to introduce log levels we have no mapping for. + if (!LogLevels.TryGetValue(level, out Microsoft.Extensions.Logging.LogLevel l)) + { + l = Microsoft.Extensions.Logging.LogLevel.Information; + } + + if (_logger.IsEnabled(l)) + { + // Passed as an argument, not as the template: liblouis messages contain table + // paths and rule text, and a stray brace would otherwise be parsed as a + // placeholder. + _logger.Log(l, "{LiblouisMessage}", message); + } + } + catch { - _logger.Log(l, message); + // A logger that throws must not become a native crash. } } /// /// Returns version number of the native liblouis library. /// + /// + /// Readable after disposal: lou_version returns a compile-time constant and touches nothing + /// lou_free released, and version information is worth having while diagnosing a shutdown. + /// public string Version { get { - string version = NativeMethods.lou_version(); - return version; + lock (NativeLock) + { + return NativeMethods.lou_version(); + } } } @@ -130,16 +186,18 @@ public string? DataPath { get { - lock (_lock) + lock (NativeLock) { + ThrowIfShutDown(); return NativeMethods.lou_getDataPath(); } } set { ArgumentException.ThrowIfNullOrWhiteSpace(value, nameof(value)); - lock (_lock) + lock (NativeLock) { + ThrowIfShutDown(); NativeMethods.lou_setDataPath(value); } } @@ -161,8 +219,9 @@ public string? DataPath { ArgumentException.ThrowIfNullOrWhiteSpace(query, nameof(query)); - lock (_lock) + lock (NativeLock) { + ThrowIfShutDown(); return NativeMethods.lou_findTable(query); } } @@ -173,9 +232,17 @@ public string? DataPath /// tables must be an IEnumerable of file names. public void IndexTables(IEnumerable tables) { - lock (_lock) + ArgumentNullException.ThrowIfNull(tables); + + // liblouis walks the array until it reads a null pointer, so it needs a terminator on top + // of the table names. Without it, it reads whatever managed memory follows the array and + // hands it to _lou_logMessage as a string. + string?[] nullTerminated = [.. tables, null]; + + lock (NativeLock) { - NativeMethods.lou_indexTables(tables.ToArray()); + ThrowIfShutDown(); + NativeMethods.lou_indexTables(nullTerminated); } } @@ -188,15 +255,18 @@ public string DotsToCharacters(IEnumerable tableList, string input) { ArgumentNullException.ThrowIfNull(input, nameof(input)); + int length = CountUCSCharacters(input); + byte[] inputBuffer = PrepareUCSInputBuffer(input); - byte[] outputBuffer = PrepareUCSOutputBuffer(input.Length); + byte[] outputBuffer = PrepareUCSOutputBuffer(length); string tables = string.Join(',', tableList); bool success; - lock (_lock) + lock (NativeLock) { - success = NativeMethods.lou_dotsToChar(tables, inputBuffer, outputBuffer, input.Length, TranslationMode.Regular) > 0; + ThrowIfShutDown(); + success = NativeMethods.lou_dotsToChar(tables, inputBuffer, outputBuffer, length, TranslationMode.Regular) > 0; } if (!success) @@ -204,7 +274,7 @@ public string DotsToCharacters(IEnumerable tableList, string input) throw new LibLouisException($"String translation failed: {_lastLogMessage}"); } - return ConvertUCSOutputBufferToString(outputBuffer, input.Length); + return ConvertUCSOutputBufferToString(outputBuffer, length); } /// @@ -216,16 +286,19 @@ public string CharactersToDots(IEnumerable tableList, string input) { ArgumentNullException.ThrowIfNull(input, nameof(input)); + int length = CountUCSCharacters(input); + byte[] inputBuffer = PrepareUCSInputBuffer(input); - byte[] outputBuffer = PrepareUCSOutputBuffer(input.Length); - + byte[] outputBuffer = PrepareUCSOutputBuffer(length); + string tables = string.Join(',', tableList); bool success; - lock (_lock) + lock (NativeLock) { - success = NativeMethods.lou_charToDots(tables, inputBuffer, outputBuffer, input.Length, TranslationMode.Regular) > 0; + ThrowIfShutDown(); + success = NativeMethods.lou_charToDots(tables, inputBuffer, outputBuffer, length, TranslationMode.Regular) > 0; } if (!success) @@ -233,8 +306,7 @@ public string CharactersToDots(IEnumerable tableList, string input) throw new LibLouisException($"String translation failed: {_lastLogMessage}"); } - return ConvertUCSOutputBufferToString(outputBuffer, input.Length); - + return ConvertUCSOutputBufferToString(outputBuffer, length); } /// @@ -285,18 +357,27 @@ public TranslatedString Translate( throw new ArgumentException($"{nameof(outputPosition)} parameter must point to an array of integers with at least input length elements.", nameof(outputPosition)); } - int inputLength = input.Length + 1; + // The number of widechars to translate, excluding the NUL terminator, which is what the + // header means by inlen and what upstream callers pass. The buffer stays terminated: the + // translate functions clamp at the first NUL, so an embedded NUL still ends the input. + int inputLength = CountUCSCharacters(input); int outputBufferLength = outputLength; byte[] inputBuffer = PrepareUCSInputBuffer(input); byte[] outputBuffer = PrepareUCSOutputBuffer(outputBufferLength); + TypeForm[]? typeFormBuffer = PrepareTypeFormBuffer(formtype, inputLength, outputBufferLength); + + // The cursor arrives as a .NET string index and liblouis wants a widechar index. + int[] inputOffsets = Utf16OffsetOfWidechar(input); + int widecharCursor = ToWidecharCursor(input, cursorPosition); string tables = string.Join(',', tableList); bool success; - lock (_lock) + lock (NativeLock) { - success = NativeMethods.lou_translate(tables, inputBuffer, ref inputLength, outputBuffer, ref outputLength, formtype, spacing, outputPosition, inputPosition, ref cursorPosition, mode) > 0; + ThrowIfShutDown(); + success = NativeMethods.lou_translate(tables, inputBuffer, ref inputLength, outputBuffer, ref outputLength, typeFormBuffer, spacing, outputPosition, inputPosition, ref widecharCursor, mode) > 0; } if (!success) @@ -304,12 +385,18 @@ public TranslatedString Translate( throw new LibLouisException($"String translation failed: {_lastLogMessage}"); } + string output = ConvertUCSOutputBufferToString(outputBuffer, outputLength); + + (int[] mappedOutputPosition, int[] mappedInputPosition, int mappedCursor) = + MapPositionsToUtf16(input, output, inputOffsets, outputPosition, inputPosition, widecharCursor); + return new TranslatedString { - Output = ConvertUCSOutputBufferToString(outputBuffer, outputLength), - CursorPosition = cursorPosition, - InputPosition = inputPosition, - OutputPosition = outputPosition, + Output = output, + CursorPosition = mappedCursor, + InputPosition = mappedInputPosition, + OutputPosition = mappedOutputPosition, + OutputDots78 = ExtractOutputDots78(typeFormBuffer, outputLength), }; } @@ -339,18 +426,23 @@ public string Translate(IEnumerable tableList, string input, int outputL throw new ArgumentException("Spacing must be the same length as input or null"); } - int inputLength = input.Length + 1; + // The number of widechars to translate, excluding the NUL terminator, which is what the + // header means by inlen and what upstream callers pass. The buffer stays terminated: the + // translate functions clamp at the first NUL, so an embedded NUL still ends the input. + int inputLength = CountUCSCharacters(input); int outputBufferLength = outputLength; byte[] inputBuffer = PrepareUCSInputBuffer(input); byte[] outputBuffer = PrepareUCSOutputBuffer(outputBufferLength); + TypeForm[]? typeFormBuffer = PrepareTypeFormBuffer(formtype, inputLength, outputBufferLength); string tables = string.Join(',', tableList); bool success; - lock (_lock) + lock (NativeLock) { - success = NativeMethods.lou_translateString(tables, inputBuffer, ref inputLength, outputBuffer, ref outputLength, formtype, spacing, mode) > 0; + ThrowIfShutDown(); + success = NativeMethods.lou_translateString(tables, inputBuffer, ref inputLength, outputBuffer, ref outputLength, typeFormBuffer, spacing, mode) > 0; } if (!success) @@ -410,18 +502,27 @@ public TranslatedString BackTranslate( throw new ArgumentException($"{nameof(outputPosition)} parameter must point to an array of integers with at least input length elements.", nameof(outputPosition)); } - int inputLength = input.Length + 1; + // The number of widechars to translate, excluding the NUL terminator, which is what the + // header means by inlen and what upstream callers pass. The buffer stays terminated: the + // translate functions clamp at the first NUL, so an embedded NUL still ends the input. + int inputLength = CountUCSCharacters(input); int outputBufferLength = outputLength; byte[] inputBuffer = PrepareUCSInputBuffer(input); byte[] outputBuffer = PrepareUCSOutputBuffer(outputBufferLength); + TypeForm[]? typeFormBuffer = PrepareTypeFormBuffer(formtype, inputLength, outputBufferLength); + + // The cursor arrives as a .NET string index and liblouis wants a widechar index. + int[] inputOffsets = Utf16OffsetOfWidechar(input); + int widecharCursor = ToWidecharCursor(input, cursorPosition); string tables = string.Join(',', tableList); bool success; - lock (_lock) + lock (NativeLock) { - success = NativeMethods.lou_backTranslate(tables, inputBuffer, ref inputLength, outputBuffer, ref outputLength, formtype, spacing, outputPosition, inputPosition, ref cursorPosition, mode) > 0; + ThrowIfShutDown(); + success = NativeMethods.lou_backTranslate(tables, inputBuffer, ref inputLength, outputBuffer, ref outputLength, typeFormBuffer, spacing, outputPosition, inputPosition, ref widecharCursor, mode) > 0; } if (!success) @@ -429,12 +530,17 @@ public TranslatedString BackTranslate( throw new LibLouisException($"String translation failed: {_lastLogMessage}"); } + string output = ConvertUCSOutputBufferToString(outputBuffer, outputLength); + + (int[] mappedOutputPosition, int[] mappedInputPosition, int mappedCursor) = + MapPositionsToUtf16(input, output, inputOffsets, outputPosition, inputPosition, widecharCursor); + return new TranslatedString { - Output = ConvertUCSOutputBufferToString(outputBuffer, outputLength), - CursorPosition = cursorPosition, - InputPosition = inputPosition, - OutputPosition = outputPosition, + Output = output, + CursorPosition = mappedCursor, + InputPosition = mappedInputPosition, + OutputPosition = mappedOutputPosition, }; } @@ -462,18 +568,23 @@ public string BackTranslate(IEnumerable tableList, string input, int out throw new ArgumentException("Spacing must be the same length as input or null"); } - int inputLength = input.Length + 1; + // The number of widechars to translate, excluding the NUL terminator, which is what the + // header means by inlen and what upstream callers pass. The buffer stays terminated: the + // translate functions clamp at the first NUL, so an embedded NUL still ends the input. + int inputLength = CountUCSCharacters(input); int outputBufferLength = outputLength; byte[] inputBuffer = PrepareUCSInputBuffer(input); byte[] outputBuffer = PrepareUCSOutputBuffer(outputBufferLength); + TypeForm[]? typeFormBuffer = PrepareTypeFormBuffer(formtype, inputLength, outputBufferLength); string tables = string.Join(',', tableList); bool success; - lock (_lock) + lock (NativeLock) { - success = NativeMethods.lou_backTranslateString(tables, inputBuffer, ref inputLength, outputBuffer, ref outputLength, formtype, spacing, mode) > 0; + ThrowIfShutDown(); + success = NativeMethods.lou_backTranslateString(tables, inputBuffer, ref inputLength, outputBuffer, ref outputLength, typeFormBuffer, spacing, mode) > 0; } if (!success) @@ -491,33 +602,260 @@ public string BackTranslate(IEnumerable tableList, string input, int out /// If it does not, the function does nothing. /// /// - /// + /// The word to hyphenate. Must be shorter than 100 characters. /// - /// + /// + /// One character per character of : '1' where the word may be broken, + /// '0' where it may not, '2' after an existing hyphen. On a UCS-4 build a non-BMP character + /// counts once, so the result can be shorter than . + /// /// public string Hyphenate(IEnumerable tableList, string input, TranslationMode mode) { ArgumentNullException.ThrowIfNull(tableList); - ArgumentNullException.ThrowIfNullOrEmpty(nameof(input)); + ArgumentException.ThrowIfNullOrEmpty(input); + + int length = CountUCSCharacters(input); + + // liblouis rejects anything from HYPHSTRING characters up, and would otherwise report it + // as an ordinary hyphenation failure. + if (length >= MaxHyphenationLength) + { + throw new ArgumentException( + $"{nameof(input)} must be shorter than {MaxHyphenationLength} characters.", nameof(input)); + } string tables = string.Join(',', tableList); - string hyphens = new('\0', input.Length + 1); + + // liblouis writes one flag per character plus a NUL terminator into a caller-allocated + // char buffer. inlen must not count the terminator: lou_hyphenate memcpy's exactly inlen + // widechars rather than stopping at a NUL the way the translate functions do, so an + // inlen in the wrong unit reads straight past the input buffer. + byte[] hyphens = new byte[length + 1]; byte[] inputBuffer = PrepareUCSInputBuffer(input); bool success; - - lock (_lock) + + lock (NativeLock) { - success = NativeMethods.lou_hyphenate(tables, inputBuffer, input.Length + 1, ref hyphens, mode) > 0; + ThrowIfShutDown(); + success = NativeMethods.lou_hyphenate(tables, inputBuffer, length, hyphens, mode) > 0; } - + if (!success) { throw new LibLouisException($"Hyphenation failed {_lastLogMessage}"); } - return hyphens; + // The flags are ASCII digits; the trailing terminator is not part of the result. + return Encoding.ASCII.GetString(hyphens, 0, length); + } + + /// + /// liblouis hyphenates into a fixed 100 character buffer (HYPHSTRING) and refuses any input + /// that would not fit. + /// + private const int MaxHyphenationLength = 100; + + /// + /// Copy the caller's typeform values into a buffer that is safe to hand to liblouis. + /// + /// + /// The typeform parameter is in/out: liblouis reads one entry per input character, but on a + /// successful translation it writes one entry per *output* cell. A translation that grows the + /// text - which the marker tables do routinely - would therefore write past the end of an + /// array sized to the input, corrupting the managed heap. We give liblouis a buffer big enough + /// for both directions and treat the caller's array as input only. + /// + private static TypeForm[]? PrepareTypeFormBuffer(TypeForm[]? formtype, int inputLength, int outputLength) + { + if (formtype is null) + { + return null; + } + + TypeForm[] buffer = new TypeForm[Math.Max(inputLength, outputLength) + 1]; + formtype.AsSpan(0, Math.Min(formtype.Length, buffer.Length)).CopyTo(buffer); + + return buffer; + } + + /// + /// Reads the per-cell dot 7/8 information liblouis wrote into the scratch typeform buffer. + /// + /// + /// The write-back half of : on a successful forward + /// translation liblouis stores the ASCII character '8' in the slot of every output cell that + /// contains dot 7 or dot 8, and '0' otherwise (lou_translateString.c:1330). Those are + /// characters smuggled through a formtype array, not TypeForm flag values, which is why this + /// converts to booleans instead of exposing the buffer. + /// + private static bool[]? ExtractOutputDots78(TypeForm[]? typeFormBuffer, int outputLength) + { + if (typeFormBuffer is null) + { + return null; + } + + bool[] dots = new bool[outputLength]; + + for (int k = 0; k < outputLength; k++) + { + dots[k] = typeFormBuffer[k] == (TypeForm)'8'; + } + + return dots; + } + + /// + /// Converts a cursor given as a .NET string index into the widechar index liblouis expects. + /// + /// + /// Negative means "no cursor" to liblouis and is passed through untouched. + /// + private int ToWidecharCursor(string input, int cursorPosition) + { + if (cursorPosition < 0 || input.Length == 0) + { + return cursorPosition; + } + + int[] widechars = WidecharOfUtf16Offset(input); + + return widechars[Math.Clamp(cursorPosition, 0, input.Length - 1)]; + } + + /// + /// Rewrites liblouis's widechar-indexed position arrays as UTF-16 indices into the managed + /// strings, so every value can be used directly as a string index. + /// + /// + /// liblouis counts in widechars: on a UCS-4 build one widechar is a whole Unicode character, + /// while a .NET string counts UTF-16 code units. The two agree for BMP text and diverge from + /// the first non-BMP character on, which silently misaligns any caller that treats these + /// values as string indices - and the arrays exist for nothing else. + /// + /// The results are sized to the strings they index rather than to the caller's scratch + /// buffers, so OutputPosition has one entry per char of the input and + /// InputPosition one per char of the output. No slicing is required to use them. + /// + /// Both halves of a surrogate pair report the same position, since they are one character. + /// + private (int[] OutputPosition, int[] InputPosition, int CursorPosition) MapPositionsToUtf16( + string input, + string output, + int[] inputOffsets, + int[] outputWidecharPositions, + int[] inputWidecharPositions, + int widecharCursor) + { + int[] outputOffsets = Utf16OffsetOfWidechar(output); + int[] inputWidechars = WidecharOfUtf16Offset(input); + int[] outputWidechars = WidecharOfUtf16Offset(output); + + int lastInputWidechar = Math.Max(inputOffsets.Length - 2, 0); + int lastOutputWidechar = Math.Max(outputOffsets.Length - 2, 0); + + int[] outputPosition = new int[input.Length]; + + for (int i = 0; i < input.Length; i++) + { + int widechar = inputWidechars[i]; + + int cell = widechar < outputWidecharPositions.Length ? outputWidecharPositions[widechar] : 0; + + outputPosition[i] = outputOffsets[Math.Clamp(cell, 0, lastOutputWidechar)]; + } + + int[] inputPosition = new int[output.Length]; + + for (int t = 0; t < output.Length; t++) + { + int widechar = outputWidechars[t]; + + int character = widechar < inputWidecharPositions.Length ? inputWidecharPositions[widechar] : 0; + + inputPosition[t] = inputOffsets[Math.Clamp(character, 0, lastInputWidechar)]; + } + + // A negative cursor means "no cursor" to liblouis; leave it alone. + int cursorPosition = widecharCursor < 0 || output.Length == 0 + ? widecharCursor + : outputOffsets[Math.Clamp(widecharCursor, 0, lastOutputWidechar)]; + + return (outputPosition, inputPosition, cursorPosition); + } + + /// + /// The UTF-16 offset at which each widechar of starts, with a + /// sentinel holding the string's length at the end. + /// + private int[] Utf16OffsetOfWidechar(string value) + { + int[] offsets = new int[CountUCSCharacters(value) + 1]; + + int widechar = 0; + + for (int i = 0; i < value.Length; widechar++) + { + offsets[widechar] = i; + i += IsSurrogatePairAt(value, i) ? 2 : 1; + } + + offsets[widechar] = value.Length; + + return offsets; + } + + /// + /// The widechar that each UTF-16 offset of belongs to. Both halves of + /// a surrogate pair map to the same widechar, because they are one character to liblouis. + /// + private int[] WidecharOfUtf16Offset(string value) + { + int[] widechars = new int[value.Length]; + + int widechar = 0; + + for (int i = 0; i < value.Length; widechar++) + { + int width = IsSurrogatePairAt(value, i) ? 2 : 1; + + for (int k = 0; k < width; k++) + { + widechars[i + k] = widechar; + } + + i += width; + } + + return widechars; + } + + /// + /// Whether a surrogate pair - one widechar, two chars - starts at . + /// Never true on a UCS-2 build, where a widechar is a UTF-16 code unit. + /// + private bool IsSurrogatePairAt(string value, int index) + { + return CharacterSize == 4 + && char.IsHighSurrogate(value[index]) + && index + 1 < value.Length + && char.IsLowSurrogate(value[index + 1]); + } + + /// + /// The number of liblouis widechars occupies. + /// + /// + /// Not the same as string.Length on a UCS-4 build: a non-BMP character is one widechar but + /// two chars. Lengths handed to liblouis have to be counted in widechars, or they describe a + /// longer buffer than the one that was allocated. + /// + private int CountUCSCharacters(string input) + { + return LibLouisStringEncoder.GetByteCount(input) / CharacterSize; } /// @@ -554,27 +892,59 @@ private string ConvertUCSOutputBufferToString(byte[] outputBuffer, int outputLen return LibLouisStringEncoder.GetString(outputBuffer, 0, Math.Min(outputLength * CharacterSize, outputBuffer.Length)); } - protected virtual void Dispose(bool disposing) + /// + /// Throws if liblouis has already been torn down. + /// + /// + /// Called from inside the lock, immediately before the native call. Checking on the way in + /// instead would leave a window for Shutdown to free the tables between check and call. + /// + /// Not ObjectDisposedException: this type is not disposable, and "cannot access a disposed + /// object" would send the reader looking for a Dispose call that does not exist. + /// + private static void ThrowIfShutDown() { - if (!disposedValue) + if (_shutDown) { - if (disposing) + throw new InvalidOperationException( + "liblouis has been shut down. LibLouis.Shutdown() frees state that is global to " + + "the process and cannot be undone."); + } + } + + /// + /// Frees everything liblouis has allocated. Final: there is no way back. + /// + /// + /// Deliberately a static method rather than IDisposable. lou_free walks and frees the + /// translation and display table chains, which are global to the process, so this is teardown + /// for the whole application rather than the release of a resource one caller owns. Exposing + /// it as IDisposable invited using (LibLouis.Instance), which reads as ordinary + /// cleanup and would leave every other consumer in the process unable to translate. + /// + /// Only worth calling when you need the tables released before the process exits - checking + /// for leaks, say. Normal applications should not call it at all: liblouis caches compiled + /// tables per table list rather than per call, so nothing accumulates, and process exit + /// reclaims it anyway. + /// + /// Takes the same lock as every other native call. Freeing those chains while another thread + /// is translating is a use-after-free, which shows up as anything from a nonsense + /// "no mapping for dot pattern" error to a crash. + /// + /// Calling it more than once does nothing. + /// + public static void Shutdown() + { + lock (NativeLock) + { + if (_shutDown) { - // Dispose managed state (managed objects) + return; } - // Free unmanaged resources (unmanaged objects) and override finalizer NativeMethods.lou_free(); - // Set large fields to null - disposedValue = true; + _shutDown = true; } } - - public void Dispose() - { - // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method - Dispose(disposing: true); - GC.SuppressFinalize(this); - } } diff --git a/LibLouis.NET/Logging.cs b/LibLouis.NET/Logging.cs index a4a077d..e3449e1 100644 --- a/LibLouis.NET/Logging.cs +++ b/LibLouis.NET/Logging.cs @@ -1,10 +1,29 @@ -namespace LibLouis.NET; +using System; +namespace LibLouis.NET; + +/// +/// These change the same global liblouis state that uses, so they take the +/// same lock. Setting the callback or the log level while another thread is inside a translation +/// is otherwise an unsynchronised write to state liblouis reads as it logs. +/// public static class Logging { + /// + /// Roots the delegate behind the function pointer liblouis holds. Callers routinely pass a + /// method group, which would otherwise be collected while liblouis still calls it. + /// + private static NativeMethods.LoggingCallback? _callback; + public static void SetCallback(NativeMethods.LoggingCallback value) { - NativeMethods.lou_registerLogCallback(value); + ArgumentNullException.ThrowIfNull(value); + + lock (LibLouis.NativeLock) + { + _callback = value; + NativeMethods.lou_registerLogCallback(_callback); + } } private static LogLevel _logLevel = LogLevel.Off; @@ -13,12 +32,18 @@ public static LogLevel LogLevel { get { - return _logLevel; + lock (LibLouis.NativeLock) + { + return _logLevel; + } } set { - _logLevel = value; - NativeMethods.lou_setLogLevel(value); + lock (LibLouis.NativeLock) + { + _logLevel = value; + NativeMethods.lou_setLogLevel(value); + } } } diff --git a/LibLouis.NET/NativeMethod.cs b/LibLouis.NET/NativeMethod.cs index 4eaf17f..95f55df 100644 --- a/LibLouis.NET/NativeMethod.cs +++ b/LibLouis.NET/NativeMethod.cs @@ -1,4 +1,5 @@ using System.Runtime.InteropServices; +using System.Runtime.InteropServices.Marshalling; namespace LibLouis.NET; @@ -12,7 +13,8 @@ public static partial class NativeMethods /// /// LibLouis version. [DefaultDllImportSearchPaths(DllImportSearchPath.SafeDirectories)] - [LibraryImport("liblouis", EntryPoint = "lou_version", StringMarshalling = StringMarshalling.Custom, StringMarshallingCustomType = typeof(UTF8StringNoFreeMarshaller))] + [LibraryImport("liblouis", EntryPoint = "lou_version")] + [return: MarshalUsing(typeof(UTF8StringNoFreeMarshaller))] internal static partial string lou_version(); /// @@ -130,13 +132,22 @@ internal static partial int lou_backTranslateString( /// /// Contains a hyphenation table. /// length of the character string in inbuf. - /// inlen is the length of the character string in inbuf - /// array of characters and must be of size inlen + 1 (to account for the NULL terminator). + /// + /// The number of characters in inbuf. Unlike the translate functions, lou_hyphenate does not + /// stop at a NUL: it copies exactly inlen characters, so this must not count the terminator. + /// It must also be less than 100 (HYPHSTRING), or liblouis refuses the call. + /// + /// + /// Caller-allocated output buffer of at least inlen + 1 bytes. liblouis writes one ASCII + /// '0' / '1' / '2' per character plus a NUL terminator. It is a plain char buffer, so it must + /// be marshalled as a byte array - a string would pass a pointer to a pointer and liblouis + /// would write over the marshalling stub's own stack. + /// /// /// 0 if error, 1 if success. [DefaultDllImportSearchPaths(DllImportSearchPath.SafeDirectories)] [LibraryImport("liblouis", EntryPoint = "lou_hyphenate", StringMarshalling = StringMarshalling.Utf8)] - internal static partial int lou_hyphenate(string tableList, byte[] inbuf, int inlen, ref string hyphens, TranslationMode mode); + internal static partial int lou_hyphenate(string tableList, byte[] inbuf, int inlen, byte[] hyphens, TranslationMode mode); /// /// This function enables you to compile a table entry on the fly at run-time. @@ -174,25 +185,48 @@ internal static partial int lou_backTranslateString( [LibraryImport("liblouis", EntryPoint = "lou_registerLogCallback")] internal static partial void lou_registerLogCallback(LoggingCallback callback); + /// + /// A pointer into static storage inside liblouis, or if the path was + /// never set. Must not be freed. + /// [DefaultDllImportSearchPaths(DllImportSearchPath.SafeDirectories)] - [LibraryImport("liblouis", EntryPoint = "lou_getDataPath", StringMarshalling = StringMarshalling.Utf8)] - internal static partial string lou_getDataPath(); + [LibraryImport("liblouis", EntryPoint = "lou_getDataPath")] + [return: MarshalUsing(typeof(UTF8StringNoFreeMarshaller))] + internal static partial string? lou_getDataPath(); + /// + /// A pointer into static storage inside liblouis, or if the path was + /// rejected. Must not be freed. + /// [DefaultDllImportSearchPaths(DllImportSearchPath.SafeDirectories)] [LibraryImport("liblouis", EntryPoint = "lou_setDataPath", StringMarshalling = StringMarshalling.Utf8)] - internal static partial string lou_setDataPath(string path); + [return: MarshalUsing(typeof(UTF8StringNoFreeMarshaller))] + internal static partial string? lou_setDataPath(string path); [DefaultDllImportSearchPaths(DllImportSearchPath.SafeDirectories)] [LibraryImport("liblouis", EntryPoint = "lou_checkTable", StringMarshalling = StringMarshalling.Utf8)] internal static partial int lou_checkTable(string tableList); + /// + /// Parses, analyzes and indexes the given tables. + /// + /// + /// Must be NULL terminated: liblouis walks the array until it reads a null pointer, so the + /// final element has to be . + /// [DefaultDllImportSearchPaths(DllImportSearchPath.SafeDirectories)] [LibraryImport("liblouis", EntryPoint = "lou_indexTables", StringMarshalling = StringMarshalling.Utf8)] - internal static partial void lou_indexTables(string[] tables); + internal static partial void lou_indexTables(string?[] tables); + /// + /// The best matching table name, or when there is no match. liblouis + /// documents this as the caller's to free, but the memory comes from liblouis's own C runtime + /// - see for why we leak it instead. + /// [DefaultDllImportSearchPaths(DllImportSearchPath.SafeDirectories)] [LibraryImport("liblouis", EntryPoint = "lou_findTable", StringMarshalling = StringMarshalling.Utf8)] - internal static partial string lou_findTable(string query); + [return: MarshalUsing(typeof(UTF8StringNoFreeMarshaller))] + internal static partial string? lou_findTable(string query); [DefaultDllImportSearchPaths(DllImportSearchPath.SafeDirectories)] [LibraryImport("liblouis", EntryPoint = "lou_compileString", StringMarshalling = StringMarshalling.Utf8)] diff --git a/LibLouis.NET/TranslatedString.cs b/LibLouis.NET/TranslatedString.cs index b043b02..4e82eab 100644 --- a/LibLouis.NET/TranslatedString.cs +++ b/LibLouis.NET/TranslatedString.cs @@ -3,10 +3,45 @@ public class TranslatedString { public required string Output { get; set; } - + + /// + /// For each char of the input, the index into it translated to. One entry + /// per char, so OutputPosition.Length equals the input's length and no slicing is + /// needed. Both halves of a surrogate pair report the same position. + /// + /// + /// A UTF-16 index, usable directly against the strings. liblouis reports these in widechars - + /// whole characters on a UCS-4 build - which agrees with UTF-16 only for BMP text; the + /// wrapper translates them. This is not the array passed in, which stays as liblouis wrote it. + /// public required int[] OutputPosition { get; set; } - + + /// + /// For each char of , the index into the input it came from. One entry per + /// char, so InputPosition.Length equals Output.Length. + /// + /// + /// A UTF-16 index, on the same terms as . Values always address + /// the start of a character, never the trailing half of a surrogate pair. + /// public required int[] InputPosition { get; set; } - + + /// + /// Where the cursor ended up, as an index into . Negative when the + /// translation was given no cursor. + /// public required int CursorPosition { get; set; } + + /// + /// Per output cell, whether liblouis reported the cell as containing dot 7 or dot 8. + /// when the translation ran without a formtype array, because liblouis + /// only computes this when one is supplied. + /// + /// + /// This is the write-back half of the native typeform parameter. liblouis writes it per + /// *output* cell, which is why it cannot go into the caller's input-sized formtype array - + /// that write is exactly the buffer overrun the wrapper exists to prevent. Forward + /// translation only: back-translation zero-fills the buffer and reports nothing. + /// + public bool[]? OutputDots78 { get; set; } } diff --git a/LibLouis.NET/UTF8StringNoFreeMarshaller.cs b/LibLouis.NET/UTF8StringNoFreeMarshaller.cs index 17fa844..0b5e470 100644 --- a/LibLouis.NET/UTF8StringNoFreeMarshaller.cs +++ b/LibLouis.NET/UTF8StringNoFreeMarshaller.cs @@ -1,49 +1,43 @@ -using System; using System.Runtime.InteropServices; using System.Runtime.InteropServices.Marshalling; -using System.Text; namespace LibLouis.NET; -[CustomMarshaller(typeof(string), MarshalMode.Default, typeof(UTF8StringNoFreeMarshaller))] -public unsafe static class UTF8StringNoFreeMarshaller +/// +/// Marshals a UTF-8 string that liblouis owns, without freeing it. +/// +/// +/// Several liblouis functions return a char * the caller must not release: lou_version, +/// lou_getDataPath and lou_setDataPath all hand back a pointer into static storage inside the +/// library. The default UTF-8 marshalling frees whatever the callee returned, which for those +/// pointers aborts the process ("pointer being freed was not allocated"). +/// +/// It is deliberately restricted to - return +/// values and out parameters. Not freeing is only correct for memory we did not allocate; +/// applying it to an input parameter would leak the buffer allocated for every call, so +/// parameters keep using the built-in . +/// +/// liblouis also has functions whose result the caller *is* expected to free (lou_findTable, +/// lou_findTables, lou_getTableInfo, lou_listTables). Those use this marshaller too: the Windows +/// binaries are built with mingw-w64 and allocate from msvcrt.dll while .NET frees through +/// ucrtbase.dll, so releasing that memory from managed code would corrupt the heap. Leaking a +/// bounded number of small strings is the safer trade. +/// +[CustomMarshaller(typeof(string), MarshalMode.ManagedToUnmanagedOut, typeof(UTF8StringNoFreeMarshaller))] +public static unsafe class UTF8StringNoFreeMarshaller { - public const byte NullTerminator = (byte)0; - - public static byte* ConvertToUnmanaged(string? managedString) - { - if (managedString is null) - { - return null; - } - - int unmanagedLength = Encoding.UTF8.GetByteCount(managedString) + 1; - byte* bufferPointer = (byte*)NativeMemory.Alloc((nuint)unmanagedLength); - Span byteSpan = new(bufferPointer, unmanagedLength); - - byteSpan = Encoding.UTF8.GetBytes(managedString); - byteSpan[^1] = NullTerminator; - - return bufferPointer; - } - - + /// + /// Copies the NUL terminated UTF-8 string at into a managed string. + /// public static string? ConvertToManaged(byte* unmanaged) { - if (unmanaged == null) - { - return null; - } - - Span stringSpan = new(unmanaged, int.MaxValue); - int length = stringSpan.IndexOf(NullTerminator); - - return Encoding.UTF8.GetString(unmanaged, length); + return Marshal.PtrToStringUTF8((nint)unmanaged); } - + /// + /// Deliberately does nothing: the string belongs to liblouis. + /// public static void Free(byte* unmanaged) { - // Do nothing, not caller's responsiblity to free it. } }