Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions admin/src/components/settings/__tests__/stringEscapes.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
65 changes: 65 additions & 0 deletions admin/src/components/settings/stringEscapes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// admin/src/components/settings/stringEscapes.ts
//
// Form-view string widgets are single-line <input>s. Browsers strip line
// breaks from an <input>'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<string, string> = {
'\\': '\\\\',
'\n': '\\n',
'\r': '\\r',
'\t': '\\t',
'\b': '\\b',
'\f': '\\f',
};

const SHORT_UNESCAPES: Record<string, string> = {
'"': '"',
'\\': '\\',
'/': '/',
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;
};
38 changes: 34 additions & 4 deletions admin/src/components/settings/widgets/EnvPill.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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]';

Expand All @@ -14,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;
Expand All @@ -22,12 +31,21 @@ 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 = 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('.')}`;
Expand Down Expand Up @@ -58,12 +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);
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 = decodeDefault(v);
setInvalid(decoded === null);
if (decoded !== null) onChange(decoded);
}}
/>
{hasResolved && !isRedacted && (
Expand Down
56 changes: 45 additions & 11 deletions admin/src/components/settings/widgets/StringInput.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,53 @@
import { useEffect, useRef, useState } from 'react';
import type { JSONPath } from 'jsonc-parser';
import { escapeForInput, unescapeFromInput } from '../stringEscapes';

type Props = {
value: string;
path: JSONPath;
onChange: (next: string) => void;
};

export const StringInput = ({ value, path, onChange }: Props) => (
<input
type="text"
id={`field-${path.join('.')}`}
className="settings-widget settings-widget-string"
data-testid={`field-${path.join('.')}`}
value={value}
spellCheck={false}
onChange={e => onChange(e.target.value)}
/>
);
// The input shows the value in its JSON-escaped form (see stringEscapes.ts)
// so newlines survive the single-line <input> 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 (
<input
type="text"
id={`field-${path.join('.')}`}
className="settings-widget settings-widget-string"
data-testid={`field-${path.join('.')}`}
value={draft}
spellCheck={false}
aria-invalid={invalid || undefined}
onFocus={() => { 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);
}}
/>
);
};
19 changes: 19 additions & 0 deletions admin/src/components/settings/widgets/__tests__/EnvPill.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
79 changes: 79 additions & 0 deletions src/tests/frontend-new/admin-spec/adminsettings.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@
const titleLabel = page.locator('label[for="field-title"]');
await expect(titleLabel).toBeVisible({timeout: 10000});
const titleRow = titleLabel.locator('xpath=ancestor::*[contains(@class,"settings-row")][1]');
await expect(titleRow).toContainText('CustomTitleLabel');

Check failure on line 212 in src/tests/frontend-new/admin-spec/adminsettings.spec.ts

View workflow job for this annotation

GitHub Actions / with plugins (24)

[chromium-admin] › tests/frontend-new/admin-spec/adminsettings.spec.ts:187:7 › admin settings › form view derives label + help text from key comment

1) [chromium-admin] › tests/frontend-new/admin-spec/adminsettings.spec.ts:187:7 › admin settings › form view derives label + help text from key comment Error: expect(locator).toContainText(expected) failed Locator: locator('label[for="field-title"]').locator('xpath=ancestor::*[contains(@Class,"settings-row")][1]') Expected substring: "CustomTitleLabel" Received string: "Name your instance!" Timeout: 20000ms Call log: - Expect "toContainText" locator('label[for="field-title"]').locator('xpath=ancestor::*[contains(@Class,"settings-row")][1]') with timeout 20000ms - waiting for locator('label[for="field-title"]').locator('xpath=ancestor::*[contains(@Class,"settings-row")][1]') 44 × locator resolved to <div class="settings-row" id="settings-row-title">…</div> - unexpected value "Name your instance!" 210 | await expect(titleLabel).toBeVisible({timeout: 10000}); 211 | const titleRow = titleLabel.locator('xpath=ancestor::*[contains(@Class,"settings-row")][1]'); > 212 | await expect(titleRow).toContainText('CustomTitleLabel'); | ^ 213 | await expect(titleRow).toContainText('ExtraHelpMarker'); 214 | 215 | // Restore at /home/runner/work/etherpad/etherpad/src/tests/frontend-new/admin-spec/adminsettings.spec.ts:212:28

Check failure on line 212 in src/tests/frontend-new/admin-spec/adminsettings.spec.ts

View workflow job for this annotation

GitHub Actions / with plugins (24)

[chromium-admin] › tests/frontend-new/admin-spec/adminsettings.spec.ts:187:7 › admin settings › form view derives label + help text from key comment

1) [chromium-admin] › tests/frontend-new/admin-spec/adminsettings.spec.ts:187:7 › admin settings › form view derives label + help text from key comment Error: expect(locator).toContainText(expected) failed Locator: locator('label[for="field-title"]').locator('xpath=ancestor::*[contains(@Class,"settings-row")][1]') Expected substring: "CustomTitleLabel" Received string: "Name your instance!" Timeout: 20000ms Call log: - Expect "toContainText" locator('label[for="field-title"]').locator('xpath=ancestor::*[contains(@Class,"settings-row")][1]') with timeout 20000ms - waiting for locator('label[for="field-title"]').locator('xpath=ancestor::*[contains(@Class,"settings-row")][1]') 44 × locator resolved to <div class="settings-row" id="settings-row-title">…</div> - unexpected value "Name your instance!" 210 | await expect(titleLabel).toBeVisible({timeout: 10000}); 211 | const titleRow = titleLabel.locator('xpath=ancestor::*[contains(@Class,"settings-row")][1]'); > 212 | await expect(titleRow).toContainText('CustomTitleLabel'); | ^ 213 | await expect(titleRow).toContainText('ExtraHelpMarker'); 214 | 215 | // Restore at /home/runner/work/etherpad/etherpad/src/tests/frontend-new/admin-spec/adminsettings.spec.ts:212:28
await expect(titleRow).toContainText('ExtraHelpMarker');

// Restore
Expand Down Expand Up @@ -433,6 +433,85 @@
expect(effectiveText).toContain('[REDACTED]');
});

// 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
// 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 <input>.
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 = lastDefaultPadText(after);
expect(m).not.toBeUndefined();
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.
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);

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).
Expand Down
Loading