From 3387a5bd57591e0aece04c78c279f4d2630ecfa7 Mon Sep 17 00:00:00 2001 From: John McLear Date: Thu, 17 Sep 2026 13:20:24 +0100 Subject: [PATCH 1/3] fix(admin): honour escape sequences in form-view string fields (#8211) The settings form view rendered string values in single-line s. Browsers strip line breaks from an input's value, so values such as defaultPadText lost their newlines, and anything typed (e.g. `\n`) was JSON-encoded again on save, producing `\\n` in settings.json and a literal backslash-n in the pad text. Env placeholder defaults were shown in raw escaped form but re-escaped on save, with the same result. String inputs and env placeholder defaults now show values in their JSON-escaped form and decode typed escape sequences before writing, so `Welcome\n\ntest\n` is stored exactly like a hand-edited settings.json. Incomplete escapes (a trailing backslash while typing) are not propagated. Fixes #8211 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012kA75NPq8nGRidAwhPXeCi --- .../settings/__tests__/stringEscapes.test.ts | 45 +++++++++++ .../src/components/settings/stringEscapes.ts | 65 ++++++++++++++++ .../components/settings/widgets/EnvPill.tsx | 14 +++- .../settings/widgets/StringInput.tsx | 56 +++++++++++--- .../admin-spec/adminsettings.spec.ts | 74 +++++++++++++++++++ 5 files changed, 241 insertions(+), 13 deletions(-) create mode 100644 admin/src/components/settings/__tests__/stringEscapes.test.ts create mode 100644 admin/src/components/settings/stringEscapes.ts diff --git a/admin/src/components/settings/__tests__/stringEscapes.test.ts b/admin/src/components/settings/__tests__/stringEscapes.test.ts new file mode 100644 index 00000000000..7c32a0accbf --- /dev/null +++ b/admin/src/components/settings/__tests__/stringEscapes.test.ts @@ -0,0 +1,45 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { applyEdits, modify, parse } from 'jsonc-parser'; + +import { escapeForInput, unescapeFromInput } from '../stringEscapes.ts'; + +// Regression for https://github.com/ether/etherpad/issues/8211. + +test('newlines and backslashes are shown as JSON escapes', () => { + assert.equal(escapeForInput('Welcome\n\ntest\n'), 'Welcome\\n\\ntest\\n'); + assert.equal(escapeForInput('C:\\dir\tx\r'), 'C:\\\\dir\\tx\\r'); + assert.equal(escapeForInput('\u0001'), '\\u0001'); +}); + +test('quotes and slashes stay readable', () => { + assert.equal(escapeForInput('say "hi" https://etherpad.org'), 'say "hi" https://etherpad.org'); +}); + +test('typed escape sequences decode to the characters they name', () => { + assert.equal(unescapeFromInput('Welcome\\n\\ntest\\n'), 'Welcome\n\ntest\n'); + assert.equal(unescapeFromInput('a\\"b\\/c\\\\d\\te\\u00e9'), 'a"b/c\\d\te\u00e9'); + assert.equal(unescapeFromInput('plain "quoted" text'), 'plain "quoted" text'); +}); + +test('invalid or incomplete escapes are rejected', () => { + assert.equal(unescapeFromInput('trailing\\'), null); + assert.equal(unescapeFromInput('bad \\q escape'), null); + assert.equal(unescapeFromInput('short \\u12'), null); +}); + +test('round-trips arbitrary strings', () => { + for (const s of ['', 'x', 'Welcome to Etherpad!\n\nGet involved\n', 'a\\n', '"\\"', '\u2028\u0000']) { + assert.equal(unescapeFromInput(escapeForInput(s)), s); + } +}); + +test('typed \\n is written to settings JSON as \\n, not \\\\n', () => { + const text = '{\n "defaultPadText": "old"\n}'; + const decoded = unescapeFromInput('Welcome\\n\\ntest\\n'); + const next = applyEdits(text, modify(text, ['defaultPadText'], decoded, { + formattingOptions: { tabSize: 2, insertSpaces: true, eol: '\n' }, + })); + assert.ok(next.includes('"Welcome\\n\\ntest\\n"'), next); + assert.equal(parse(next).defaultPadText, 'Welcome\n\ntest\n'); +}); diff --git a/admin/src/components/settings/stringEscapes.ts b/admin/src/components/settings/stringEscapes.ts new file mode 100644 index 00000000000..8d794f8b227 --- /dev/null +++ b/admin/src/components/settings/stringEscapes.ts @@ -0,0 +1,65 @@ +// admin/src/components/settings/stringEscapes.ts +// +// Form-view string widgets are single-line s. Browsers strip line +// breaks from an 's value, and settings.json documents values such +// as `"defaultPadText": "Line 1\nLine 2"` using JSON escape sequences. So +// the widgets show and accept string values in that same escaped form: +// a newline is displayed as `\n`, and typing `\n` stores a newline +// (issue #8211). Double quotes and slashes are left as-is for readability; +// their escaped forms (`\"`, `\/`) are still accepted on input. + +const SHORT_ESCAPES: Record = { + '\\': '\\\\', + '\n': '\\n', + '\r': '\\r', + '\t': '\\t', + '\b': '\\b', + '\f': '\\f', +}; + +const SHORT_UNESCAPES: Record = { + '"': '"', + '\\': '\\', + '/': '/', + b: '\b', + f: '\f', + n: '\n', + r: '\r', + t: '\t', +}; + +/** Turn a decoded string value into the escaped text shown in an input. */ +export const escapeForInput = (value: string): string => + // eslint-disable-next-line no-control-regex + value.replace(/[\\\u0000-\u001f\u2028\u2029]/g, (c) => + SHORT_ESCAPES[c] ?? `\\u${c.charCodeAt(0).toString(16).padStart(4, '0')}`); + +/** + * Decode the escaped text typed into an input back into the string value. + * Returns null when the text contains an invalid or incomplete escape + * sequence (e.g. a trailing `\` while the user is still typing). + */ +export const unescapeFromInput = (text: string): string | null => { + let out = ''; + for (let i = 0; i < text.length; i++) { + const c = text[i]; + if (c !== '\\') { + out += c; + continue; + } + const next = text[i + 1]; + if (next === undefined) return null; + if (next === 'u') { + const hex = text.slice(i + 2, i + 6); + if (!/^[0-9a-fA-F]{4}$/.test(hex)) return null; + out += String.fromCharCode(parseInt(hex, 16)); + i += 5; + continue; + } + const decoded = SHORT_UNESCAPES[next]; + if (decoded === undefined) return null; + out += decoded; + i += 1; + } + return out; +}; diff --git a/admin/src/components/settings/widgets/EnvPill.tsx b/admin/src/components/settings/widgets/EnvPill.tsx index 9ec97e949c4..ff3ec771b7f 100644 --- a/admin/src/components/settings/widgets/EnvPill.tsx +++ b/admin/src/components/settings/widgets/EnvPill.tsx @@ -2,6 +2,7 @@ import { useEffect, useRef, useState } from 'react'; import { useTranslation } from 'react-i18next'; import type { JSONPath } from 'jsonc-parser'; import type { EnvPlaceholder } from '../envPill'; +import { escapeForInput, unescapeFromInput } from '../stringEscapes'; const REDACTED = '[REDACTED]'; @@ -22,7 +23,12 @@ const formatDisplay = (v: unknown): string => { export const EnvPill = ({ placeholder, path, onChange, resolvedValue }: Props) => { const { t } = useTranslation(); - const initial = placeholder.defaultValue ?? ''; + // defaultValue is sliced from the raw JSON text, so it is still escaped. + // Normalise it through decode/escape so it matches what StringInput shows + // (e.g. `https:\/\/` displays as `https://`) — see stringEscapes.ts. + const rawDefault = placeholder.defaultValue ?? ''; + const decodedDefault = unescapeFromInput(rawDefault); + const initial = decodedDefault === null ? rawDefault : escapeForInput(decodedDefault); const [draft, setDraft] = useState(initial); const focused = useRef(false); @@ -63,7 +69,11 @@ export const EnvPill = ({ placeholder, path, onChange, resolvedValue }: Props) = onChange={e => { const v = sanitize(e.target.value); setDraft(v); - onChange(v); + // The draft is in escaped form; hand the decoded value to the + // JSON writer so typed `\n` is stored as `\n`, not `\\n` (#8211). + // Incomplete escapes (a trailing `\`) are not propagated. + const decoded = unescapeFromInput(v); + if (decoded !== null) onChange(sanitize(decoded)); }} /> {hasResolved && !isRedacted && ( diff --git a/admin/src/components/settings/widgets/StringInput.tsx b/admin/src/components/settings/widgets/StringInput.tsx index dd3efe51591..b4af9e2368d 100644 --- a/admin/src/components/settings/widgets/StringInput.tsx +++ b/admin/src/components/settings/widgets/StringInput.tsx @@ -1,4 +1,6 @@ +import { useEffect, useRef, useState } from 'react'; import type { JSONPath } from 'jsonc-parser'; +import { escapeForInput, unescapeFromInput } from '../stringEscapes'; type Props = { value: string; @@ -6,14 +8,46 @@ type Props = { onChange: (next: string) => void; }; -export const StringInput = ({ value, path, onChange }: Props) => ( - onChange(e.target.value)} - /> -); +// The input shows the value in its JSON-escaped form (see stringEscapes.ts) +// so newlines survive the single-line and typed `\n` is stored as a +// newline. While focused we keep the user's own text as a draft so partial +// escapes (a lone trailing `\`) aren't reformatted mid-typing; invalid +// drafts are not propagated. +export const StringInput = ({ value, path, onChange }: Props) => { + const escaped = escapeForInput(value); + const [draft, setDraft] = useState(escaped); + const [invalid, setInvalid] = useState(false); + const focused = useRef(false); + + useEffect(() => { + if (!focused.current) { + setDraft(escaped); + setInvalid(false); + } + }, [escaped]); + + return ( + { focused.current = true; }} + onBlur={() => { + focused.current = false; + setDraft(escaped); + setInvalid(false); + }} + onChange={e => { + const text = e.target.value; + setDraft(text); + const decoded = unescapeFromInput(text); + setInvalid(decoded === null); + if (decoded !== null) onChange(decoded); + }} + /> + ); +}; diff --git a/src/tests/frontend-new/admin-spec/adminsettings.spec.ts b/src/tests/frontend-new/admin-spec/adminsettings.spec.ts index b82438d1bc7..92c33ca26aa 100644 --- a/src/tests/frontend-new/admin-spec/adminsettings.spec.ts +++ b/src/tests/frontend-new/admin-spec/adminsettings.spec.ts @@ -433,6 +433,80 @@ test.describe('admin settings',()=> { expect(effectiveText).toContain('[REDACTED]'); }); + // Regression for https://github.com/ether/etherpad/issues/8211. + // Form inputs are single-line, so string values are shown and edited in + // their JSON-escaped form (the same form used in settings.json). Typing + // `\n` must be saved as the JSON escape `\n` (a newline), not re-escaped + // to `\\n` (a literal backslash + n). + test('#8211 escape sequences typed in a form string field are saved unescaped', async ({page}) => { + await page.goto('http://localhost:9001/admin/settings'); + await page.waitForSelector('[data-testid="settings-form-view"]', {timeout: 30000}); + await page.getByTestId('mode-toggle-raw').click(); + const raw = page.getByTestId('settings-raw-textarea'); + await expect(raw).toBeVisible({timeout: 10000}); + const original = await raw.inputValue(); + + await page.getByTestId('mode-toggle-form').click(); + const field = page.getByTestId('field-defaultPadText'); + await expect(field).toBeVisible({timeout: 10000}); + // Existing newlines are displayed as `\n` rather than silently dropped + // by the single-line . + await expect(field).toHaveValue(/^Welcome to Etherpad!\\n\\nThis pad text/); + await field.fill('Welcome\\n\\ntest "quoted" C:\\\\dir\\n'); + await saveSettings(page); + + await page.reload(); + await page.waitForSelector('[data-testid="settings-form-view"]', {timeout: 30000}); + await expect(page.getByTestId('field-defaultPadText')) + .toHaveValue('Welcome\\n\\ntest "quoted" C:\\\\dir\\n'); + await page.getByTestId('mode-toggle-raw').click(); + const after = await page.getByTestId('settings-raw-textarea').inputValue(); + const m = /^\s*"defaultPadText"\s*:\s*("(?:[^"\\]|\\.)*")/m.exec(after); + expect(m).not.toBeNull(); + expect(JSON.parse(m![1])).toEqual('Welcome\n\ntest "quoted" C:\\dir\n'); + + // Restore + await page.getByTestId('settings-raw-textarea').fill(original); + await saveSettings(page); + }); + + test('#8211 escape sequences typed in an env placeholder default are saved unescaped', async ({page}) => { + await page.goto('http://localhost:9001/admin/settings'); + await page.waitForSelector('[data-testid="settings-form-view"]', {timeout: 30000}); + await page.getByTestId('mode-toggle-raw').click(); + const raw = page.getByTestId('settings-raw-textarea'); + await expect(raw).toBeVisible({timeout: 10000}); + const original = await raw.inputValue(); + + // The shape settings.json.docker uses for defaultPadText. + // Anchor to the start of a line so the documentation comment in the + // template (` * "defaultPadText" : ...`) is not the one replaced. + const withEnv = original.replace( + /^(\s*)"defaultPadText"\s*:\s*"(?:[^"\\]|\\.)*"/m, + '$1"defaultPadText": "${DEFAULT_PAD_TEXT:Line 1\\nLine 2}"', + ); + expect(withEnv).not.toEqual(original); + await raw.fill(withEnv); + await saveSettings(page); + + await page.getByTestId('mode-toggle-form').click(); + const pill = page.getByTestId('env-defaultPadText'); + await expect(pill).toBeVisible({timeout: 10000}); + await expect(pill).toHaveValue('Line 1\\nLine 2'); + await pill.fill('Welcome\\n\\ntest\\n'); + await saveSettings(page); + + await page.reload(); + await page.waitForSelector('[data-testid="settings-form-view"]', {timeout: 30000}); + await page.getByTestId('mode-toggle-raw').click(); + const after = await page.getByTestId('settings-raw-textarea').inputValue(); + expect(after).toContain('"${DEFAULT_PAD_TEXT:Welcome\\n\\ntest\\n}"'); + + // Restore + await page.getByTestId('settings-raw-textarea').fill(original); + await saveSettings(page); + }); + test('toggling form on broken raw JSON shows parse error banner', async ({page}) => { await page.goto('http://localhost:9001/admin/settings'); // Wait for settings to load (form view renders once socket emits settings). From dec36a8ead843610c261f99feff57ceb951b3ed0 Mon Sep 17 00:00:00 2001 From: John McLear Date: Thu, 17 Sep 2026 13:35:04 +0100 Subject: [PATCH 2/3] test(admin): locate defaultPadText robustly in #8211 specs Earlier admin specs rewrite settings.json as minified JSON, so the key is not at the start of a line in CI. Take the last match instead, which also skips the template's documentation comment. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012kA75NPq8nGRidAwhPXeCi --- .../admin-spec/adminsettings.spec.ts | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/tests/frontend-new/admin-spec/adminsettings.spec.ts b/src/tests/frontend-new/admin-spec/adminsettings.spec.ts index 92c33ca26aa..c8d4cf3673b 100644 --- a/src/tests/frontend-new/admin-spec/adminsettings.spec.ts +++ b/src/tests/frontend-new/admin-spec/adminsettings.spec.ts @@ -434,6 +434,12 @@ test.describe('admin settings',()=> { }); // Regression for https://github.com/ether/etherpad/issues/8211. + // settings.json.template also mentions `"defaultPadText"` inside its + // documentation comment, which precedes the real key; earlier specs may + // also have minified the file onto one line. So take the last match. + const lastDefaultPadText = (text: string) => + [...text.matchAll(/"defaultPadText"\s*:\s*("(?:[^"\\]|\\.)*")/g)].pop(); + // Form inputs are single-line, so string values are shown and edited in // their JSON-escaped form (the same form used in settings.json). Typing // `\n` must be saved as the JSON escape `\n` (a newline), not re-escaped @@ -461,8 +467,8 @@ test.describe('admin settings',()=> { .toHaveValue('Welcome\\n\\ntest "quoted" C:\\\\dir\\n'); await page.getByTestId('mode-toggle-raw').click(); const after = await page.getByTestId('settings-raw-textarea').inputValue(); - const m = /^\s*"defaultPadText"\s*:\s*("(?:[^"\\]|\\.)*")/m.exec(after); - expect(m).not.toBeNull(); + const m = lastDefaultPadText(after); + expect(m).not.toBeUndefined(); expect(JSON.parse(m![1])).toEqual('Welcome\n\ntest "quoted" C:\\dir\n'); // Restore @@ -479,12 +485,11 @@ test.describe('admin settings',()=> { const original = await raw.inputValue(); // The shape settings.json.docker uses for defaultPadText. - // Anchor to the start of a line so the documentation comment in the - // template (` * "defaultPadText" : ...`) is not the one replaced. - const withEnv = original.replace( - /^(\s*)"defaultPadText"\s*:\s*"(?:[^"\\]|\\.)*"/m, - '$1"defaultPadText": "${DEFAULT_PAD_TEXT:Line 1\\nLine 2}"', - ); + const m = lastDefaultPadText(original); + expect(m).not.toBeUndefined(); + const withEnv = original.slice(0, m!.index) + + '"defaultPadText": "${DEFAULT_PAD_TEXT:Line 1\\nLine 2}"' + + original.slice(m!.index + m![0].length); expect(withEnv).not.toEqual(original); await raw.fill(withEnv); await saveSettings(page); From f3bd223d7858fb6c6cab308225be0d8c657c598e Mon Sep 17 00:00:00 2001 From: John McLear Date: Thu, 17 Sep 2026 13:36:28 +0100 Subject: [PATCH 3/3] fix(admin): reset rejected env-pill drafts on blur, don't drop decoded braces Address review: EnvPill now marks undecodable drafts aria-invalid and restores the applied value on blur (matching StringInput), and a default whose decoded form contains `}` is rejected/kept raw instead of having the brace silently stripped. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012kA75NPq8nGRidAwhPXeCi --- .../components/settings/widgets/EnvPill.tsx | 30 +++++++++++++++---- .../widgets/__tests__/EnvPill.test.tsx | 19 ++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/admin/src/components/settings/widgets/EnvPill.tsx b/admin/src/components/settings/widgets/EnvPill.tsx index ff3ec771b7f..9e4e33eda36 100644 --- a/admin/src/components/settings/widgets/EnvPill.tsx +++ b/admin/src/components/settings/widgets/EnvPill.tsx @@ -15,6 +15,14 @@ type Props = { const sanitize = (s: string) => s.replace(/[}]/g, ''); +// Decode an escaped default. A `}` (e.g. from `\u007d`) would terminate the +// `${VAR:default}` placeholder, so treat it as invalid rather than silently +// dropping it. +const decodeDefault = (s: string): string | null => { + const decoded = unescapeFromInput(s); + return decoded === null || decoded.includes('}') ? null : decoded; +}; + const formatDisplay = (v: unknown): string => { if (v === null) return 'null'; if (typeof v === 'string') return v; @@ -27,13 +35,17 @@ export const EnvPill = ({ placeholder, path, onChange, resolvedValue }: Props) = // Normalise it through decode/escape so it matches what StringInput shows // (e.g. `https:\/\/` displays as `https://`) — see stringEscapes.ts. const rawDefault = placeholder.defaultValue ?? ''; - const decodedDefault = unescapeFromInput(rawDefault); + const decodedDefault = decodeDefault(rawDefault); const initial = decodedDefault === null ? rawDefault : escapeForInput(decodedDefault); const [draft, setDraft] = useState(initial); + const [invalid, setInvalid] = useState(false); const focused = useRef(false); useEffect(() => { - if (!focused.current) setDraft(initial); + if (!focused.current) { + setDraft(initial); + setInvalid(false); + } }, [initial]); const id = `field-${path.join('.')}`; @@ -64,16 +76,24 @@ export const EnvPill = ({ placeholder, path, onChange, resolvedValue }: Props) = value={draft} spellCheck={false} aria-label={t('admin_settings.env_pill.input_aria', { variable: placeholder.variable })} + aria-invalid={invalid || undefined} onFocus={() => { focused.current = true; }} - onBlur={() => { focused.current = false; }} + onBlur={() => { + focused.current = false; + // Drop a rejected draft so the field never shows a value that + // was not applied to the settings text. + setDraft(initial); + setInvalid(false); + }} onChange={e => { const v = sanitize(e.target.value); setDraft(v); // The draft is in escaped form; hand the decoded value to the // JSON writer so typed `\n` is stored as `\n`, not `\\n` (#8211). // Incomplete escapes (a trailing `\`) are not propagated. - const decoded = unescapeFromInput(v); - if (decoded !== null) onChange(sanitize(decoded)); + const decoded = decodeDefault(v); + setInvalid(decoded === null); + if (decoded !== null) onChange(decoded); }} /> {hasResolved && !isRedacted && ( diff --git a/admin/src/components/settings/widgets/__tests__/EnvPill.test.tsx b/admin/src/components/settings/widgets/__tests__/EnvPill.test.tsx index 1ea2f3b2fc9..51a3780f3d2 100644 --- a/admin/src/components/settings/widgets/__tests__/EnvPill.test.tsx +++ b/admin/src/components/settings/widgets/__tests__/EnvPill.test.tsx @@ -84,3 +84,22 @@ test('renders null resolved value as the string null', () => { } as any)); assert.ok(html.includes('null'), `expected "null" in ${html}`); }); + +// https://github.com/ether/etherpad/issues/8211 +test('shows escaped defaults in their escaped form', () => { + const html = wrap(React.createElement(EnvPill, { + placeholder: { variable: 'DEFAULT_PAD_TEXT', defaultValue: 'Line 1\\nLine 2 https:\\/\\/x' }, + path: ['defaultPadText'], + onChange: () => {}, + })); + assert.ok(html.includes('value="Line 1\\nLine 2 https://x"'), html); +}); + +test('keeps a default whose decoded form contains } in raw form', () => { + const html = wrap(React.createElement(EnvPill, { + placeholder: { variable: 'X', defaultValue: 'a\\u007db' }, + path: ['x'], + onChange: () => {}, + })); + assert.ok(html.includes('value="a\\u007db"'), html); +});