Repository navigation
host: an outside string that arrives ill-formed reaches visible text (peer name header, room-share notice) - #1938
Conversation
JSON carries an unpaired surrogate escape through as it came, so a peer's room notice, a roster entry's name and owner name, and a copied chat line reached the room and the guest's copy with a lone surrogate; the notice then went on to the bot-room standing prompt in that shape. parseRoomShare now makes those strings well formed after validation, one unit for one U+FFFD, so no length or code point bound moves. Docs-not-needed: docs/subsystems/room-share.md documents these fields as display data with their bounds; this change only makes a string that arrives broken well formed before it is passed on — no new field, bound, tool or path the page describes is added or changed.
|
Verdict: PASS at 1028aa9 Ran at this tip: Red first: reverting only the two changed sources to their Range: 9 files, +198/−7, exactly the two sites of #1937 plus their mirrors and one changelog fragment — Checks at this head, GitHub REST: 16 check-runs, every conclusion success except Checkpoint block on this body validates against |
3c131d7 to
1028aa9
Compare
Fixes #1937
Docs-not-needed:
docs/subsystems/core-host.mdanddocs/subsystems/room-share.mdalready describe a 60-unit name that lands in the fence from-attribute as whole characters and the envelope fields as display data with their bounds; this change only repairs ill-formed input before it is handed on — no new field, bound, constant, path or tool.Owner: Dev
Reviewer: Birfy
Written-by: Dev-2
Claim-Id: ef4507e7308a459da2dac2d64b7b6e5a
What this fixes
A peer name, and a shared room's own text, could arrive as a string that is not well formed: JSON and JavaScript both accept an unpaired surrogate, while UTF-8 cannot encode one, so a renderer shows U+FFFD and a byte round trip cannot reproduce the string. Two places handed such a string on exactly as it came.
core/src/host/delivery.jscleanName— the name reached the header a person reads ([Message from …]), thefrom="…"attribute of an untrusted-message fence, andsource.senderName, with a lone half in it.core/src/shared/room-share.jsparseRoomShare— a peer'snotice, a roster entry'snameandownerName, and a copied chat line were returned asJSON.parseproduced them; the notice then went on togroups.jsstandingTextand thebot-roomstanding prompt in that shape.cleanNamenow makes the name well formed before cleaning it, andparseRoomSharemakes those five string fields well formed after validation. Each repair is one UTF-16 unit in, one U+FFFD out; no length, bound, cut point, constant or path moves —cut(…, 60),NOTICE_MAXandHISTORY_TEXTare untouched, andask/inviteparse paths return as before.Evidence — two independent red-first cases, one per site
Site 1,
core/test/fence.test.js(before:# tests 9 / # pass 7 / # fail 2; after9 / 9)not ok 8 - a peer name that arrives with a lone surrogate is made well formed→not well formed: "Ann\ud83dX"not ok 9 - the line a broken peer name reaches stays well formed on every byte→not well formed: "<untrusted-message from=\"Ann\ud83dX\">"ok 7 - a peer name at the 60 bound keeps the whole character (#P282b)stays green; the assertions arecleanName('Ann\ud83dX') === 'Ann\uFFFDX',isWellFormed(),<= 60, no\p{Cc}, and a UTF-8 round trip of the fence's first line.Site 2,
core/test/remote-room.test.js—a room-share envelope hands on well formed notice, roster and history text (#1937)(before:# tests 82 / # pass 81 / # fail 1; after82 / 82)not ok 34→notice is not well formed: "aaaaaaaaaaaaaaaaaaa\ud83d tail"encodeRoomShare→parseRoomShareand pins five fields separately (notice,roster[0].name,roster[0].ownerName,history[0].who,history[0].text): each well formed, each equal to what was sent with U+FFFD for the unpaired half and nothing else changed, each with the length it arrived with. It also pins that the wire text is well formed already (JSON escapes the lone surrogate, so the break happens inJSON.parse), that a secondparse(encode(parsed))changes nothing, and the shortest shapenotice: 'Ann\ud83dX'→'Ann\uFFFDX'.Mutations, each restored and re-run green:
return wellFormedShare(data)→return data: site 2 red (# pass 81 / # fail 1).noticeonly, roster/history lines removed: red onroster[0].name is not well formed: "Ann\ud83dX"— the five assertions are independent, not carried by the notice.cutback to.slice: only the corresponding site 1 case goes red; neither site's fix silences the other.What I ran (at tip
1028aa9a50a78ef7c6a13beb6bb569da51e5d24f, tree7b028e95e6bfa155d5badceeba08b963640d9dcc, base4d851e83)node scripts/ds-bot-patches.mjs generatethree times — thepatches/ds-botcontent hash is identical before and after, so it is a no-op, andgit status --porcelainis empty afterwardscmp patches/ds-bot/files/src/shared/room-share.js core/src/shared/room-share.jsand the same fortest/remote-room.test.js— byte-identicalnode scripts/ds-bot-patches.mjs check→ 3 ok;node scripts/ds-bot-patches.mjs canonical→ okTZ=UTC ARCH_DOCS_BASE=4d851e832084bdbf1fb8f8a2b1b78fce49bc4056 npm run verify→ exit 0:arch-docs: OK (9 changed file(s)), core 2342 tests / 0 fail, social 541, tree 127, timeline 135, workflow 336,.github563, interop 37, live-e2e 24, core lint 73 warnings / 0 errors (the base's count)node .github/scripts/changelog.mjs check→23 fragment(s) in changelog.d/ are well formedNot run: the bench smoke test (no
index.jsor turn-path change;delivery.jsis insrc/host/, so the smoke bench would need the desktop checkout), and the pack check, typecheck and actionlint that CI'spackagejob runs.Docs-not-needed
docs/subsystems/core-host.mdalready records that a 60-unit name goes through the boundary rule and lands infenceUntrusted'sfromattribute as whole characters, anddocs/subsystems/room-share.mdalready documents the envelope's fields as display data with their bounds. This change only makes the input half of those sentences hold — a string that arrives broken is repaired before it is handed on — with no new field, bound, constant, tool or path, so neither page describes anything different afterwards. No docs change is needed.Checkpoint
{ "pr": 1938, "claim": "Dev", "issue": 1937, "summary": "A peer name, and a shared room's notice, roster names and copied chat lines, could arrive as strings that are not well formed, because JSON accepts an unpaired surrogate escape. cleanName now makes the name well formed before cleaning it, and parseRoomShare makes those five string fields well formed after validation, one unit for one U+FFFD. No bound, cut point, constant or path moves. Two independent red-first cases pin the two sites.", "files": ["core/src/host/delivery.js", "core/src/shared/room-share.js", "core/test/fence.test.js", "core/test/remote-room.test.js", "patches/ds-bot/0005-social-seams.patch", "patches/ds-bot/files/src/shared/room-share.js", "patches/ds-bot/files/test/fence.test.js", "patches/ds-bot/files/test/remote-room.test.js", "changelog.d/1937-peer-name-well-formed.fixed.md"], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=4d851e83 npm run verify", "result": "green", "at": "1028aa9a50a78ef7c6a13beb6bb569da51e5d24f" }, "next": "none — ready for review" }