Skip to content

Commit 3387a5b

Browse files
JohnMcLearclaude
JohnMcLear
and
claude
committed
fix(admin): honour escape sequences in form-view string fields (#8211)
The settings form view rendered string values in single-line <input>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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012kA75NPq8nGRidAwhPXeCi
1 parent f95d67b commit 3387a5b

5 files changed

Lines changed: 241 additions & 13 deletions

File tree

‎admin/src/components/settings/__tests__/stringEscapes.test.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { test } from 'node:test';
2+
import assert from 'node:assert/strict';
3+
import { applyEdits, modify, parse } from 'jsonc-parser';
4+
5+
import { escapeForInput, unescapeFromInput } from '../stringEscapes.ts';
6+
7+
// Regression for https://github.com/ether/etherpad/issues/8211.
8+
9+
test('newlines and backslashes are shown as JSON escapes', () => {
10+
assert.equal(escapeForInput('Welcome\n\ntest\n'), 'Welcome\\n\\ntest\\n');
11+
assert.equal(escapeForInput('C:\\dir\tx\r'), 'C:\\\\dir\\tx\\r');
12+
assert.equal(escapeForInput('\u0001'), '\\u0001');
13+
});
14+
15+
test('quotes and slashes stay readable', () => {
16+
assert.equal(escapeForInput('say "hi" https://etherpad.org'), 'say "hi" https://etherpad.org');
17+
});
18+
19+
test('typed escape sequences decode to the characters they name', () => {
20+
assert.equal(unescapeFromInput('Welcome\\n\\ntest\\n'), 'Welcome\n\ntest\n');
21+
assert.equal(unescapeFromInput('a\\"b\\/c\\\\d\\te\\u00e9'), 'a"b/c\\d\te\u00e9');
22+
assert.equal(unescapeFromInput('plain "quoted" text'), 'plain "quoted" text');
23+
});
24+
25+
test('invalid or incomplete escapes are rejected', () => {
26+
assert.equal(unescapeFromInput('trailing\\'), null);
27+
assert.equal(unescapeFromInput('bad \\q escape'), null);
28+
assert.equal(unescapeFromInput('short \\u12'), null);
29+
});
30+
31+
test('round-trips arbitrary strings', () => {
32+
for (const s of ['', 'x', 'Welcome to Etherpad!\n\nGet involved\n', 'a\\n', '"\\"', '\u2028\u0000']) {
33+
assert.equal(unescapeFromInput(escapeForInput(s)), s);
34+
}
35+
});
36+
37+
test('typed \\n is written to settings JSON as \\n, not \\\\n', () => {
38+
const text = '{\n "defaultPadText": "old"\n}';
39+
const decoded = unescapeFromInput('Welcome\\n\\ntest\\n');
40+
const next = applyEdits(text, modify(text, ['defaultPadText'], decoded, {
41+
formattingOptions: { tabSize: 2, insertSpaces: true, eol: '\n' },
42+
}));
43+
assert.ok(next.includes('"Welcome\\n\\ntest\\n"'), next);
44+
assert.equal(parse(next).defaultPadText, 'Welcome\n\ntest\n');
45+
});

‎admin/src/components/settings/stringEscapes.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// admin/src/components/settings/stringEscapes.ts
2+
//
3+
// Form-view string widgets are single-line <input>s. Browsers strip line
4+
// breaks from an <input>'s value, and settings.json documents values such
5+
// as `"defaultPadText": "Line 1\nLine 2"` using JSON escape sequences. So
6+
// the widgets show and accept string values in that same escaped form:
7+
// a newline is displayed as `\n`, and typing `\n` stores a newline
8+
// (issue #8211). Double quotes and slashes are left as-is for readability;
9+
// their escaped forms (`\"`, `\/`) are still accepted on input.
10+
11+
const SHORT_ESCAPES: Record<string, string> = {
12+
'\\': '\\\\',
13+
'\n': '\\n',
14+
'\r': '\\r',
15+
'\t': '\\t',
16+
'\b': '\\b',
17+
'\f': '\\f',
18+
};
19+
20+
const SHORT_UNESCAPES: Record<string, string> = {
21+
'"': '"',
22+
'\\': '\\',
23+
'/': '/',
24+
b: '\b',
25+
f: '\f',
26+
n: '\n',
27+
r: '\r',
28+
t: '\t',
29+
};
30+
31+
/** Turn a decoded string value into the escaped text shown in an input. */
32+
export const escapeForInput = (value: string): string =>
33+
// eslint-disable-next-line no-control-regex
34+
value.replace(/[\\\u0000-\u001f\u2028\u2029]/g, (c) =>
35+
SHORT_ESCAPES[c] ?? `\\u${c.charCodeAt(0).toString(16).padStart(4, '0')}`);
36+
37+
/**
38+
* Decode the escaped text typed into an input back into the string value.
39+
* Returns null when the text contains an invalid or incomplete escape
40+
* sequence (e.g. a trailing `\` while the user is still typing).
41+
*/
42+
export const unescapeFromInput = (text: string): string | null => {
43+
let out = '';
44+
for (let i = 0; i < text.length; i++) {
45+
const c = text[i];
46+
if (c !== '\\') {
47+
out += c;
48+
continue;
49+
}
50+
const next = text[i + 1];
51+
if (next === undefined) return null;
52+
if (next === 'u') {
53+
const hex = text.slice(i + 2, i + 6);
54+
if (!/^[0-9a-fA-F]{4}$/.test(hex)) return null;
55+
out += String.fromCharCode(parseInt(hex, 16));
56+
i += 5;
57+
continue;
58+
}
59+
const decoded = SHORT_UNESCAPES[next];
60+
if (decoded === undefined) return null;
61+
out += decoded;
62+
i += 1;
63+
}
64+
return out;
65+
};

‎admin/src/components/settings/widgets/EnvPill.tsx‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { useEffect, useRef, useState } from 'react';
22
import { useTranslation } from 'react-i18next';
33
import type { JSONPath } from 'jsonc-parser';
44
import type { EnvPlaceholder } from '../envPill';
5+
import { escapeForInput, unescapeFromInput } from '../stringEscapes';
56

67
const REDACTED = '[REDACTED]';
78

@@ -22,7 +23,12 @@ const formatDisplay = (v: unknown): string => {
2223

2324
export const EnvPill = ({ placeholder, path, onChange, resolvedValue }: Props) => {
2425
const { t } = useTranslation();
25-
const initial = placeholder.defaultValue ?? '';
26+
// defaultValue is sliced from the raw JSON text, so it is still escaped.
27+
// Normalise it through decode/escape so it matches what StringInput shows
28+
// (e.g. `https:\/\/` displays as `https://`) — see stringEscapes.ts.
29+
const rawDefault = placeholder.defaultValue ?? '';
30+
const decodedDefault = unescapeFromInput(rawDefault);
31+
const initial = decodedDefault === null ? rawDefault : escapeForInput(decodedDefault);
2632
const [draft, setDraft] = useState(initial);
2733
const focused = useRef(false);
2834

@@ -63,7 +69,11 @@ export const EnvPill = ({ placeholder, path, onChange, resolvedValue }: Props) =
6369
onChange={e => {
6470
const v = sanitize(e.target.value);
6571
setDraft(v);
66-
onChange(v);
72+
// The draft is in escaped form; hand the decoded value to the
73+
// JSON writer so typed `\n` is stored as `\n`, not `\\n` (#8211).
74+
// Incomplete escapes (a trailing `\`) are not propagated.
75+
const decoded = unescapeFromInput(v);
76+
if (decoded !== null) onChange(sanitize(decoded));
6777
}}
6878
/>
6979
{hasResolved && !isRedacted && (

‎admin/src/components/settings/widgets/StringInput.tsx‎

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,53 @@
1+
import { useEffect, useRef, useState } from 'react';
12
import type { JSONPath } from 'jsonc-parser';
3+
import { escapeForInput, unescapeFromInput } from '../stringEscapes';
24

35
type Props = {
46
value: string;
57
path: JSONPath;
68
onChange: (next: string) => void;
79
};
810

9-
export const StringInput = ({ value, path, onChange }: Props) => (
10-
<input
11-
type="text"
12-
id={`field-${path.join('.')}`}
13-
className="settings-widget settings-widget-string"
14-
data-testid={`field-${path.join('.')}`}
15-
value={value}
16-
spellCheck={false}
17-
onChange={e => onChange(e.target.value)}
18-
/>
19-
);
11+
// The input shows the value in its JSON-escaped form (see stringEscapes.ts)
12+
// so newlines survive the single-line <input> and typed `\n` is stored as a
13+
// newline. While focused we keep the user's own text as a draft so partial
14+
// escapes (a lone trailing `\`) aren't reformatted mid-typing; invalid
15+
// drafts are not propagated.
16+
export const StringInput = ({ value, path, onChange }: Props) => {
17+
const escaped = escapeForInput(value);
18+
const [draft, setDraft] = useState(escaped);
19+
const [invalid, setInvalid] = useState(false);
20+
const focused = useRef(false);
21+
22+
useEffect(() => {
23+
if (!focused.current) {
24+
setDraft(escaped);
25+
setInvalid(false);
26+
}
27+
}, [escaped]);
28+
29+
return (
30+
<input
31+
type="text"
32+
id={`field-${path.join('.')}`}
33+
className="settings-widget settings-widget-string"
34+
data-testid={`field-${path.join('.')}`}
35+
value={draft}
36+
spellCheck={false}
37+
aria-invalid={invalid || undefined}
38+
onFocus={() => { focused.current = true; }}
39+
onBlur={() => {
40+
focused.current = false;
41+
setDraft(escaped);
42+
setInvalid(false);
43+
}}
44+
onChange={e => {
45+
const text = e.target.value;
46+
setDraft(text);
47+
const decoded = unescapeFromInput(text);
48+
setInvalid(decoded === null);
49+
if (decoded !== null) onChange(decoded);
50+
}}
51+
/>
52+
);
53+
};

‎src/tests/frontend-new/admin-spec/adminsettings.spec.ts‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,80 @@ test.describe('admin settings',()=> {
433433
expect(effectiveText).toContain('[REDACTED]');
434434
});
435435

436+
// Regression for https://github.com/ether/etherpad/issues/8211.
437+
// Form inputs are single-line, so string values are shown and edited in
438+
// their JSON-escaped form (the same form used in settings.json). Typing
439+
// `\n` must be saved as the JSON escape `\n` (a newline), not re-escaped
440+
// to `\\n` (a literal backslash + n).
441+
test('#8211 escape sequences typed in a form string field are saved unescaped', async ({page}) => {
442+
await page.goto('http://localhost:9001/admin/settings');
443+
await page.waitForSelector('[data-testid="settings-form-view"]', {timeout: 30000});
444+
await page.getByTestId('mode-toggle-raw').click();
445+
const raw = page.getByTestId('settings-raw-textarea');
446+
await expect(raw).toBeVisible({timeout: 10000});
447+
const original = await raw.inputValue();
448+
449+
await page.getByTestId('mode-toggle-form').click();
450+
const field = page.getByTestId('field-defaultPadText');
451+
await expect(field).toBeVisible({timeout: 10000});
452+
// Existing newlines are displayed as `\n` rather than silently dropped
453+
// by the single-line <input>.
454+
await expect(field).toHaveValue(/^Welcome to Etherpad!\\n\\nThis pad text/);
455+
await field.fill('Welcome\\n\\ntest "quoted" C:\\\\dir\\n');
456+
await saveSettings(page);
457+
458+
await page.reload();
459+
await page.waitForSelector('[data-testid="settings-form-view"]', {timeout: 30000});
460+
await expect(page.getByTestId('field-defaultPadText'))
461+
.toHaveValue('Welcome\\n\\ntest "quoted" C:\\\\dir\\n');
462+
await page.getByTestId('mode-toggle-raw').click();
463+
const after = await page.getByTestId('settings-raw-textarea').inputValue();
464+
const m = /^\s*"defaultPadText"\s*:\s*("(?:[^"\\]|\\.)*")/m.exec(after);
465+
expect(m).not.toBeNull();
466+
expect(JSON.parse(m![1])).toEqual('Welcome\n\ntest "quoted" C:\\dir\n');
467+
468+
// Restore
469+
await page.getByTestId('settings-raw-textarea').fill(original);
470+
await saveSettings(page);
471+
});
472+
473+
test('#8211 escape sequences typed in an env placeholder default are saved unescaped', async ({page}) => {
474+
await page.goto('http://localhost:9001/admin/settings');
475+
await page.waitForSelector('[data-testid="settings-form-view"]', {timeout: 30000});
476+
await page.getByTestId('mode-toggle-raw').click();
477+
const raw = page.getByTestId('settings-raw-textarea');
478+
await expect(raw).toBeVisible({timeout: 10000});
479+
const original = await raw.inputValue();
480+
481+
// The shape settings.json.docker uses for defaultPadText.
482+
// Anchor to the start of a line so the documentation comment in the
483+
// template (` * "defaultPadText" : ...`) is not the one replaced.
484+
const withEnv = original.replace(
485+
/^(\s*)"defaultPadText"\s*:\s*"(?:[^"\\]|\\.)*"/m,
486+
'$1"defaultPadText": "${DEFAULT_PAD_TEXT:Line 1\\nLine 2}"',
487+
);
488+
expect(withEnv).not.toEqual(original);
489+
await raw.fill(withEnv);
490+
await saveSettings(page);
491+
492+
await page.getByTestId('mode-toggle-form').click();
493+
const pill = page.getByTestId('env-defaultPadText');
494+
await expect(pill).toBeVisible({timeout: 10000});
495+
await expect(pill).toHaveValue('Line 1\\nLine 2');
496+
await pill.fill('Welcome\\n\\ntest\\n');
497+
await saveSettings(page);
498+
499+
await page.reload();
500+
await page.waitForSelector('[data-testid="settings-form-view"]', {timeout: 30000});
501+
await page.getByTestId('mode-toggle-raw').click();
502+
const after = await page.getByTestId('settings-raw-textarea').inputValue();
503+
expect(after).toContain('"${DEFAULT_PAD_TEXT:Welcome\\n\\ntest\\n}"');
504+
505+
// Restore
506+
await page.getByTestId('settings-raw-textarea').fill(original);
507+
await saveSettings(page);
508+
});
509+
436510
test('toggling form on broken raw JSON shows parse error banner', async ({page}) => {
437511
await page.goto('http://localhost:9001/admin/settings');
438512
// Wait for settings to load (form view renders once socket emits settings).

0 commit comments

Comments
 (0)