fix(domain): make SanitizeString idempotent (#745)
Docker / Build and Push (push) Canceled after 0s
github-pages / deploy (push) Canceled after 0s
Test / make test (push) Canceled after 0s
Docker / release (push) Canceled after 0s

NFC normalisation ran before control and format characters were stripped.
Removing a character can leave a base letter next to a combining mark the
earlier pass never saw as a pair, so a second call composes it:

    input    U+0041 U+0009 U+0300   ("A", tab, combining grave)
    once  -> U+0041 U+0300
    twice -> U+00C0

Found by TestPropertySanitizeStringIdempotent. Category Cf characters
behave the same way.

Strip first, then normalise. Normalisation still precedes truncation
because composing changes the rune count. One side effect: invalid UTF-8
is now dropped by the strip loop instead of surviving as U+FFFD, so an
identifier containing such bytes sanitises differently than before.

Signed-off-by: clark-ja <37738506+clark-ja@users.noreply.github.com>
This commit is contained in:
Jacopo Clark
2026-09-01 22:30:37 +02:00
committed by GitHub
parent 3690ff3baa
commit a0cb7d9d39
2 changed files with 52 additions and 4 deletions
+16 -4
View File
@@ -47,16 +47,25 @@ var reservedUserIdentifiers = map[string]struct{}{
CtxSystemDBMigrator: {}, CtxSystemDBMigrator: {},
} }
// SanitizeString normalizes to NFC, trims leading and trailing whitespace, strips Unicode // SanitizeString trims leading and trailing whitespace, strips Unicode control and format
// control and format characters, drops invalid UTF-8 bytes, and truncates the result to // characters, drops invalid UTF-8 bytes, normalizes to NFC, and truncates the result to
// maxLen runes. If maxLen <= 0, returns "". // 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 { func SanitizeString(s string, maxLen int) string {
if maxLen <= 0 { if maxLen <= 0 {
return "" 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 var b strings.Builder
b.Grow(len(s)) b.Grow(len(s))
for len(s) > 0 { for len(s) > 0 {
@@ -69,7 +78,10 @@ func SanitizeString(s string, maxLen int) string {
b.WriteRune(r) 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 { if utf8.RuneCountInString(s) > maxLen {
runes := []rune(s) runes := []rune(s)
+36
View File
@@ -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)
}
}