mirror of
https://github.com/ether/etherpad-lite.git
synced 2026-07-18 00:57:55 +00:00
* fix(7686): legacy padOptions.userName/userColor=false breaks pad
Settings.json files generated before December 2021 used `false` as the
default for these two string options (commit 8c857a85a switched the
template default to `null` and noted "this change has no effect due to
a bug in how pad options are processed; that bug will be fixed in a
future commit" — the follow-up never landed). pad.ts:getParams() then
runs `false.toString()`, the resulting string "false" passes the
`!== false` sentinel check at _afterHandshake, and notifyChangeName
ships USERINFO_UPDATE with name="false" and colorId="false" (clobbered
via clientVars.userColor). The server's hex regex rejects the colour
and throws `malformed color: false`; the user sees their name as
"false" and a white swatch.
Defense in depth:
- Server: Settings.ts::reloadSettings() coerces legacy boolean false
to null for padOptions.userName / padOptions.userColor and warns the
operator, matching the existing disableIPlogging shim pattern.
- Client: the pad.ts userName / userColor callbacks reject the
literal "false" string so URL params (?userName=false) and any
other path that surfaces the sentinel as a string are also no-ops.
- Backend regression test mirrors the shim and asserts it normalizes
legacy false, leaves explicit values intact, leaves null untouched,
does not coerce other padOptions keys, and does not coerce the
string "false" (that path is the client guard's responsibility).
Closes #7686
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(7686): guard padOptions shim against non-object config (Qodo)
Qodo flagged that storeSettings() will overwrite settings.padOptions
raw with whatever settings.json supplies — including null, primitives,
or arrays — which would make the new userName/userColor shim crash on
property access. Add a shape guard so the shim is a no-op for malformed
padOptions, and extend the regression test to cover null / primitive /
array shapes.
This doesn't change which configs work (Pad.ts also assumes padOptions
is an object and would already crash on a null padOptions when a pad
is opened) but it stops the shim from being the loud thing in the
stack trace if someone hits it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
98 lines
3.8 KiB
TypeScript
98 lines
3.8 KiB
TypeScript
'use strict';
|
|
|
|
import {strict as assert} from 'assert';
|
|
|
|
describe(__filename, function () {
|
|
// Replicates the shim block from Settings.ts::reloadSettings so we can
|
|
// assert the mapping without rebooting the whole server in this spec.
|
|
// Keep this in lockstep with the production block — if you change the
|
|
// shim there, change it here too.
|
|
const applyShim = (padOptions: any) => {
|
|
if (padOptions != null && typeof padOptions === 'object' && !Array.isArray(padOptions)) {
|
|
for (const key of ['userName', 'userColor']) {
|
|
if (padOptions[key] === false) padOptions[key] = null;
|
|
}
|
|
}
|
|
return padOptions;
|
|
};
|
|
|
|
describe('legacy padOptions.userName/userColor=false → null shim', function () {
|
|
it('coerces userName=false to null', function () {
|
|
const out = applyShim({userName: false});
|
|
assert.strictEqual(out.userName, null);
|
|
});
|
|
|
|
it('coerces userColor=false to null', function () {
|
|
const out = applyShim({userColor: false});
|
|
assert.strictEqual(out.userColor, null);
|
|
});
|
|
|
|
it('coerces both legacy defaults in one pass', function () {
|
|
const out = applyShim({userName: false, userColor: false});
|
|
assert.strictEqual(out.userName, null);
|
|
assert.strictEqual(out.userColor, null);
|
|
});
|
|
|
|
it('leaves an explicit string userName intact', function () {
|
|
const out = applyShim({userName: 'Etherpad User'});
|
|
assert.strictEqual(out.userName, 'Etherpad User');
|
|
});
|
|
|
|
it('leaves an explicit hex userColor intact', function () {
|
|
const out = applyShim({userColor: '#ff9900'});
|
|
assert.strictEqual(out.userColor, '#ff9900');
|
|
});
|
|
|
|
it('leaves null values untouched', function () {
|
|
const out = applyShim({userName: null, userColor: null});
|
|
assert.strictEqual(out.userName, null);
|
|
assert.strictEqual(out.userColor, null);
|
|
});
|
|
|
|
it('does not affect other padOptions keys that legitimately use false', function () {
|
|
// showChat:false, rtl:false, etc. are real, meaningful values — only
|
|
// the two string options carry the legacy boolean sentinel.
|
|
const out = applyShim({
|
|
userName: false,
|
|
userColor: false,
|
|
showChat: false,
|
|
rtl: false,
|
|
useMonospaceFont: false,
|
|
});
|
|
assert.strictEqual(out.userName, null);
|
|
assert.strictEqual(out.userColor, null);
|
|
assert.strictEqual(out.showChat, false);
|
|
assert.strictEqual(out.rtl, false);
|
|
assert.strictEqual(out.useMonospaceFont, false);
|
|
});
|
|
|
|
it('does not coerce the string "false" — that is handled in the client guard', function () {
|
|
// The server-side shim only normalizes the boolean sentinel from
|
|
// legacy settings.json. URL-supplied or stringified "false" is
|
|
// rejected by pad.ts::getParameters.userName/userColor callbacks.
|
|
const out = applyShim({userName: 'false', userColor: 'false'});
|
|
assert.strictEqual(out.userName, 'false');
|
|
assert.strictEqual(out.userColor, 'false');
|
|
});
|
|
|
|
it('skips the shim if padOptions is null', function () {
|
|
// storeSettings() overwrites settings.padOptions raw if settings.json
|
|
// declares it as a non-object — the shim must not throw on that.
|
|
assert.doesNotThrow(() => applyShim(null));
|
|
assert.strictEqual(applyShim(null), null);
|
|
});
|
|
|
|
it('skips the shim if padOptions is a primitive', function () {
|
|
assert.doesNotThrow(() => applyShim(false));
|
|
assert.doesNotThrow(() => applyShim('not an object'));
|
|
assert.doesNotThrow(() => applyShim(42));
|
|
});
|
|
|
|
it('skips the shim if padOptions is an array', function () {
|
|
const arr: any = [false, false];
|
|
assert.doesNotThrow(() => applyShim(arr));
|
|
// Arrays pass through untouched — index 0/1 (numeric) are not coerced.
|
|
assert.deepEqual(applyShim(arr), [false, false]);
|
|
});
|
|
});
|
|
});
|