diff --git a/src/static/js/collab_client.ts b/src/static/js/collab_client.ts index c90f92e80d3..bc5a533bb0b 100644 --- a/src/static/js/collab_client.ts +++ b/src/static/js/collab_client.ts @@ -26,6 +26,7 @@ const chat = require('./chat').chat; const hooks = require('./pluginfw/hooks'); const browser = require('./vendors/browser'); +import {stampAuthorOnInserts} from './stampAuthorOnInserts'; // Dependency fill on init. This exists for `pad.socket` only. // TODO: bind directly to the socket. @@ -48,6 +49,23 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad) let commitDelay = 500; const userId = initialUserInfo.userId; + + // Build the outgoing changeset and guarantee every insert carries an author. + // collab_client only exists after CLIENT_VARS has arrived, so `userId` is always + // populated here — whereas the editor can build a changeset with an empty author + // if the user typed before CLIENT_VARS landed (the early-typing race that the + // server's pad-corruption guard rejects). Stamping here is the last line of + // defense before the changeset goes on the wire. See stampAuthorOnInserts. + const prepareUserChangeset = () => { + const data = editor.prepareUserChangeset(); + if (data.changeset) { + const stamped = stampAuthorOnInserts(data.changeset, data.apool, userId); + data.changeset = stamped.changeset; + data.apool = stamped.apool; + } + return data; + }; + // var socket; const userSet = {}; // userId -> userInfo userSet[userId] = initialUserInfo; @@ -114,7 +132,7 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad) // Check if there are any pending revisions to be received from server. // Allow only if there are no pending revisions to be received from server if (!isPendingRevision) { - const userChangesData = editor.prepareUserChangeset(); + const userChangesData = prepareUserChangeset(); if (userChangesData.changeset) { lastCommitTime = now; committing = true; @@ -416,7 +434,7 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad) obj.committedChangesetAPool = stateMessage.apool; editor.applyPreparedChangesetToBase(); } - const userChangesData = editor.prepareUserChangeset(); + const userChangesData = prepareUserChangeset(); if (userChangesData.changeset) { obj.furtherChangeset = userChangesData.changeset; obj.furtherChangesetAPool = userChangesData.apool; diff --git a/src/static/js/stampAuthorOnInserts.ts b/src/static/js/stampAuthorOnInserts.ts new file mode 100644 index 00000000000..a8f8afbf776 --- /dev/null +++ b/src/static/js/stampAuthorOnInserts.ts @@ -0,0 +1,58 @@ +'use strict'; + +import {deserializeOps, pack, unpack} from './Changeset'; +import AttributeMap from './AttributeMap'; +import AttributePool from './AttributePool'; +import {SmartOpAssembler} from './SmartOpAssembler'; + +/** + * Ensure every insert (`+`) op in a wire changeset carries an `author` attribute. + * + * Why this exists: the local author id (`clientVars.userId`) only becomes available + * once the CLIENT_VARS socket message arrives. Under load (notably Firefox with + * plugins) the editor can become editable and the user can type *before* that + * message lands, so the editor tags the insert with an empty author. An empty + * author canonicalizes to "no author", producing an unattributed insert that the + * server's pad-corruption guard rejects — dropping the whole USER_CHANGES and + * silently losing the typed text's authorship (the clear_authorship_color flake). + * + * This runs in collab_client, which only exists after CLIENT_VARS has arrived, so + * the author id passed here is always populated. Stamping any author-less insert + * just before the changeset is sent guarantees the server never sees an + * unattributed insert, independent of editor-init timing. + * + * @param changeset - The wire changeset string from prepareUserChangeset(). + * @param apoolJsonable - The jsonable wire attribute pool that accompanies it. + * @param authorId - The local author id (collab_client's userId). If falsy, the + * inputs are returned unchanged (nothing better to stamp with). + * @returns The (possibly) rewritten changeset + jsonable pool. Returns the inputs + * unchanged when no insert needed an author, so the common path is a no-op. + */ +export const stampAuthorOnInserts = ( + changeset: string, + apoolJsonable: any, + authorId: string, +): {changeset: string, apool: any} => { + if (!authorId) return {changeset, apool: apoolJsonable}; + const pool = (new AttributePool()).fromJsonable(apoolJsonable); + const unpacked = unpack(changeset); + const assem = new SmartOpAssembler(); + let modified = false; + for (const op of deserializeOps(unpacked.ops)) { + if (op.opcode === '+') { + const attribs = AttributeMap.fromString(op.attribs, pool); + if (!attribs.get('author')) { + attribs.set('author', authorId); + op.attribs = attribs.toString(); + modified = true; + } + } + assem.append(op); + } + if (!modified) return {changeset, apool: apoolJsonable}; + assem.endDocument(); + return { + changeset: pack(unpacked.oldLen, unpacked.newLen, assem.toString(), unpacked.charBank), + apool: pool.toJsonable(), + }; +}; diff --git a/src/tests/backend-new/specs/stampAuthorOnInserts.test.ts b/src/tests/backend-new/specs/stampAuthorOnInserts.test.ts new file mode 100644 index 00000000000..449814a2adc --- /dev/null +++ b/src/tests/backend-new/specs/stampAuthorOnInserts.test.ts @@ -0,0 +1,76 @@ +'use strict'; + +import {describe, it, expect} from 'vitest'; +import {stampAuthorOnInserts} from '../../../static/js/stampAuthorOnInserts'; +import {checkRep, deserializeOps, unpack} from '../../../static/js/Changeset'; +import AttributeMap from '../../../static/js/AttributeMap'; +import AttributePool from '../../../static/js/AttributePool'; + +const AUTHOR = 'a.test1234567890'; +const EMPTY_POOL = () => (new AttributePool()).toJsonable(); + +// Read the author attribute off the first '+' op of a (changeset, jsonable pool). +const firstInsertAuthor = (changeset: string, apoolJsonable: any): string | undefined => { + const pool = (new AttributePool()).fromJsonable(apoolJsonable); + for (const op of deserializeOps(unpack(changeset).ops)) { + if (op.opcode === '+') return AttributeMap.fromString(op.attribs, pool).get('author'); + } + return undefined; +}; + +describe('stampAuthorOnInserts', () => { + it('stamps the author onto an unattributed insert (the flake changeset)', () => { + // `Z:1>5+5$Hello` — insert "Hello" with NO author, exactly what the editor + // emits during the early-typing race and what the server rejects. + const input = 'Z:1>5+5$Hello'; + const {changeset, apool} = stampAuthorOnInserts(input, EMPTY_POOL(), AUTHOR); + // The insert now carries the author... + expect(firstInsertAuthor(changeset, apool)).toBe(AUTHOR); + // ...the result is a valid canonical changeset... + expect(() => checkRep(changeset)).not.toThrow(); + // ...and the text is preserved. + expect(unpack(changeset).charBank).toBe('Hello'); + // It actually changed (was unattributed before). + expect(changeset).not.toBe(input); + }); + + it('leaves an already-attributed insert unchanged', () => { + // Build `Z:1>5*0+5$Hello` with author already in the pool. + const pool = new AttributePool(); + const n = pool.putAttrib(['author', AUTHOR]); // index 0 + const attributed = `Z:1>5*${n}+5$Hello`; + const jsonable = pool.toJsonable(); + const {changeset, apool} = stampAuthorOnInserts(attributed, jsonable, AUTHOR); + expect(changeset).toBe(attributed); // unchanged (no-op path) + expect(firstInsertAuthor(changeset, apool)).toBe(AUTHOR); + }); + + it('does not invent an author when authorId is empty', () => { + const input = 'Z:1>5+5$Hello'; + const {changeset} = stampAuthorOnInserts(input, EMPTY_POOL(), ''); + expect(changeset).toBe(input); // nothing to stamp with → unchanged + }); + + it('does not touch keep/remove-only changesets', () => { + // `Z:6<1=5-1$` — keep 5, remove 1; no insert ops. + const input = 'Z:6<1=5-1$x'; + const {changeset} = stampAuthorOnInserts(input, EMPTY_POOL(), AUTHOR); + expect(changeset).toBe(input); + }); + + it('preserves a non-author attribute already on the insert while adding author', () => { + // Insert with a bold attribute but no author. + const pool = new AttributePool(); + const b = pool.putAttrib(['bold', 'true']); // index 0 + const input = `Z:1>5*${b}+5$Hello`; + const {changeset, apool} = stampAuthorOnInserts(input, pool.toJsonable(), AUTHOR); + const outPool = (new AttributePool()).fromJsonable(apool); + let amap: AttributeMap | null = null; + for (const op of deserializeOps(unpack(changeset).ops)) { + if (op.opcode === '+') { amap = AttributeMap.fromString(op.attribs, outPool); break; } + } + expect(amap!.get('author')).toBe(AUTHOR); + expect(amap!.get('bold')).toBe('true'); + expect(() => checkRep(changeset)).not.toThrow(); + }); +});