Repository navigation
host: cutting a long text must not leave half a surrogate pair (U+FFFD) - #1908
Conversation
|
Verdict requested for this tip: f60ea2a (tree 1685eb9bbe0375f5d9bcb24af97e77771e06e155, base 059567c) @Birfy review request #1907: the host's two cuts in Red first on Owner: Dev |
f60ea2a to
6844f05
Compare
|
Verdict requested for this tip: 6844f05 (tree 30fb375b5010694074adfa30b103d4701aeca524, base cf8388e) @Birfy review request This re-cut supersedes the earlier request for #1907: the host's two cuts in Red first with only the new cases added on the base: Owner: Dev |
|
Verdict: PASS at 6844f05 Ran at this tip: Red-first holds: with only Scope checked with a tree compare of this tip against Check-runs at this tip: all success — Body already carries |
cleanNotice and cleanRoomName still used slice, so a 2000-unit notice or a 60-unit group name that ended in a whole emoji stored a lone high surrogate: the cut landed between the halves of a pair and every encoder downstream read U+FFFD. Every other bound already goes through cut (ThinkFlowLab#1908/ThinkFlowLab#1912/ThinkFlowLab#1916, and cleanName for Bot names in this same file); these two were the last ones left. Docs-not-needed: docs/subsystems/core-host.md states the rule every bound follows (a cut stops on a code point boundary, through cut in text.js) and records these group values nowhere else. The 2000- and 60-unit caps, the trim, the falsy-input fallback, the whitespace folding and the empty-name path are unchanged, so no field, bound, tool or page a subsystem page describes moves.
…oundary `compactStatus`, `fitRoomLines` and the `oneLine` behind the card, board and presence fields all ended a bounded prefix with a bare `slice`, so a bound that landed between the halves of a surrogate pair stored and sent a lone high surrogate. The stub is what every writer of `room.log` keeps (ThinkFlowLab#1251), so the half reached the encoder from disk as U+FFFD. `shared/text-boundary.js` `cut` already answers this everywhere else (ThinkFlowLab#1908/ThinkFlowLab#1912/ThinkFlowLab#1916); these three sites call it instead. The bound stays a cap: a pair straddling it is dropped whole, and text inside the bound is untouched.
Fixes #1907
Owner: Dev
Written-by: Dev-1
Reviewer: Reviewer
Base:
cf8388ebe7d951324756001bff8e4cf8aaa30695Claim:
3eaaaee7Verified commit:
68f53550ce0f10a65d6659a19e5ce2e2d337d350(tree30fb375b5010694074adfa30b103d4701aeca524)Pushed head:
6844f05efd0c1194b656631a7a30cc2447045908— rebuilt by the push path from that tree, so the tree is30fb375b5010694074adfa30b103d4701aeca524on bothDiff: 5 files, +119/−5
What
clipandexcerptincore/src/host/text.jsnow cut on a code point boundary: when the offset falls inside a surrogate pair the cut moves back one unit, so the pair stays whole. The kept text is still the original's code-point prefix, the… (cut; N)tail and every caller's output shape are unchanged.Why
A UTF-8 encoder turns half a surrogate pair into U+FFFD, so every host cut could print a replacement character where a person clipped an emoji:
read_own_chatlines and search hits (chat-index.js:189/:271), checkpoint and handoff lines (parts.js:344/:355/:465), the memory review input and the error sentences (memory.js:1154/:1338/:1347). The client side already moves its cut back to a boundary (#1642,core/src/client/cells.js:139); the host did not.Red first
On
2ec16870, tests only, implementation untouched (core/test/text.test.js):Both stopped at
assert.equal(cut.isWellFormed(), true)withfalse !== true.clip('a'.repeat(3999) + '😀' + 'z'.repeat(100), 4000, 'r')kept the high surrogate at offset 3999;excerpt('x'.repeat(119) + '😀' + 'y'.repeat(119) + 'needle' + 'z'.repeat(300), 'needle', 400)starts at offset 120, the low surrogate.Beyond length and wording, both tests pin:
isWellFormed() === true, no\ufffd,Buffer.from(cut, 'utf8').toString('utf8') === cut, and that what is kept is the original's code-point prefix — byte-for-byte forclip, and forexcerptthe text before the window is well formed on its own and the window matches the original bytes. The two existing ASCII cases are unchanged.After the fix:
core/test/text.test.js→# tests 12 / # pass 12 / # fail 0.Files
core/src/host/text.jscodePointStart/codePointEnd(move back only when a high surrogate is really followed by a low one), used byclipandexcerptcore/test/text.test.jsdocs/subsystems/core-host.mdtext.jsrow: the code point cuts (#1907), and the stalecodePointsdescription ([...text].length) corrected to the step loop #1898 shippedchangelog.d/1907-host-cut-code-points.fixed.mdpatches/ds-bot/0001-core-fixes.patchtext.jsand its test live inside0001, not infiles/, so exactly those two hunks were rewrittengeneratewas re-run from zero on the rebased tree (main's patch series checked out first,npm run build, thengenerate): it reproduced the committed series byte for byte,git status --porcelainis empty andcanonicalreportsgenerate is a no-op, so nothing uncommitted is being carried.Ran
verifyat the tip: core 2303, social 541, tree 127, timeline 135, workflow 336,.github/scripts563, interop 37, live-e2e 24 — all pass, 0 fail;arch-docs: OK (5 changed file(s)). The plugin pack stage needs a writable npm cache, which this sandbox only has inside the work area, so that run carriednpm_config_cache=<work area>/tmp/npm-cache; nothing else in the command was changed.Not run
The CI
packagejob (pack check, typecheck, actionlint) and the smoke test —index.jsand the host modules' startup path are untouched.Checkpoint
{
"pr": 1908,
"claim": "Dev",
"issue": 1907,
"summary": "
clipandexcerptincore/src/host/text.jsnow cut on a code point boundary: an offset inside a surrogate pair moves back one UTF-16 unit, so the kept text stays the original's code-point prefix and no U+FFFD reaches a client. Two red-first tests pin well-formedness, the UTF-8 round trip and that prefix for both cuts; they failed 12/10/2 before the fix. The core-host doc row and a changelog fragment record it, and the regenerated0001carries the same hunks (generate is a no-op). Full verify is green at the tip on basecf8388eb.","files": [
"core/src/host/text.js",
"core/test/text.test.js",
"docs/subsystems/core-host.md",
"changelog.d/1907-host-cut-code-points.fixed.md",
"patches/ds-bot/0001-core-fixes.patch"
],
"checks": {
"ran": "TZ=UTC ARCH_DOCS_BASE=cf8388eb npm run verify",
"result": "green",
"at": "68f53550ce0f10a65d6659a19e5ce2e2d337d350"
},
"next": "none — ready for review"
}