diff --git a/src/node/utils/Settings.ts b/src/node/utils/Settings.ts index 71bb4dd1e7d..54840f03a7a 100644 --- a/src/node/utils/Settings.ts +++ b/src/node/utils/Settings.ts @@ -1087,6 +1087,30 @@ export const reloadSettings = () => { settings.privacyBanner.dismissal = 'dismissible'; } + // Settings.json files generated before December 2021 used `false` as the + // default for these string options. The client treats the boolean `false` + // as a sentinel meaning "no enforced value", but the dispatch in + // pad.ts:getParams() coerces the boolean to the string "false" before + // applying it, which then propagates as the user's name and color and + // triggers `malformed color: false` on the server (#7686). Normalize + // legacy booleans to null at the boundary so downstream code sees the + // expected sentinel. Guard against a malformed padOptions (null, array, + // primitive) — storeSettings() will overwrite it raw if settings.json + // declares it as anything other than a plain object. + if (settings.padOptions != null + && typeof settings.padOptions === 'object' + && !Array.isArray(settings.padOptions)) { + for (const key of ['userName', 'userColor'] as const) { + if ((settings.padOptions as any)[key] === false) { + logger.warn( + `padOptions.${key}=false is a legacy default (pre-2021) and is ` + + `now treated as null. Update settings.json to use null instead ` + + `to silence this warning.`); + (settings.padOptions as any)[key] = null; + } + } + } + // Init logging config settings.logconfig = defaultLogConfig( settings.loglevel ? settings.loglevel : defaultLogLevel, diff --git a/src/static/js/pad.ts b/src/static/js/pad.ts index 72364c26149..12406197ecf 100644 --- a/src/static/js/pad.ts +++ b/src/static/js/pad.ts @@ -136,6 +136,14 @@ const getParameters = [ name: 'userName', checkVal: null, callback: (val) => { + // The default for globalUserName/globalUserColor is the boolean `false` + // (sentinel meaning "no enforced value"). Older settings.json files used + // boolean `false` for these options too, which getParams() coerces to + // the string "false" — that fooled the !== false sentinel checks at + // _afterHandshake and shipped the literal string "false" as the user's + // name and color (#7686). Reject the sentinel string here so URL + // parameters like ?userName=false also no-op. + if (!val || val === 'false') return; settings.globalUserName = val; clientVars.userName = val; }, @@ -144,6 +152,7 @@ const getParameters = [ name: 'userColor', checkVal: null, callback: (val) => { + if (!val || val === 'false') return; settings.globalUserColor = val; clientVars.userColor = val; }, diff --git a/src/tests/backend/specs/padOptionsLegacyDefaults.ts b/src/tests/backend/specs/padOptionsLegacyDefaults.ts new file mode 100644 index 00000000000..73d0a08d588 --- /dev/null +++ b/src/tests/backend/specs/padOptionsLegacyDefaults.ts @@ -0,0 +1,98 @@ +'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]); + }); + }); +});