diff --git a/internal/domain/sanitize.go b/internal/domain/sanitize.go index 9999a60..23ee874 100644 --- a/internal/domain/sanitize.go +++ b/internal/domain/sanitize.go @@ -47,16 +47,25 @@ var reservedUserIdentifiers = map[string]struct{}{ CtxSystemDBMigrator: {}, } -// SanitizeString normalizes to NFC, trims leading and trailing whitespace, strips Unicode -// control and format characters, drops invalid UTF-8 bytes, and truncates the result to +// SanitizeString trims leading and trailing whitespace, strips Unicode control and format +// characters, drops invalid UTF-8 bytes, normalizes to NFC, and truncates the result to // maxLen runes. If maxLen <= 0, returns "". +// +// The order matters and is load-bearing for idempotency: see the comments in the body. +// SanitizeString(SanitizeString(s, n), n) == SanitizeString(s, n) for all s and n. func SanitizeString(s string, maxLen int) string { if maxLen <= 0 { return "" } - s = norm.NFC.String(strings.TrimSpace(s)) + s = strings.TrimSpace(s) + // Strip control/format characters and invalid UTF-8 *before* normalizing. + // Normalizing first is not idempotent: removing a character can leave a + // base letter next to a combining mark that the earlier normalization never + // saw as a pair. For example "A\t̀" is already NFC (the tab keeps the + // letter and the combining grave apart), but stripping the tab yields + // "À", which a second call would compose to "À". var b strings.Builder b.Grow(len(s)) for len(s) > 0 { @@ -69,7 +78,10 @@ func SanitizeString(s string, maxLen int) string { b.WriteRune(r) } } - s = b.String() + + // Normalize before truncating, not after: composition can change the rune + // count, so normalizing afterwards could push the result back over maxLen. + s = norm.NFC.String(b.String()) if utf8.RuneCountInString(s) > maxLen { runes := []rune(s) diff --git a/internal/domain/sanitize_test.go b/internal/domain/sanitize_test.go index d3e5b6a..dfd0418 100644 --- a/internal/domain/sanitize_test.go +++ b/internal/domain/sanitize_test.go @@ -501,3 +501,39 @@ func TestPropertySanitizeIdentifierRejectsReservedValues(t *testing.T) { } }) } + +// --------------------------------------------------------------------------- +// Regression: normalization must run after control characters are stripped +// --------------------------------------------------------------------------- + +// Found by TestPropertySanitizeStringIdempotent. Normalizing before stripping +// control characters is not idempotent: "A\t̀" is already NFC because the +// tab separates the letter from the combining grave, but removing the tab +// leaves "À", which a second call composes to "À". +func TestSanitizeStringNormalizesAfterStripping(t *testing.T) { + const input = "A\t\u0300" // "A", tab, combining grave accent + + got := SanitizeString(input, 2) + + if got != "À" { + t.Errorf("expected the combining mark to be composed after the tab is stripped, got %q (% x)", + got, []rune(got)) + } + if again := SanitizeString(got, 2); again != got { + t.Errorf("not idempotent: once=%q twice=%q", got, again) + } +} + +// The same hazard exists for format characters (category Cf), not just controls. +func TestSanitizeStringStripsFormatCharsBeforeNormalizing(t *testing.T) { + const input = "A\u00ad\u0300" // "A", soft hyphen (Cf), combining grave accent + + got := SanitizeString(input, 2) + + if again := SanitizeString(got, 2); again != got { + t.Errorf("not idempotent: once=%q twice=%q", got, again) + } + if got != "À" { + t.Errorf("expected %q, got %q", "À", got) + } +}