Repository navigation
Fix channel-name trim parity between frontend and server - #112
Merged
Merged
Conversation
…ow-up) normalizeName() in public/channel-proposals.js trims with JS's native String.prototype.trim(), which disagrees with Go's strings.TrimSpace (used by internal/channelregistry.NormalizeName) on two codepoints: U+FEFF (trimmed by JS, not by Go) and U+0085 NEL (trimmed by Go, not by JS). Add failing-first coverage for both mismatches, plus a Go test that pins the full unicode.IsSpace rune enumeration so a drift in either the JS mirror or the Go stdlib's White_Space table is caught. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji
normalizeName() called String(raw).trim() before the invisible-format
check, so JS's native trim rules decided what counted as leading/
trailing whitespace. That set differs from Go's unicode.IsSpace (what
strings.TrimSpace, and therefore internal/channelregistry.NormalizeName,
use):
- U+FEFF (BOM): JS's trim() strips it, Go's does not. A name typed as
"mychannel" showed and submitted as "#mychannel" in the UI,
then the server rejected it with ErrNameInvisible (400) — a
confusing rejection for a name the user never saw accepted.
- U+0085 (NEL): Go's TrimSpace strips it, JS's trim() does not. A
trailing NEL made the frontend reject the name as a control
character, while the server would have trimmed it and accepted
"#mychannel".
Replace the .trim() call with trimGoSpace(), which trims exactly the
codepoint set enumerated from Go's unicode.IsSpace (pinned in
GO_SPACE_CODEPOINTS and cross-checked against a matching Go test in
internal/channelregistry/name_test.go). The server's NormalizeName is
unchanged and remains authoritative; this only aligns the frontend's
pre-submit validation/preview with what the server will actually do.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji
- test-channel-proposals.js: fix a section heading and a comment that had literal ─/— escape text instead of the real ── and — characters used elsewhere in the file. - name_test.go (TestGoSpaceCodepointsParity) and the GO_SPACE_CODEPOINTS comment in channel-proposals.js: correct an inaccurate claim that a drift in the JS list would fail the Go test. The Go test only catches a change in Go's own unicode.IsSpace/White_Space table; the JS test only catches drift in its own copy. The two lists are kept in sync by hand. No logic changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji
Owner
Author
|
Review feedback addressed (commit
No logic changes. Re-ran both suites after the fix: Generated by Claude Code |
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.
Summary
Small follow-up to
dborup/CoreScope#99(shared channels): the frontend'schannel-name trimming in
normalizeName()(public/channel-proposals.js)did not match the server's trimming in
NormalizeName()(
internal/channelregistry/name.go).Low priority, no security impact — the server is authoritative and already
rejects everything it should. This only fixes the frontend's pre-submit
validation/preview so it agrees with what the server will actually decide.
The two mismatches
normalizeName()calledString(raw).trim()before the invisible-formatcheck. JS's native
trim()and Go'sstrings.TrimSpace(unicode.IsSpace)disagree on two codepoints:
U+FEFF (BOM) — JS's
trim()strips it, Go'sTrimSpacedoes not."<BOM>test"(a leading U+FEFF) → frontend trims the BOM away, shows/submits"#test"→ server'sNormalizeNamesees the BOM still there (notwhitespace to Go), rejects with
ErrNameInvisible(400). The user seesa rejection for a name that looked fine and accepted in the UI.
INVISIBLE_REcheck,and rejects up front with "The channel name contains invisible
formatting characters."
U+0085 (NEL) — Go's
TrimSpacestrips it (unicode.IsSpace(0x85) == true), JS'strim()does not."test\u0085"→ frontend does not trim the NEL, it fallsin
CONTROL_RE's range and gets rejected as "contains controlcharacters" → server's
NormalizeNametrims it away and would haveaccepted
"#test"."#test", matching theserver.
An input right after
#in the middle of a name (e.g.#mid<BOM>dle, amid-string U+FEFF) was already rejected before this change and still is —
only the leading/trailing codepoints handled by
trim()were affected.Fix
normalizeName()now calls a small localtrimGoSpace()helper instead of.trim(). It trims exactly the codepoint set Go'sunicode.IsSpacecovers,enumerated from the Go standard library (not guessed) and pinned as
GO_SPACE_CODEPOINTS:Parity proof
internal/channelregistry/name_test.goaddsTestGoSpaceCodepointsParity,which enumerates all runes
0..0x10FFFFwithunicode.IsSpaceand comparesthe result against the same fixed 25-codepoint list used in the JS test
(
GO_SPACE_CODEPOINTSintest-channel-proposals.js, also exported frompublic/channel-proposals.jsasCP.GO_SPACE_CODEPOINTS). A drift oneither side — a Go stdlib
White_Spacetable update, or an edit to onelist but not the other — fails a test.
TestNormalizeNameTrimsU0085LikeGoTrimSpace(Go) and the matching JStests lock in the U+0085 trim-and-accept behavior;
TestNormalizeNameRejectsLeadingTrailingFEFF(Go) and the matching JS tests lock in the U+FEFF reject-as-invisible
behavior.
No server/API changes —
internal/channelregistry/name.gois untouched.Tests run (counts from actual output, not estimated)
node test-channel-proposals.js→ 32 passed, 0 failed (28 pre-existingthe fix, 0 after)
cd internal/channelregistry && go test -count=1 -v ./...→ 16 passed,0 failed (13 pre-existing + 3 new)
cd cmd/server && go test -count=1 -v ./...→ 1976 passed, 0 failed(confirms the read-only server package is unaffected)
bash scripts/check-xss-sinks.sh --diff origin/master→ clean, no flaggedsinks
What changed
public/channel-proposals.js—trimGoSpace()helper +GO_SPACE_CODEPOINTS,used in
normalizeName()in place of.trim()test-channel-proposals.js— new coverage for both mismatches and theparity list
internal/channelregistry/name_test.go—TestGoSpaceCodepointsParityandmatching Go-side regression tests
No npm dependencies, no workflow changes, no new
map[string]interface{}.🤖 Generated with Claude Code
https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji
Generated by Claude Code