fix(7686): username 'false' / 'malformed color: false' for legacy settings.json

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,

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;
     },

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]);
+    });
+  });
+});