Repository navigation
Fix lone surrogates in run text throwing during normalization and save - #414
Open
johnml1135 wants to merge 7 commits into
Open
johnml1135 wants to merge 7 commits into
johnml1135 wants to merge 7 commits into
Conversation
A TsString containing an unpaired UTF-16 surrogate threw from get_NormalizedForm (EnumerateSegmentLimits -> char.ConvertToUtf32), so it could not be saved. TsString normalization now passes a lone surrogate through unchanged (as ICU does), and TsStringSerializer drops lone surrogates when writing run text so the XML reloads. CharAt, IsCharWordForming and WordMaker.IsWhite no longer throw either. Adds Surrogates.CodePointAt, a ConvertToUtf32 that tolerates lone surrogates. Fixes sillsdev#412 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… on save Review of sillsdev#414: stripping a lone surrogate on write let stored text differ from in-memory text, so segment offsets and analyses were wrong after a save and reload. On master a lone surrogate could not enter the model at all (every TsString setter threw), so: - TsStringUtils.NormalizeNfd(ITsString), which every model setter goes through, now replaces each lone surrogate with U+FFFD (same length). - TsStringSerializer replaces with U+FFFD instead of deleting, as a backstop for TsStrings that never went through the model, and decides xml:space from the text it writes. - IsCharWordForming steps back over a low surrogate only when a high surrogate precedes it. - Restore the BOM in TsString.cs; clarify Surrogates.CodePointAt docs; use \u escapes in tests; add parser, fuzz and save/reload tests. Fixes sillsdev#412 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
.NET Framework string.Normalize rejects unassigned code points such as the D800+DFFF pair, so the net462 run of the fuzz failed in the test itself. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…r custom String fields - GetXmlSafeText replaces lone surrogates first, so it keeps the length whether or not StripInvalidXmlChars also removes lone surrogates (as sillsdev#410's regex does), and removing a control character cannot join a lone high and low surrogate into a false pair. Test pins the order. - CmObject.SetCustomPropertyForSDA runs NormalizeNfd on custom String fields, like model String properties. Test added. - Tests use \u escapes for every non-ASCII character. - CHANGELOG: say that callers relying on the exception now get U+FFFD. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The generated setter for plain Unicode properties (propaccessors_simple template) and WfiWordformRepository.TryGetObject called string.Normalize, which throws on an unpaired surrogate. Replace it with U+FFFD first, as the TsString paths do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… expectations A custom Unicode field set to a lone surrogate stored it raw, and the next Save() failed the whole project save. SetCustomPropertyForSDA now handles CellarPropertyType.Unicode like the generated setter (replace with U+FFFD, then NFD). Test first: file-backed cache, custom Unicode field, Save. Also write expected U+FFFD values in tests as \u escapes, and update the CHANGELOG wording. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #412
On master a lone (unpaired) surrogate cannot enter the model: every TsString setter throws in
TsStringUtils.NormalizeNfd. #412's repro reaches the serializer only for strings that bypass the model.Design
NormalizeNfd(ITsString), the choke point for the generated setters, custom String fields,MultiAccessor.set_String,DomainDataByFlid.SetStringandSetMultiStringAlt, replaces each lone surrogate with U+FFFD, keeping length and run properties. The generated and custom plainUnicodesetters andWfiWordformRepository.TryGetObjectdo the same. Stored text equals in-memory text, so segment offsets and analyses survive save and reload.TsStringpasses a lone surrogate through, as ICU does.IsCharWordFormingsteps back only over a real pair.Behaviour change: text that used to throw now stores U+FFFD silently, including for direct callers of
NormalizeNfd(ITsString)and plainUnicodesetters such asLexEntry.LiftResidue.Known gap: search-query text in
StringSearcherandFindWordFormInStringon raw non-model strings can still throw on a lone surrogate; neither stores data.Merges
<Field name>are stripped (Make text property XML reader and writer agree #410, head 935e5a1).TsStringSerializer.cs. UsewrittenText = GetXmlSafeText(runText); preserveSpace = writtenText != string.Empty && writtenText.All(char.IsWhiteSpace);.GetXmlSafeTextreplaces first, so it stays correct. Make text property XML reader and writer agree #410'sGetXmlSafeStringmust strip unpaired surrogates itself.Tests: setters, plain Unicode and custom Unicode setters, wordform lookup, parser, fuzz, serializer order, and parse, save, reload of segment offsets. Green on net462 and net8.0.
🤖 Generated with Claude Code
This change is